Bug 2246032

Summary:	Likely bogus errors from valgrind 3.22.0-0.1.RC1.fc40.i686
Product:	[Fedora] Fedora	Reporter:	Mattias Ellert <mattias.ellert>
Component:	valgrind	Assignee:	Mark Wielaard <mjw>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	high	Docs Contact:
Priority:	unspecified
Version:	rawhide	CC:	ahajkova, dodji, fw, fweimer, jakub, jchecahi, mjw
Target Milestone:	---	Keywords:	Regression
Target Release:	---
Hardware:	i686
OS:	Linux
Whiteboard:
Fixed In Version:	valgrind-3.22.0-0.1.RC2.fc40	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2023-10-26 13:00:47 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	2245854

Description Mattias Ellert 2023-10-25 03:38:56 UTC

I recently attempted tp rebuild the HepMC3 package for Fedora rawhide. This package has a test suite that runs some of its tests using valgrind.

The build succeeded an all architectures except i686, where there are test failures due to errors for valgrind.

The build is using the current version in rawhide: valgrind-3.22.0-0.1.RC1.fc40.i686

See logs on koji:
https://koji.fedoraproject.org/koji/taskinfo?taskID=108050939

The error is reproducible using a mock chroot:
$ fedpkg clone HepMC3
$ cd HepMC3
$ fedpkg mockbuild --root fedora-rawhide-i386 --no-cleanup-after

Downgrading valgrind in the mock chroot to the previous version using the RPM downloaded from koji:
$ mock --root fedora-rawhide-i386 --install valgrind-3.21.0-10.fc40.i686.rpm

and then rerunning ctest in the chroot, it succeeds.


Reproducible: Always

Comment 1 Florian Weimer 2023-10-25 09:27:55 UTC

Quoting some of the errors in case the log expires:

