Cooperate with the Valgrind instrumentation framework.

Valgrind "client requests" in aset.c and mcxt.c teach Valgrind and its
Memcheck tool about the PostgreSQL allocator.  This makes Valgrind
roughly as sensitive to memory errors involving palloc chunks as it is
to memory errors involving malloc chunks.  Further client requests in
PageAddItem() and printtup() verify that all bits being added to a
buffer page or furnished to an output function are predictably-defined.
Those tests catch failures of C-language functions to fully initialize
the bits of a Datum, which in turn stymie optimizations that rely on
_equalConst().  Define the USE_VALGRIND symbol in pg_config_manual.h to
enable these additions.  An included "suppression file" silences nominal
errors we don't plan to fix.

Reviewed in earlier versions by Peter Geoghegan and Korry Douglas.

src/backend/access/common/printtup.c

@@ -20,6 +20,7 @@
 #include "libpq/pqformat.h"
 #include "tcop/pquery.h"
 #include "utils/lsyscache.h"
+#include "utils/memdebug.h"
 
 
 static void printtup_startup(DestReceiver *self, int operation,
@@ -324,9 +325,26 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
 		/*
 		 * If we have a toasted datum, forcibly detoast it here to avoid
 		 * memory leakage inside the type's output routine.
+		 *
+		 * Here we catch undefined bytes in tuples that are returned to the
+		 * client without hitting disk; see comments at the related check in
+		 * PageAddItem().  Whether to test before or after detoast is somewhat
+		 * arbitrary, as is whether to test external/compressed data at all.
+		 * Undefined bytes in the pre-toast datum will have triggered Valgrind
+		 * errors in the compressor or toaster; any error detected here for
+		 * such datums would indicate an (unlikely) bug in a type-independent
+		 * facility.  Therefore, this test is most useful for uncompressed,
+		 * non-external datums.
+		 *
+		 * We don't presently bother checking non-varlena datums for undefined
+		 * data.  PageAddItem() does check them.
 		 */
 		if (thisState->typisvarlena)
+		{
+			VALGRIND_CHECK_MEM_IS_DEFINED(origattr, VARSIZE_ANY(origattr));
+
 			attr = PointerGetDatum(PG_DETOAST_DATUM(origattr));
+		}
 		else
 			attr = origattr;
 

src/backend/storage/page/bufpage.c

@@ -17,6 +17,7 @@
 #include "access/htup_details.h"
 #include "access/xlog.h"
 #include "storage/checksum.h"
+#include "utils/memdebug.h"
 #include "utils/memutils.h"
 
 
@@ -297,6 +298,20 @@ PageAddItem(Page page,
 	/* set the item pointer */
 	ItemIdSetNormal(itemId, upper, size);
 
+	/*
+	 * Items normally contain no uninitialized bytes.  Core bufpage consumers
+	 * conform, but this is not a necessary coding rule; a new index AM could
+	 * opt to depart from it.  However, data type input functions and other
+	 * C-language functions that synthesize datums should initialize all
+	 * bytes; datumIsEqual() relies on this.  Testing here, along with the
+	 * similar check in printtup(), helps to catch such mistakes.
+	 *
+	 * Values of the "name" type retrieved via index-only scans may contain
+	 * uninitialized bytes; see comment in btrescan().  Valgrind will report
+	 * this as an error, but it is safe to ignore.
+	 */
+	VALGRIND_CHECK_MEM_IS_DEFINED(item, size);
+
 	/* copy the item's data onto the page */
 	memcpy((char *) page + upper, item, size);
 

src/backend/tcop/postgres.c

@@ -69,6 +69,7 @@
 #include "tcop/tcopprot.h"
 #include "tcop/utility.h"
 #include "utils/lsyscache.h"
+#include "utils/memdebug.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/snapmgr.h"
@@ -846,6 +847,10 @@ exec_simple_query(const char *query_string)
 
 	TRACE_POSTGRESQL_QUERY_START(query_string);
 
+#ifdef USE_VALGRIND
+	VALGRIND_PRINTF("statement: %s\n", query_string);
+#endif
+
 	/*
 	 * We use save_log_statement_stats so ShowUsage doesn't report incorrect
 	 * results because ResetUsage wasn't called.

src/backend/utils/mmgr/mcxt.c

@@ -24,6 +24,7 @@
 
 #include "postgres.h"
 
+#include "utils/memdebug.h"
 #include "utils/memutils.h"
 
 
@@ -135,6 +136,8 @@ MemoryContextReset(MemoryContext context)
 	{
 		(*context->methods->reset) (context);
 		context->isReset = true;
+		VALGRIND_DESTROY_MEMPOOL(context);
+		VALGRIND_CREATE_MEMPOOL(context, 0, false);
 	}
 }
 
@@ -184,6 +187,7 @@ MemoryContextDelete(MemoryContext context)
 	MemoryContextSetParent(context, NULL);
 
 	(*context->methods->delete_context) (context);
+	VALGRIND_DESTROY_MEMPOOL(context);
 	pfree(context);
 }
 
@@ -555,6 +559,8 @@ MemoryContextCreate(NodeTag tag, Size size,
 		parent->firstchild = node;
 	}
 
+	VALGRIND_CREATE_MEMPOOL(node, 0, false);
+
 	/* Return to type-specific creation routine to finish up */
 	return node;
 }
@@ -580,6 +586,7 @@ MemoryContextAlloc(MemoryContext context, Size size)
 	context->isReset = false;
 
 	ret = (*context->methods->alloc) (context, size);
+	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
 	return ret;
 }
