Skip to content

Commit 65c3d05

Browse files
tglsfdctglsfdc
committed
Add some debug support code to try to catch future mistakes in the area of
input functions that include garbage bytes in their results. Provide a compile-time option RANDOMIZE_ALLOCATED_MEMORY to make palloc fill returned blocks with variable contents. This option also makes the parser perform conversions of literal constants twice and compare the results, emitting a WARNING if they don't match. (This is the code I used to catch the input function bugs fixed in the previous commit.) For the moment, I've set it to be activated automatically by --enable-cassert.
1 parent c846f7c commit 65c3d05

File tree

3 files changed

+98
-14
lines changed

3 files changed

+98
-14
lines changed

‎src/backend/parser/parse_type.c

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/parser/parse_type.c,v 1.94 2008/01/01 19:45:51 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/parser/parse_type.c,v 1.95 2008/04/11 22:54:23 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -22,6 +22,7 @@
2222
#include "parser/parse_type.h"
2323
#include "utils/array.h"
2424
#include "utils/builtins.h"
25+
#include "utils/datum.h"
2526
#include "utils/lsyscache.h"
2627
#include "utils/syscache.h"
2728

@@ -485,13 +486,38 @@ typeTypeRelid(Type typ)
485486
Datum
486487
stringTypeDatum(Type tp, char *string, int32 atttypmod)
487488
{
488-
Oid typinput;
489-
Oid typioparam;
489+
Form_pg_type typform = (Form_pg_type) GETSTRUCT(tp);
490+
Oid typinput = typform->typinput;
491+
Oid typioparam = getTypeIOParam(tp);
492+
Datum result;
490493

491-
typinput = ((Form_pg_type) GETSTRUCT(tp))->typinput;
492-
typioparam = getTypeIOParam(tp);
493-
return OidInputFunctionCall(typinput, string,
494-
typioparam, atttypmod);
494+
result = OidInputFunctionCall(typinput, string,
495+
typioparam, atttypmod);
496+
497+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
498+
/*
499+
* For pass-by-reference data types, repeat the conversion to see if the
500+
* input function leaves any uninitialized bytes in the result. We can
501+
* only detect that reliably if RANDOMIZE_ALLOCATED_MEMORY is enabled,
502+
* so we don't bother testing otherwise. The reason we don't want any
503+
* instability in the input function is that comparison of Const nodes
504+
* relies on bytewise comparison of the datums, so if the input function
505+
* leaves garbage then subexpressions that should be identical may not get
506+
* recognized as such. See pgsql-hackers discussion of 2008-04-04.
507+
*/
508+
if (string && !typform->typbyval)
509+
{
510+
Datum result2;
511+
512+
result2 = OidInputFunctionCall(typinput, string,
513+
typioparam, atttypmod);
514+
if (!datumIsEqual(result, result2, typform->typbyval, typform->typlen))
515+
elog(WARNING, "type %s has unstable input conversion for \"%s\"",
516+
NameStr(typform->typname), string);
517+
}
518+
#endif
519+
520+
return result;
495521
}
496522

497523
/* given a typeid, return the type's typrelid (associated relation, if any) */

‎src/backend/utils/mmgr/aset.c

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1994, Regents of the University of California
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.76 2008/01/01 19:45:55 momjian Exp $
14+
* $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.77 2008/04/11 22:54:23 tgl Exp $
1515
*
1616
* NOTE:
1717
* This is a new (Feb. 05, 1999) implementation of the allocation set
@@ -282,6 +282,32 @@ AllocSetFreeIndex(Size size)
282282
return idx;
283283
}
284284

285+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
286+
287+
/*
288+
* Fill a just-allocated piece of memory with "random" data. It's not really
289+
* very random, just a repeating sequence with a length that's prime. What
290+
* we mainly want out of it is to have a good probability that two palloc's
291+
* of the same number of bytes start out containing different data.
292+
*/
293+
static void
294+
randomize_mem(char *ptr, size_t size)
295+
{
296+
static int save_ctr = 1;
297+
int ctr;
298+
299+
ctr = save_ctr;
300+
while (size-- > 0)
301+
{
302+
*ptr++ = ctr;
303+
if (++ctr > 251)
304+
ctr = 1;
305+
}
306+
save_ctr = ctr;
307+
}
308+
309+
#endif /* RANDOMIZE_ALLOCATED_MEMORY */
310+
285311

