First stab at Valgrind integration. First goal: eliminate false

positives when gc is accessing uninitialized parts of the stack.

3 files changed, 71 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index f0a326b0..12d02a11 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2009-11-25  Kaz Kylheku  <kkylheku@gmail.com>
+
+	First stab at Valgrind integration. First goal: eliminate false
+	positives when gc is accessing uninitialized parts of the stack.
+
+	* configure (valgrind): New variable. Defaults to false (do not
+	build valgrind support). New check for whether the valgrind API
+	is actually avilable if --valgrind is selected.
+	(HAVE_VALGRIND): Conditionally added to config.h.
+
+	* gc.c: Conditionally include valgrind memcheck.h header.
+	(mark_mem_region): After pulling out a value from the stack,
+	mark that copy as defined memory using VALGRIND_MAKE_MEM_DEFINED.
+	(mark): Removed check for a registered root variable pointer
+	being null; this cannot happen, unless someone registers a
+	null pointer, or the stack is trashed. The comment about a
+	possible null was misleading.
+
 2009-11-24  Kaz Kylheku  <kkylheku@gmail.com>
 
 	Fix uninitialized memory locations.
diff --git a/configure b/configure
index 58737386..5c71ef51 100755
--- a/configure
+++ b/configure
@@ -120,6 +120,8 @@ platform_flags=${platform_flags-}
 remove_flags=${remove_flags-}
 lex_dbg_flags=${lex_dbg_flags-}
 txr_dbg_opts=${txr_dbg_opts---gc-debug}
+valgrind=${valgrind-}
+
 #
 # If --help was given (or --help=<nonempty> or help=<nonempty>) then
 # print help and exit. The termination status is failed, to indicate
@@ -295,6 +297,13 @@ txr_dbg_opts [$txr_dbg_opts]
 
   Specifies debug flags to pass to the txr program during the execution
   of "make tests".
+
+valgrind [$valgrind]
+
+  Use --valgrind to to build txr with valgrind integration.
+  Valgrind integration means that when the program is running under valgrind,
+  it advises valgrind about stack memory locations accessed by the garbage
+  collector, to suppress diagnostics about uninitialized accesses.
 !
   exit 1
 fi
@@ -635,15 +644,44 @@ $inline int func(void)
   return 0;
 }
 !
-    if ! make conftest2 > conftest.err 2>&1 ; then
-	continue
-    fi
-    break
-  done
+	if ! make conftest2 > conftest.err 2>&1 ; then
+	  continue
+	fi
+	break
+    done
+  fi
+
+  printf '"%s"\n' "$inline"
+  printf "#define INLINE $inline\n" >> config.h
+
+  if [ -n "$valgrind" ] ; then
+    printf "Checking valgrind API availability ... "
+
+    cat > conftest.c <<!
+  #include <valgrind/memcheck.h>
+
+  #ifdef VALGRIND_DO_CLIENT_REQUEST
+
+  int main(void)
+  {
+    return 0;
+  }
+
+  #else
+  syntax error
+  #endif
+!
+  if ! make conftest > conftest.err 2>&1 || ! [ -x conftest ] ; then
+    printf "failed\n\n"
+    printf "Errors from compilation: \n\n"
+    cat conftest.err
+    exit 1
+  fi
+
+  printf "okay\n"
+  printf "#define HAVE_VALGRIND\n" >> config.h
 fi
 
-printf '"%s"\n' "$inline"
-printf "#define INLINE $inline\n" >> config.h
 
 #
 # Clean up
diff --git a/gc.c b/gc.c
index 4503d894..277b2adf 100644
--- a/gc.c
+++ b/gc.c
@@ -32,6 +32,9 @@
 #include <dirent.h>
 #include <wchar.h>
 #include "config.h"
+#ifdef HAVE_VALGRIND
+#include <valgrind/memcheck.h>
+#endif
 #include "lib.h"
 #include "stream.h"
 #include "hash.h"
@@ -281,6 +284,9 @@ static void mark_mem_region(val *low, val *high)
 
   while (low < high) {
     val maybe_obj = *low;
+#ifdef HAVE_VALGRIND
+    VALGRIND_MAKE_MEM_DEFINED(&maybe_obj, sizeof maybe_obj);
+#endif
     if (in_heap(maybe_obj)) {
       type_t t = maybe_obj->t.type;
       if ((t & FREE) == 0)
@@ -299,10 +305,8 @@ static void mark(void)
    * First, scan the officially registered locations.
    */
 
-  for (rootloc = prot_stack; rootloc != top; rootloc++) {
-    if (*rootloc) /* stack may have nulls */
-      mark_obj(**rootloc);
-  }
+  for (rootloc = prot_stack; rootloc != top; rootloc++)
+    mark_obj(**rootloc);
 
   mark_mem_region(&gc_stack_top, gc_stack_bottom);
 }