Re: OSX doesn't accept identical source/target for strcpy() anymore

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Matthias Schmitt <freak002(at)mmp(dot)lu>
Subject: Re: OSX doesn't accept identical source/target for strcpy() anymore
Date: 2013-10-28 20:02:36
Message-ID: 10044.1382990556@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Peter Geoghegan 2013-10-28 20:11:19 Re: OSX doesn't accept identical source/target for strcpy() anymore
Previous Message Andres Freund 2013-10-28 19:46:09 Re: OSX doesn't accept identical source/target for strcpy() anymore

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2013-10-28 20:03:41 Re: better atomics
Previous Message Robert Haas 2013-10-28 19:58:10 Re: better atomics