Summary: | memcheck/tests/leak_cpp_interior fails with gcc11 | ||
---|---|---|---|
Product: | [Developer tools] valgrind | Reporter: | Khem Raj <raj.khem> |
Component: | memcheck | Assignee: | Julian Seward <jseward> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | mark, pjfloyd, sam |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Other | ||
OS: | Linux | ||
See Also: | https://bugs.kde.org/show_bug.cgi?id=403802 | ||
Latest Commit: | Version Fixed In: |
Description
Khem Raj
2021-04-14 17:39:54 UTC
Just a few comments on this. I'm not too sure how the std::string heuristic works these days. Here is the code from mc_leakcheck.c if (HiS(LchStdString, heur_set)) { // Detects inner pointers to Std::String for layout being // length capacity refcount char_array[] \0 // where ptr points to the beginning of the char_array. // Note: we check definedness for length and capacity but // not for refcount, as refcount size might be smaller than // a SizeT, giving a uninitialised hole in the first 3 SizeT. if ( ptr == ch->data + 3 * sizeof(SizeT) && MC_(is_valid_aligned_word)(ch->data + sizeof(SizeT))) { const SizeT capacity = *((SizeT*)(ch->data + sizeof(SizeT))); if (3 * sizeof(SizeT) + capacity + 1 == ch->szB && MC_(is_valid_aligned_word)(ch->data)) { const SizeT length = *((SizeT*)ch->data); if (length <= capacity) { // ??? could check there is no null byte from ptr to ptr+length-1 // ??? and that there is a null byte at ptr+length. // ??? // ??? could check that ch->allockind is MC_AllocNew ??? // ??? probably not a good idea, as I guess stdstring // ??? allocator can be done via custom allocator // ??? or even a call to malloc ???? VG_(set_fault_catcher) (prev_catcher); return LchStdString; } } } } Modern C++ does not use an std:string layout that looks like // length capacity refcount char_array[] \0 In fact, since C++11, refounts in strings has been forbidden. GCC was a bit slow implementing this, which was done for GCC 5. With GCC 6 or 7 (can't remember which) the default C++ compilation changed to C++14. This is fairly important as GCC maintained backwards compatibility using C++ namespaces, So it is possible to use either the old or the new layout [https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html]. GCC 11 might have switched to defaulting to C++17, which could change things again (but I'm not aware of any such changes). So what are the newer layouts? With GCC / libstdc++ it is like this // _Alloc_hider just contains // pointer _M_p; // The actual data. _Alloc_hider _M_dataplus; size_type _M_string_length; enum { _S_local_capacity = 15 / sizeof(_CharT) }; union { _CharT _M_local_buf[_S_local_capacity + 1]; size_type _M_allocated_capacity; }; With clang/libc++, see here https://eu90h.github.io/cpp-strings.html Roughly speaking, the std:string now uses something called the "short string optimization" which avoids any struct __long { size_type __cap_; size_type __size_; pointer __data_; }; struct __short { union { unsigned char __size_; value_type __lx; }; value_type __data_[__min_cap]; }; where __min_cap is 23. Either way, the actual data pointer is not where the current code assumes that it is. I don't know of a way to tell which ABI is being used. Hmm. The stuff that I wrote for C++11 was mostly right, but it is wrong for C++03 and earlier. libstdc++ prior to C++11 jus has a pointer in std::string. This points to memory containing the layout len/cap/refcount/sting data. I need to check to see how this testcase is compiled and do some debugging. I can reproduce this problem with a much older g++, 5.3.0. Running the testcase in place gives the following information ==13544== 72,704 bytes in 1 blocks are still reachable in loss record 10 of 10 ==13544== at 0x402D80A: malloc (vg_replace_malloc.c:380) ==13544== by 0x40D14BF: pool (eh_alloc.cc:117) ==13544== by 0x40D14BF: __static_initialization_and_destruction_0 (eh_alloc.cc:244) ==13544== by 0x40D14BF: _GLOBAL__sub_I_eh_alloc.cc (eh_alloc.cc:307) ==13544== by 0x400F902: _dl_init (in /usr/lib64/ld-2.17.so) ==13544== by 0x4001159: ??? (in /usr/lib64/ld-2.17.so) I would expect the freeres function to clean this up. Need to do more debugging to see why it is not being called (or if it is being called, why it isn't freeing this memory). The doesn't reproduce with g++11 (FreeBSD Ports Collection) 11.2.0 We should be looking at leak_cpp_interior.stderr.diff-64bit rather than leak_cpp_interior.stderr.diff (same for 403802) With g++ 9.2 on RHEL 7 I get a similar failure, but without the still reachable that I get with g++ 5.3. Running the test standalone as follows ../../vg-in-place --leak-check-heuristics=multipleinheritance,stdstring,newarray,length64 --suppressions=libstdc++.supp --leak-check=full --show-reachable=yes ./leak_cpp_interior it looks like the problem is due to mismatches of the suppressions in libstdc++.supp and those produced by different GCC/libstc++ versions. The following diff may be a fix diff --git a/memcheck/tests/libstdc++.supp b/memcheck/tests/libstdc++.supp index 520e6613a..475e2d9fa 100644 --- a/memcheck/tests/libstdc++.supp +++ b/memcheck/tests/libstdc++.supp @@ -81,10 +81,7 @@ fun:pool fun:__static_initialization_and_destruction_0 fun:_GLOBAL__sub_I_eh_alloc.cc - fun:call_init.part.0 - fun:call_init - fun:_dl_init - obj:*lib*/ld-2.*.so + obj:*lib*/libstdc++.so* } { malloc-leaks-cxx-stl-string-classes-debug2 I think that an even broader suppression for the stdio buffer should be OK, as follows diff --git a/memcheck/tests/libstdc++.supp b/memcheck/tests/libstdc++.supp index 520e6613a..3cd2e628d 100644 --- a/memcheck/tests/libstdc++.supp +++ b/memcheck/tests/libstdc++.supp @@ -81,10 +81,6 @@ fun:pool fun:__static_initialization_and_destruction_0 fun:_GLOBAL__sub_I_eh_alloc.cc - fun:call_init.part.0 - fun:call_init - fun:_dl_init - obj:*lib*/ld-2.*.so } { malloc-leaks-cxx-stl-string-classes-debug2 I made the suppression even broader. It's gone from failing to passing for me on Fedora 34 / GCC 11.2.1 In the nightly tests I see that this is failing for several platforms still (s390x Fedora 34, amd64 Fedora 34 to 36). It could be that the difference is down to whether debuginfo is installed/available for libstfc++. I do see a difference because of debuginfo, and pushed a second change for that. If the nightlies are clean I'll close this (and 403802) In only see Philippe W's GCC 4.8.5 / Contos 7 test failing now, and I think that is due to some other issue. So this can be closed. |