@@ -605,6 +612,7 @@ MemoryContextAllocZero(MemoryContext context, Size size)
 	context->isReset = false;
 
 	ret = (*context->methods->alloc) (context, size);
+	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
 	MemSetAligned(ret, 0, size);
 
@@ -632,6 +640,7 @@ MemoryContextAllocZeroAligned(MemoryContext context, Size size)
 	context->isReset = false;
 
 	ret = (*context->methods->alloc) (context, size);
+	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
 	MemSetLoop(ret, 0, size);
 
@@ -653,6 +662,7 @@ palloc(Size size)
 	CurrentMemoryContext->isReset = false;
 
 	ret = (*CurrentMemoryContext->methods->alloc) (CurrentMemoryContext, size);
+	VALGRIND_MEMPOOL_ALLOC(CurrentMemoryContext, ret, size);
 
 	return ret;
 }
@@ -672,6 +682,7 @@ palloc0(Size size)
 	CurrentMemoryContext->isReset = false;
 
 	ret = (*CurrentMemoryContext->methods->alloc) (CurrentMemoryContext, size);
+	VALGRIND_MEMPOOL_ALLOC(CurrentMemoryContext, ret, size);
 
 	MemSetAligned(ret, 0, size);
 
@@ -704,6 +715,7 @@ pfree(void *pointer)
 	AssertArg(MemoryContextIsValid(context));
 
 	(*context->methods->free_p) (context, pointer);
+	VALGRIND_MEMPOOL_FREE(context, pointer);
 }
 
 /*
@@ -740,6 +752,7 @@ repalloc(void *pointer, Size size)
 	Assert(!context->isReset);
 
 	ret = (*context->methods->realloc) (context, pointer, size);
+	VALGRIND_MEMPOOL_CHANGE(context, pointer, ret, size);
 
 	return ret;
 }

src/include/pg_config_manual.h

@@ -206,6 +206,21 @@
  *------------------------------------------------------------------------
  */
 
+/*
+ * Include Valgrind "client requests", mostly in the memory allocator, so
+ * Valgrind understands PostgreSQL memory contexts.  This permits detecting
+ * memory errors that Valgrind would not detect on a vanilla build.  See also
+ * src/tools/valgrind.supp.  "make installcheck" runs 20-30x longer under
+ * Valgrind.  Note that USE_VALGRIND slowed older versions of Valgrind by an
+ * additional order of magnitude; Valgrind 3.8.1 does not have this problem.
+ * The client requests fall in hot code paths, so USE_VALGRIND also slows
+ * native execution by a few percentage points.
+ *
+ * You should normally use MEMORY_CONTEXT_CHECKING with USE_VALGRIND;
+ * instrumentation of repalloc() is inferior without it.
+ */
+/* #define USE_VALGRIND */
+
 /*
  * Define this to cause pfree()'d memory to be cleared immediately, to
  * facilitate catching bugs that refer to already-freed values.
@@ -218,9 +233,9 @@
 /*
  * Define this to check memory allocation errors (scribbling on more
  * bytes than were allocated).	Right now, this gets defined
- * automatically if --enable-cassert.
+ * automatically if --enable-cassert or USE_VALGRIND.
  */