==25039== Conditional jump or move depends on uninitialised value(s)
==25039==    at 0x4846443: operator delete[](void*) (vg_replace_malloc.c:1331)
==25039==    by 0x4A0509D: std::basic_filebuf<char, std::char_traits<char> >::_M_destroy_internal_buffer() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x4A08325: std::basic_filebuf<char, std::char_traits<char> >::close() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x4891347: UnknownInlinedFun (fstream:1002)
==25039==    by 0x4891347: HepMC3::WriterAscii::close() (WriterAscii.cc:368)
==25039==    by 0x10DBBE: main (testBoost.cc:133)
==25039== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZdaPv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE26_M_destroy_internal_bufferEv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE5closeEv
   fun:UnknownInlinedFun
   fun:_ZN6HepMC311WriterAscii5closeEv
   fun:main
}
GenParticle:   1 PDGID:  2212 (P,E)=-8.32e+02,-2.50e+03,+8.67e+03,+9.06e+03 Stat: 3 PV: 0 EV: 0 Attr: 3 flow1=231 phi=2.47798101119221 theta=2.63952755984961
GenParticle:   2 PDGID:  2212 (P,E)=-6.75e+02,-2.03e+03,-5.65e+03,+6.04e+03 Stat: 3 PV: 0 EV: -2 Attr: 3 flow1=243 phi=5.01674668700084 theta=2.46017876839191
GenParticle:   3 PDGID:     1 (P,E)=-3.11e+00,-1.31e+01,+3.99e+01,+4.21e+01 Stat: 3 PV: -1 EV: -3 Attr: 3 flow1=231 phi=1.2412518609574 theta=2.86402464235875
GenParticle:   4 PDGID:    -2 (P,E)=-9.02e+00,-3.69e+01,-4.27e+01,+5.72e+01 Stat: 3 PV: -2 EV: -3 Attr: 3 flow1=243 phi=4.82692890266268 theta=1.05313334667002
GenParticle:   5 PDGID:    22 (P,E)=-4.27e+00,-1.25e+00,-9.22e-01,+4.54e+00 Stat: 1 PV: -3 EV: 0 Attr: 3 flow1=231 phi=3.48069588687277 theta=0.872654990812324
GenParticle:   6 PDGID:   -24 (P,E)=-7.86e+00,-4.88e+01,-1.86e+00,+9.47e+01 Stat: 3 PV: -3 EV: -4 Attr: 3 flow1=243 phi=3.95131255457656 theta=1.49978707097559
GenParticle:   7 PDGID:     1 (P,E)=-5.23e+00,+2.05e+01,+1.17e+01,+2.41e+01 Stat: 1 PV: -4 EV: 0 Attr: 0
GenParticle:   8 PDGID:    -2 (P,E)=-2.63e+00,-6.93e+01,-1.35e+01,+7.06e+01 Stat: 1 PV: -4 EV: 0 Attr: 0
GenParticle:   1 PDGID:  2212 (P,E)=+1.00e+00,+1.00e+00,+7.00e+03,+7.00e+03 Stat: 3 PV: 0 EV: 0 Attr: 3 flow1=231 phi=2.47798101119221 theta=2.63952755984961
GenParticle:   2 PDGID:  2212 (P,E)=+1.00e+00,+1.00e+00,-7.00e+03,+7.00e+03 Stat: 3 PV: 0 EV: -2 Attr: 3 flow1=243 phi=5.01674668700084 theta=2.46017876839191
GenParticle:   3 PDGID:     1 (P,E)=+7.50e-01,-1.57e+00,+3.22e+01,+3.22e+01 Stat: 3 PV: -1 EV: -3 Attr: 3 flow1=231 phi=1.2412518609574 theta=2.86402464235875
GenParticle:   4 PDGID:    -2 (P,E)=-3.05e+00,-1.90e+01,-5.46e+01,+5.79e+01 Stat: 3 PV: -2 EV: -3 Attr: 3 flow1=243 phi=4.82692890266268 theta=1.05313334667002
GenParticle:   5 PDGID:    22 (P,E)=-3.81e+00,+1.13e-01,-1.83e+00,+4.23e+00 Stat: 1 PV: -3 EV: 0 Attr: 3 flow1=231 phi=3.48069588687277 theta=0.872654990812324
GenParticle:   6 PDGID:   -24 (P,E)=+1.52e+00,-2.07e+01,-2.06e+01,+8.59e+01 Stat: 3 PV: -3 EV: -4 Attr: 3 flow1=243 phi=3.95131255457656 theta=1.49978707097559
GenParticle:   7 PDGID:     1 (P,E)=-2.44e+00,+2.88e+01,+6.08e+00,+2.96e+01 Stat: 1 PV: -4 EV: 0 Attr: 0
GenParticle:   8 PDGID:    -2 (P,E)=+3.96e+00,-4.95e+01,-2.67e+01,+5.64e+01 Stat: 1 PV: -4 EV: 0 Attr: 0
==25039== Conditional jump or move depends on uninitialised value(s)
==25039==    at 0x4844A73: operator delete(void*) (vg_replace_malloc.c:1052)
==25039==    by 0x4A3EDAC: std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_mutate(unsigned int, unsigned int, char const*, unsigned int) (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x49B0861: ??? (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x49B0D87: std::basic_istream<char, std::char_traits<char> >& std::getline<char, std::char_traits<char>, std::allocator<char> >(std::basic_istream<char, std::char_traits<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, char) (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x112A61: UnknownInlinedFun (basic_string.h:4062)
==25039==    by 0x112A61: COMPARE_ASCII_FILES(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (HepMC3TestUtils.h:27)
==25039==    by 0x10E13C: main (testBoost.cc:150)
==25039== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZdlPv
   fun:_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE9_M_mutateEjjPKcj
   obj:/usr/lib/libstdc++.so.6.0.32
   fun:_ZSt7getlineIcSt11char_traitsIcESaIcEERSt13basic_istreamIT_T0_ES7_RNSt7__cxx1112basic_stringIS4_S5_T1_EES4_
   fun:UnknownInlinedFun
   fun:_Z19COMPARE_ASCII_FILESRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES6_
   fun:main
}
==25039== Conditional jump or move depends on uninitialised value(s)
==25039==    at 0x4846443: operator delete[](void*) (vg_replace_malloc.c:1331)
==25039==    by 0x4A0509D: std::basic_filebuf<char, std::char_traits<char> >::_M_destroy_internal_buffer() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x4A08325: std::basic_filebuf<char, std::char_traits<char> >::close() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x112E6B: UnknownInlinedFun (fstream:256)
==25039==    by 0x112E6B: UnknownInlinedFun (fstream:1126)
==25039==    by 0x112E6B: COMPARE_ASCII_FILES(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (HepMC3TestUtils.h:45)
==25039==    by 0x10E13C: main (testBoost.cc:150)
==25039== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZdaPv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE26_M_destroy_internal_bufferEv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE5closeEv
   fun:UnknownInlinedFun
   fun:UnknownInlinedFun
   fun:_Z19COMPARE_ASCII_FILESRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES6_
   fun:main
}
==25039== Conditional jump or move depends on uninitialised value(s)
==25039==    at 0x4846443: operator delete[](void*) (vg_replace_malloc.c:1331)
==25039==    by 0x4A0509D: std::basic_filebuf<char, std::char_traits<char> >::_M_destroy_internal_buffer() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x4A08325: std::basic_filebuf<char, std::char_traits<char> >::close() (in /usr/lib/libstdc++.so.6.0.32)
==25039==    by 0x112F47: UnknownInlinedFun (fstream:256)
==25039==    by 0x112F47: UnknownInlinedFun (fstream:1126)
==25039==    by 0x112F47: COMPARE_ASCII_FILES(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (HepMC3TestUtils.h:45)
==25039==    by 0x10E13C: main (testBoost.cc:150)
==25039== 
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZdaPv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE26_M_destroy_internal_bufferEv
   fun:_ZNSt13basic_filebufIcSt11char_traitsIcEE5closeEv
   fun:UnknownInlinedFun
   fun:UnknownInlinedFun
   fun:_Z19COMPARE_ASCII_FILESRKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEES6_
   fun:main
}
Run comparison

Comment 2 Mark Wielaard 2023-10-25 10:23:28 UTC

Thanks, this is probably indeed a valgrind issue. delete[] is now handled differently than before:
https://sourceware.org/cgit/valgrind/commit/?id=e534908ce28736a64203dc74b9c54835aafff1e7
delete[] (_ZdaPv) used to be handled by FREE, now DELETE.

I am not sure why this issue shows up on 32bit, but not 64bit.

Comment 3 Mark Wielaard 2023-10-25 21:39:34 UTC

Found it. There are sized and non-sized deletes. We were checking the size also for the non-sized deletes. No idea why this didn't show up on other arches. It does fix the issues on i386 and doesn't introduce regressions on x86_64:

diff --git a/coregrind/m_replacemalloc/vg_replace_malloc.c b/coregrind/m_replacemalloc/vg_replace_malloc.c
index e238a52f3..7859f5f32 100644
--- a/coregrind/m_replacemalloc/vg_replace_malloc.c
+++ b/coregrind/m_replacemalloc/vg_replace_malloc.c
@@ -1027,13 +1027,12 @@ extern int * __error(void) __attribute__((weak));
 
 #define DELETE(soname, fnname, vg_replacement, tag) \
  \
-    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p, SizeT size); \
-    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p, SizeT size)  \
+    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p); \
+    void VG_REPLACE_FUNCTION_EZU(10050,soname,fnname) (void *p)  \
  { \
  struct AlignedAllocInfo aligned_alloc_info = { .mem=p, .alloc_kind=AllocKind##tag }; \
       \
       DO_INIT; \
-      TRIGGER_MEMCHECK_ERROR_IF_UNDEFINED((UWord)size); \
       VERIFY_ALIGNMENT(&aligned_alloc_info); \
       MALLOC_TRACE(#fnname "(%p)\n", p ); \
       if (p == NULL)  \

Comment 4 Mark Wielaard 2023-10-26 10:34:27 UTC

Upstream bug: https://bugs.kde.org/show_bug.cgi?id=476108

Comment 5 Mark Wielaard 2023-10-26 13:00:47 UTC

Should be fixed in valgrind 3.22.0-RC2

Comment 6 Mattias Ellert 2023-10-26 15:10:46 UTC

Thank you for the fast fix. The HepMC3 package now passed all test for the package build in rawhide also for i386.

Comment 7 Florian Weimer 2023-10-26 16:06:59 UTC

(In reply to Mark Wielaard from comment #3)
> Found it. There are sized and non-sized deletes. We were checking the size
> also for the non-sized deletes. No idea why this didn't show up on other
> arches.

The other architectures pass the size in a register, which apparently has a defined value in most cases.