286312
/*
287313
* Public routines
@@ -552,6 +578,10 @@ AllocSetAlloc(MemoryContext context, Size size)
552578
if (size < chunk_size)
553579
((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
554580
#endif
581+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
582+
/* fill the allocated space with junk */
583+
randomize_mem((char *) AllocChunkGetPointer(chunk), size);
584+
#endif
555585

556586
/*
557587
* Stick the new block underneath the active allocation block, so that
@@ -596,6 +626,10 @@ AllocSetAlloc(MemoryContext context, Size size)
596626
if (size < chunk->size)
597627
((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
598628
#endif
629+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
630+
/* fill the allocated space with junk */
631+
randomize_mem((char *) AllocChunkGetPointer(chunk), size);
632+
#endif
599633

600634
/* isReset must be false already */
601635
Assert(!set->isReset);
@@ -752,6 +786,10 @@ AllocSetAlloc(MemoryContext context, Size size)
752786
if (size < chunk->size)
753787
((char *) AllocChunkGetPointer(chunk))[size] = 0x7E;
754788
#endif
789+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
790+
/* fill the allocated space with junk */
791+
randomize_mem((char *) AllocChunkGetPointer(chunk), size);
792+
#endif
755793

756794
set->isReset = false;
757795

@@ -864,6 +902,13 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
864902
if (oldsize >= size)
865903
{
866904
#ifdef MEMORY_CONTEXT_CHECKING
905+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
906+
/* We can only fill the extra space if we know the prior request */
907+
if (size > chunk->requested_size)
908+
randomize_mem((char *) AllocChunkGetPointer(chunk) + chunk->requested_size,
909+
size - chunk->requested_size);
910+
#endif
911+
867912
chunk->requested_size = size;
868913
/* set mark to catch clobber of "unused" space */
869914
if (size < oldsize)
@@ -921,6 +966,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
921966
chunk->size = chksize;
922967

923968
#ifdef MEMORY_CONTEXT_CHECKING
969+
#ifdef RANDOMIZE_ALLOCATED_MEMORY
970+
/* We can only fill the extra space if we know the prior request */
971+
randomize_mem((char *) AllocChunkGetPointer(chunk) + chunk->requested_size,
972+
size - chunk->requested_size);
973+
#endif
974+
924975
chunk->requested_size = size;
925976
/* set mark to catch clobber of "unused" space */
926977
if (size < chunk->size)

‎src/include/pg_config_manual.h

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* for developers. If you edit any of these, be sure to do a *full*
77
* rebuild (and an initdb if noted).
88
*
9-
* $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.30 2008/03/27 03:57:34 tgl Exp $
9+
* $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.31 2008/04/11 22:54:23 tgl Exp $
1010
*------------------------------------------------------------------------
1111
*/
1212

@@ -214,11 +214,19 @@
214214
*------------------------------------------------------------------------
215215
*/
216216

217+
/*
218+
* Define this to cause palloc()'d memory to be filled with random data, to
219+
* facilitate catching code that depends on the contents of uninitialized
220+
* memory. Right now, this gets defined automatically if --enable-cassert.
221+
*/
222+
#ifdef USE_ASSERT_CHECKING
223+
#define RANDOMIZE_ALLOCATED_MEMORY
224+
#endif
225+
217226
/*
218227
* Define this to cause pfree()'d memory to be cleared immediately, to
219-
* facilitate catching bugs that refer to already-freed values. XXX
220-
* Right now, this gets defined automatically if --enable-cassert. In
221-
* the long term it probably doesn't need to be on by default.
228+
* facilitate catching bugs that refer to already-freed values.
229+
* Right now, this gets defined automatically if --enable-cassert.
222230
*/
223231
#ifdef USE_ASSERT_CHECKING
224232
#define CLOBBER_FREED_MEMORY
@@ -227,8 +235,7 @@
227235
/*
228236
* Define this to check memory allocation errors (scribbling on more
229237
* bytes than were allocated). Right now, this gets defined
230-
* automatically if --enable-cassert. In the long term it probably
231-
* doesn't need to be on by default.
238+
* automatically if --enable-cassert.
232239
*/
233240
#ifdef USE_ASSERT_CHECKING
234241
#define MEMORY_CONTEXT_CHECKING

0 commit comments

Comments
 (0)