-#ifdef USE_ASSERT_CHECKING
+#if defined(USE_ASSERT_CHECKING) || defined(USE_VALGRIND)
 #define MEMORY_CONTEXT_CHECKING
 #endif
 

src/include/utils/memdebug.h

+/*-------------------------------------------------------------------------
+ *
+ * memdebug.h
+ *	  Memory debugging support.
+ *
+ * Currently, this file either wraps <valgrind/memcheck.h> or substitutes
+ * empty definitions for Valgrind client request macros we use.
+ *
+ *
+ * Portions Copyright (c) 1996-2013, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/utils/memdebug.h
+ *
+ *-------------------------------------------------------------------------
+ */
+#ifndef MEMDEBUG_H
+#define MEMDEBUG_H
+
+#ifdef USE_VALGRIND
+#include <valgrind/memcheck.h>
+#else
+#define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size)			do {} while (0)
+#define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed)	do {} while (0)
+#define VALGRIND_DESTROY_MEMPOOL(context)					do {} while (0)
+#define VALGRIND_MAKE_MEM_DEFINED(addr, size)				do {} while (0)
+#define VALGRIND_MAKE_MEM_NOACCESS(addr, size)				do {} while (0)
+#define VALGRIND_MAKE_MEM_UNDEFINED(addr, size)				do {} while (0)
+#define VALGRIND_MEMPOOL_ALLOC(context, addr, size)			do {} while (0)
+#define VALGRIND_MEMPOOL_FREE(context, addr)				do {} while (0)
+#define VALGRIND_MEMPOOL_CHANGE(context, optr, nptr, size)	do {} while (0)
+#endif
+
+#endif   /* MEMDEBUG_H */

src/tools/valgrind.supp

+# This is a suppression file for use with Valgrind tools.  File format
+# documentation:
+#	http://valgrind.org/docs/manual/mc-manual.html#mc-manual.suppfiles
+
+# The libc symbol that implements a particular standard interface is
+# implementation-dependent.  For example, strncpy() shows up as "__GI_strncpy"
+# on some platforms.  Use wildcards to avoid mentioning such specific names.
+
+
+# We have occasion to write raw binary structures to disk or to the network.
+# These may contain uninitialized padding bytes.  Since recipients also ignore
+# those bytes as padding, this is harmless.
+
+{
+	padding_pgstat_send
+	Memcheck:Param
+	socketcall.send(msg)
+
+	fun:*send*
+	fun:pgstat_send
+}
+
+{
+	padding_pgstat_sendto
+	Memcheck:Param
+	socketcall.sendto(msg)
+
+	fun:*send*
+	fun:pgstat_send
+}
+
+{
+	padding_pgstat_write
+	Memcheck:Param
+	write(buf)
+
+	...
+	fun:pgstat_write_statsfiles
+}
+
+{
+	padding_XLogRecData_CRC
+	Memcheck:Value8
+
+	fun:XLogInsert
+}
+
+{
+	padding_XLogRecData_write
+	Memcheck:Param
+	write(buf)
+
+    ...
+	fun:XLogWrite
+}
+
+{
+	padding_relcache
+	Memcheck:Param
+	write(buf)
+
+	...
+	fun:write_relcache_init_file
+}
+
+
+# resolve_polymorphic_tupdesc(), a subroutine of internal_get_result_type(),
+# can instigate a memcpy() call where the two pointer arguments are exactly
+# equal.  The behavior thereof is formally undefined, but implementations
+# where it's anything other than a no-op are thought unlikely.
+{
+	noopmemcpy_internal_get_result_type
+	Memcheck:Overlap
+
+	fun:*strncpy*
+	fun:namestrcpy
+	fun:TupleDescInitEntry
+	...
+	fun:internal_get_result_type
+}
+
+
+# gcc on ppc64 can generate a four-byte read to fetch the final "char" fields
+# of a FormData_pg_cast.  This is valid compiler behavior, because a proper
+# FormData_pg_cast has trailing padding.  Tuples we treat as structures omit
+# that padding, so Valgrind reports an invalid read.  Practical trouble would
+# entail the missing pad bytes falling in a different memory page.  So long as
+# the structure is aligned, that will not happen.
+{
+	overread_tuplestruct_pg_cast
+	Memcheck:Addr4
+
+	fun:IsBinaryCoercible
+}