On 12/14/18 5:10 AM, Tom Lane wrote:
> Tomas Vondra <tomas.vondra@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