Re: OSX doesn't accept identical source/target for strcpy() anymore - Mailing list pgsql-hackers

From Tom Lane
Subject Re: OSX doesn't accept identical source/target for strcpy() anymore
Date
Msg-id 10044.1382990556@sss.pgh.pa.us
Whole thread Raw
In response to Re: OSX doesn't accept identical source/target for strcpy() anymore  (Andres Freund <andres@2ndquadrant.com>)
Responses Re: OSX doesn't accept identical source/target for strcpy() anymore  (Andres Freund <andres@2ndquadrant.com>)
Re: OSX doesn't accept identical source/target for strcpy() anymore  (Noah Misch <noah@leadboat.com>)
List pgsql-hackers
Andres Freund <andres@2ndquadrant.com> writes:
> It'd be relatively easy to add support for make check (not installcheck)
> wrapping postgres in valgrind via pg_regress, but I am not sure that's
> the best way to go.

> I think defining an additional CFLAG (USE_VALGRIND) shouldn't be a
> problem?

CFLAGS doesn't seem to have anything to do with this.  I'd be more
inclined to add a "--wrapper=prog" argument to pg_regress and invoke
it with something like --wrapper="valgrind --trace-children=yes".

The larger problem though is what you'd do with the output.  There's
enough false-positive noise from valgrind that I can't see having
the buildfarm run just fail if there are any messages.  What to do
instead isn't very clear.

Anyway, to get back to the original problem, I've confirmed that
valgrind complains about the particular case at hand:

==9497== Source and destination overlap in strncpy(0xe1d5a3d, 0xe1d5a3d, 64)
==9497==    at 0x4A081EF: strncpy (mc_replace_strmem.c:476)
==9497==    by 0x6D2398: namestrcpy (name.c:221)
==9497==    by 0x45F478: TupleDescInitEntry (tupdesc.c:507)
==9497==    by 0x756FE1: internal_get_result_type (funcapi.c:557)
==9497==    by 0x75727C: get_expr_result_type (funcapi.c:235)
==9497==    by 0x534146: expandRecordVariable (parse_target.c:1524)
==9497==    by 0x52C267: ParseFuncOrColumn (parse_func.c:1494)
==9497==    by 0x5285CF: transformExprRecurse (parse_expr.c:463)
==9497==    by 0x528DB1: transformExpr (parse_expr.c:117)
==9497==    by 0x5350C5: transformTargetEntry (parse_target.c:94)
==9497==    by 0x535AD4: transformTargetList (parse_target.c:167)
==9497==    by 0x505FFF: transformStmt (analyze.c:929)

It seems to me the most reasonable fix for this is to make
TupleDescInitEntry notice that the passed "attributeName" points
at the tupdesc's name field and not call namestrcpy if so.
This would go with an API clarification stating that callers can
pass that if they want the name field to be unchanged.  (Or we
could invent some other way to signal that, but note that NULL
is already in use for a different purpose.)

Another possibly-useful approach would be to hack namestrcpy itself
to handle name == str specially.  However, that's legitimizing a
usage that's really a type cheat, so I don't like it as much, even
though it might fix more cases besides this one.

Any other thoughts about it?
        regards, tom lane



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: better atomics
Next
From: Heikki Linnakangas
Date:
Subject: Re: better atomics