On 12/14/18 5:10 AM, Tom Lane wrote: > Tomas Vondra <tomas.von...@2ndquadrant.com> writes: >> On 11/8/18 1:07 PM, Tomas Vondra wrote: >>> Not sure what to do about this - maybe we should wildcard this first >>> frame, to accept all wcsnlen variants. Opinions? > >> I've committed this with the first frame wirldcarded, and backpatched it >> all the way back to 9.4. > > So I just got around to trying to do some valgrind testing for the first > time in a couple of months, and what I find is that this patch completely > breaks valgrind for me: it exits immediately with complaints like > > ==00:00:00:00.222 14821== FATAL: in suppressions file > "/home/postgres/pgsql/src/tools/valgrind.supp" near line 227: > ==00:00:00:00.222 14821== unknown tool suppression type > ==00:00:00:00.222 14821== exiting now. > > After a bit of hair-pulling I found that the line number report is > a lie, and what it's really unhappy about is the "Memcheck:Addr32" > specification on line 236. This is not terribly surprising perhaps > considering that "Addr32" isn't even documented on what should be > the authoritative page: > http://valgrind.org/docs/manual/mc-manual.html#mc-manual.suppfiles >
Hmm :-( FWIW the Addr32 comes directly from valgrind itself, which explicitly suggested to add suppression like this: { <insert_a_suppression_name_here> Memcheck:Addr32 fun:__wcsnlen_avx2 fun:wcsrtombs fun:wcstombs fun:wchar2char fun:str_tolower fun:lower fun:DirectFunctionCall1Coll fun:Generic_Text_IC_like ... } My guess is that documentation is either somewhat obsolete, or is meant more like a general description than an exhaustive and authoritative source of truth regarding suppressions. > This is valgrind 3.8.1 (RHEL6's version), so not bleeding edge exactly, > but we have very little evidence about how far back the committed patch > works. I'm inclined to think we can't use this solution. > Fedora 28 has 3.14.0, so this seems to be due to some improvements since 3.8.1. I'm not sure how to deal with this - surely we don't want to just revert the change because that would start generating noise again. For a moment I thought we might wildcard the error type somehow, so that it would accept Memcheck:* but AFAICs that is not supported. regards -- Tomas Vondra http://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services