Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9a9473f

Browse files
committed
Prevent using strncpy with src == dest in TupleDescInitEntry.
The C and POSIX standards state that strncpy's behavior is undefined when source and destination areas overlap. While it remains dubious whether any implementations really misbehave when the pointers are exactly equal, some platforms are now starting to force the issue by complaining when an undefined call occurs. (In particular OS X 10.9 has been seen to dump core here, though the exact set of circumstances needed to trigger that remain elusive. Similar behavior can be expected to be optional on Linux and other platforms in the near future.) So tweak the code to explicitly do nothing when nothing need be done. Back-patch to all active branches. In HEAD, this also lets us get rid of an exception in valgrind.supp. Per discussion of a report from Matthias Schmitt.
1 parent d2aecae commit 9a9473f

File tree

2 files changed

+10
-20
lines changed

2 files changed

+10
-20
lines changed

src/backend/access/common/tupdesc.c

+10-4
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,12 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
468468
* This function initializes a single attribute structure in
469469
* a previously allocated tuple descriptor.
470470
*
471+
* If attributeName is NULL, the attname field is set to an empty string
472+
* (this is for cases where we don't know or need a name for the field).
473+
* Also, some callers use this function to change the datatype-related fields
474+
* in an existing tupdesc; they pass attributeName = NameStr(att->attname)
475+
* to indicate that the attname field shouldn't be modified.
476+
*
471477
* Note that attcollation is set to the default for the specified datatype.
472478
* If a nondefault collation is needed, insert it afterwards using
473479
* TupleDescInitEntryCollation.
@@ -501,12 +507,12 @@ TupleDescInitEntry(TupleDesc desc,
501507
/*
502508
* Note: attributeName can be NULL, because the planner doesn't always
503509
* fill in valid resname values in targetlists, particularly for resjunk
504-
* attributes.
510+
* attributes. Also, do nothing if caller wants to re-use the old attname.
505511
*/
506-
if (attributeName != NULL)
507-
namestrcpy(&(att->attname), attributeName);
508-
else
512+
if (attributeName == NULL)
509513
MemSet(NameStr(att->attname), 0, NAMEDATALEN);
514+
else if (attributeName != NameStr(att->attname))
515+
namestrcpy(&(att->attname), attributeName);
510516

511517
att->attstattarget = -1;
512518
att->attcacheoff = -1;

src/tools/valgrind.supp

-16
Original file line numberDiff line numberDiff line change
@@ -64,22 +64,6 @@
6464
}
6565

6666

67-
# resolve_polymorphic_tupdesc(), a subroutine of internal_get_result_type(),
68-
# can instigate a memcpy() call where the two pointer arguments are exactly
69-
# equal. The behavior thereof is formally undefined, but implementations
70-
# where it's anything other than a no-op are thought unlikely.
71-
{
72-
noopmemcpy_internal_get_result_type
73-
Memcheck:Overlap
74-
75-
fun:*strncpy*
76-
fun:namestrcpy
77-
fun:TupleDescInitEntry
78-
...
79-
fun:internal_get_result_type
80-
}
81-
82-
8367
# gcc on ppc64 can generate a four-byte read to fetch the final "char" fields
8468
# of a FormData_pg_cast. This is valid compiler behavior, because a proper
8569
# FormData_pg_cast has trailing padding. Tuples we treat as structures omit

0 commit comments

Comments
 (0)