Bug 2246032 - Likely bogus errors from valgrind 3.22.0-0.1.RC1.fc40.i686
Summary: Likely bogus errors from valgrind 3.22.0-0.1.RC1.fc40.i686
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: valgrind
Version: rawhide
Hardware: i686
OS: Linux
unspecified
high
Target Milestone: ---
Assignee: Mark Wielaard
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 2245854
TreeView+ depends on / blocked
 
Reported: 2023-10-25 03:38 UTC by Mattias Ellert
Modified: 2023-10-26 16:06 UTC (History)
7 users (show)

Fixed In Version: valgrind-3.22.0-0.1.RC2.fc40
Clone Of:
Environment:
Last Closed: 2023-10-26 13:00:47 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
KDE Software Compilation 476108 0 NOR UNCONFIRMED vg_replace_malloc DELETE checks size 2023-10-26 10:34:27 UTC

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.


Note You need to log in before you can comment on or make changes to this bug.