From 65c3d05e18e7c530ef671a956b3325f016659baa Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 11 Apr 2008 22:54:23 +0000 Subject: [PATCH] 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. --- src/backend/parser/parse_type.c | 40 ++++++++++++++++++++----- src/backend/utils/mmgr/aset.c | 53 ++++++++++++++++++++++++++++++++- src/include/pg_config_manual.h | 19 ++++++++---- 3 files changed, 98 insertions(+), 14 deletions(-) diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c index e7fc0e3513..e07879b184 100644 --- a/src/backend/parser/parse_type.c +++ b/src/backend/parser/parse_type.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_type.c,v 1.94 2008/01/01 19:45:51 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_type.c,v 1.95 2008/04/11 22:54:23 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -22,6 +22,7 @@ #include "parser/parse_type.h" #include "utils/array.h" #include "utils/builtins.h" +#include "utils/datum.h" #include "utils/lsyscache.h" #include "utils/syscache.h" @@ -485,13 +486,38 @@ typeTypeRelid(Type typ) Datum stringTypeDatum(Type tp, char *string, int32 atttypmod) { - Oid typinput; - Oid typioparam; + Form_pg_type typform = (Form_pg_type) GETSTRUCT(tp); + Oid typinput = typform->typinput; + Oid typioparam = getTypeIOParam(tp); + Datum result; - typinput = ((Form_pg_type) GETSTRUCT(tp))->typinput; - typioparam = getTypeIOParam(tp); - return OidInputFunctionCall(typinput, string, - typioparam, atttypmod); + result = OidInputFunctionCall(typinput, string, + typioparam, atttypmod); + +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* + * For pass-by-reference data types, repeat the conversion to see if the + * input function leaves any uninitialized bytes in the result. We can + * only detect that reliably if RANDOMIZE_ALLOCATED_MEMORY is enabled, + * so we don't bother testing otherwise. The reason we don't want any + * instability in the input function is that comparison of Const nodes + * relies on bytewise comparison of the datums, so if the input function + * leaves garbage then subexpressions that should be identical may not get + * recognized as such. See pgsql-hackers discussion of 2008-04-04. + */ + if (string && !typform->typbyval) + { + Datum result2; + + result2 = OidInputFunctionCall(typinput, string, + typioparam, atttypmod); + if (!datumIsEqual(result, result2, typform->typbyval, typform->typlen)) + elog(WARNING, "type %s has unstable input conversion for \"%s\"", + NameStr(typform->typname), string); + } +#endif + + return result; } /* given a typeid, return the type's typrelid (associated relation, if any) */ diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index 81f73f963d..a6ac6ab032 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -11,7 +11,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.76 2008/01/01 19:45:55 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/mmgr/aset.c,v 1.77 2008/04/11 22:54:23 tgl Exp $ * * NOTE: * This is a new (Feb. 05, 1999) implementation of the allocation set @@ -282,6 +282,32 @@ AllocSetFreeIndex(Size size) return idx; } +#ifdef RANDOMIZE_ALLOCATED_MEMORY + +/* + * Fill a just-allocated piece of memory with "random" data. It's not really + * very random, just a repeating sequence with a length that's prime. What + * we mainly want out of it is to have a good probability that two palloc's + * of the same number of bytes start out containing different data. + */ +static void +randomize_mem(char *ptr, size_t size) +{ + static int save_ctr = 1; + int ctr; + + ctr = save_ctr; + while (size-- > 0) + { + *ptr++ = ctr; + if (++ctr > 251) + ctr = 1; + } + save_ctr = ctr; +} + +#endif /* RANDOMIZE_ALLOCATED_MEMORY */ + /* * Public routines @@ -552,6 +578,10 @@ AllocSetAlloc(MemoryContext context, Size size) if (size < chunk_size) ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E; #endif +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* fill the allocated space with junk */ + randomize_mem((char *) AllocChunkGetPointer(chunk), size); +#endif /* * Stick the new block underneath the active allocation block, so that @@ -596,6 +626,10 @@ AllocSetAlloc(MemoryContext context, Size size) if (size < chunk->size) ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E; #endif +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* fill the allocated space with junk */ + randomize_mem((char *) AllocChunkGetPointer(chunk), size); +#endif /* isReset must be false already */ Assert(!set->isReset); @@ -752,6 +786,10 @@ AllocSetAlloc(MemoryContext context, Size size) if (size < chunk->size) ((char *) AllocChunkGetPointer(chunk))[size] = 0x7E; #endif +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* fill the allocated space with junk */ + randomize_mem((char *) AllocChunkGetPointer(chunk), size); +#endif set->isReset = false; @@ -864,6 +902,13 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) if (oldsize >= size) { #ifdef MEMORY_CONTEXT_CHECKING +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* We can only fill the extra space if we know the prior request */ + if (size > chunk->requested_size) + randomize_mem((char *) AllocChunkGetPointer(chunk) + chunk->requested_size, + size - chunk->requested_size); +#endif + chunk->requested_size = size; /* set mark to catch clobber of "unused" space */ if (size < oldsize) @@ -921,6 +966,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) chunk->size = chksize; #ifdef MEMORY_CONTEXT_CHECKING +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* We can only fill the extra space if we know the prior request */ + randomize_mem((char *) AllocChunkGetPointer(chunk) + chunk->requested_size, + size - chunk->requested_size); +#endif + chunk->requested_size = size; /* set mark to catch clobber of "unused" space */ if (size < chunk->size) diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 5eead55481..6d7ae4ed57 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -6,7 +6,7 @@ * for developers. If you edit any of these, be sure to do a *full* * rebuild (and an initdb if noted). * - * $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.30 2008/03/27 03:57:34 tgl Exp $ + * $PostgreSQL: pgsql/src/include/pg_config_manual.h,v 1.31 2008/04/11 22:54:23 tgl Exp $ *------------------------------------------------------------------------ */ @@ -214,11 +214,19 @@ *------------------------------------------------------------------------ */ +/* + * Define this to cause palloc()'d memory to be filled with random data, to + * facilitate catching code that depends on the contents of uninitialized + * memory. Right now, this gets defined automatically if --enable-cassert. + */ +#ifdef USE_ASSERT_CHECKING +#define RANDOMIZE_ALLOCATED_MEMORY +#endif + /* * Define this to cause pfree()'d memory to be cleared immediately, to - * facilitate catching bugs that refer to already-freed values. XXX - * Right now, this gets defined automatically if --enable-cassert. In - * the long term it probably doesn't need to be on by default. + * facilitate catching bugs that refer to already-freed values. + * Right now, this gets defined automatically if --enable-cassert. */ #ifdef USE_ASSERT_CHECKING #define CLOBBER_FREED_MEMORY @@ -227,8 +235,7 @@ /* * Define this to check memory allocation errors (scribbling on more * bytes than were allocated). Right now, this gets defined - * automatically if --enable-cassert. In the long term it probably - * doesn't need to be on by default. + * automatically if --enable-cassert. */ #ifdef USE_ASSERT_CHECKING #define MEMORY_CONTEXT_CHECKING -- 2.40.0