valgrind error in tsvectorin

Gregory Stark <stark@enterprisedb.com> writes:

Was just running the regression tests under valgrind and aside from the usual
false positives caused by structure padding I noticed this:

==19366== Source and destination overlap in memcpy(0x4BB7FC0, 0x4BB7FC0, 12)
==19366== at 0x4026B12: memcpy (mc_replace_strmem.c:402)
==19366== by 0x8389750: uniqueentry (tsvector.c:128)
==19366== by 0x8389C63: tsvectorin (tsvector.c:265)
==19366== by 0x83B1888: InputFunctionCall (fmgr.c:1878)
==19366== by 0x83B1B46: OidInputFunctionCall (fmgr.c:2009)
==19366== by 0x8171651: stringTypeDatum (parse_type.c:497)
==19366== by 0x8171CAC: coerce_type (parse_coerce.c:239)
==19366== by 0x8171A72: coerce_to_target_type (parse_coerce.c:86)
==19366== by 0x8166DB5: transformTypeCast (parse_expr.c:2016)
==19366== by 0x8162FA8: transformExpr (parse_expr.c:181)
==19366== by 0x8174990: transformTargetEntry (parse_target.c:75)
==19366== by 0x8174B01: transformTargetList (parse_target.c:145)

After a quick glance at the code I suspect res and ptr end up pointing to the
same object, perhaps the loop condition has a fencepost error. But I don't
really understand what it's trying to do at all.

Yeah, it looks like the memcpy is sometimes unnecessary because res and
ptr point to the same place. It might be worth cleaning up just to
avoid the valgrind warning, but I doubt it would save any noticeable
number of cycles.

regards, tom lane

Yeah, it looks like the memcpy is sometimes unnecessary because res and
ptr point to the same place.  It might be worth cleaning up just to
avoid the valgrind warning, but I doubt it would save any noticeable
number of cycles.

I assume valgrind is warning about it because memcpy's behaviour is
undefined if the blocks overlap. I'm having trouble imagining an
implementation that would fail if they're precisely the same pointer
though.

--
greg

Greg Stark <stark@enterprisedb.com> writes:

Yeah, it looks like the memcpy is sometimes unnecessary because res and
ptr point to the same place. �It might be worth cleaning up just to
avoid the valgrind warning, but I doubt it would save any noticeable
number of cycles.

I assume valgrind is warning about it because memcpy's behaviour is
undefined if the blocks overlap. I'm having trouble imagining an
implementation that would fail if they're precisely the same pointer
though.

Me either. A counterexample is that compilers typically implement
structure assignment via memcpy, and the behavior of "*d = *s" is
not undefined merely because d and s point to the same place.

In this particular example it looks like res and ptr might be the same
often enough that adding an "if (res != ptr)" test would save enough
cycles to be worth its cost ... but it's pretty marginal.

regards, tom lane