Suppressing Valgrind errors: a small note

[Alexander Potapenko]

Hi all,

If you do not write Valgrind suppressions, you can stop reading now.

I see that 30+ people have committed to
src/tools/valgrind/memcheck/suppressions.txt -- and that's great!
I just wanted to give you some dont's on writing nicer suppressions.
The do's are covered by Valgrind documentation:
http://valgrind.org/docs/manual/manual-core.html#manual-core.suppress

1. Tail does not matter.
The suppression matches a stack iff it matches its top. That is,
removing the last line of the suppression still leaves it working.
So there's no reason to keep the frame-level wildcard ("...") as the
last suppression line.
It also makes little sense to put an "obj:*" wildcard as the last
line. This should mean "some unknown symbol in any object file", which
may match practically everything.

2. Anonymous namespaces
Valgrind suppressions contain mangled function names, which may
slightly differ for different systems and compilers. The main
difference is probably the anonymous namespace mangling.

Consider the following function name:
(anonymous namespace)::AddKeyedCookieToList(std::basic_string<char,
std::char_traits<char>, std::allocator<char> > const&,
std::basic_string<char, std::char_traits<char>, std::allocator<char> >
const&, base::Time const&,
std::vector<std::pair<std::basic_string<char, std::char_traits<char>,
std::allocator<char> >, net::CookieMonster::CanonicalCookie*>,
std::allocator<std::pair<std::basic_string<char,
std::char_traits<char>, std::allocator<char> >,
net::CookieMonster::CanonicalCookie*> > >*)

A Mac Valgrind bot generates the following symbol for it:
fun:_ZN12_GLOBAL__N_1L20AddKeyedCookieToListERKSsS1_RKN4base4TimeEPSt6vectorISt4pairISsPN3net13CookieMonster15CanonicalCookieEESaISC_EE

, whereas the Linux bot prints the following:
fun:_ZN64_GLOBAL__N_net_base_cookie_monster_unittest.cc_00000000_C9F6D94420AddKeyedCookieToListERKSsS1_RKN4base4TimeEPSt6vectorISt4pairISsPN3net13CookieMonster15CanonicalCookieEESaISC_EE

Note that the anonymous namespace is replaced with some unique string
different for this systems. On Linux it depends on the file path
(_net_base_cookie_monster_unittest.cc_) and is salted (_C9F6D944), so
keeping this string in a suppression may lead to new reports.

I've even seen suppression lines containing nothing but the anonymous
namespace names, like
"fun:_ZN3gfx51_GLOBAL__N_gfx_codec_png_codec.cc_00000000_*". These
make almost no sense.

Please also spend a couple of minutes reading
http://en.wikipedia.org/wiki/Name_mangling and playing with c++filt to
get the idea of how the function names are mangled.

3. Don't forget to remove them!
Once a leak is found, the current sheriff opens a bug and writes a
suppression that prevents the bots from reporting the same bug again.
Once you fix a leak, consider updating the bug and removing all the
corresponding suppressions. This will help to notice a regression, and
it's just easier to maintain a small list of suppressions (that's why
the memory sheriffs love people that fix Valgrind bugs!)

Thanks for your attention,
Alex

[Jonathan Dixon]

This is great info, definitely would have helped me when I needed to edit the suppressions file. Would it be OK to add this to http://dev.chromium.org/developers/how-tos/using-valgrind ?

May also be worth having a link to that page at the top of each of the suppressions.txt files.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Jeremy Orlow]

Better yet, is it possible to have the script that generates the suppression syntax at the end of the run do these transforms or at lest suggest the user read that page?

[Timur Iskhodzhanov]

Should be easy

Also, I think these checks should be in the tools/valgrind/memcheck/PRESUBMIT.py

I've already asked Alexander to add these.