[gnugo-devel] better valgrind checking

[Arend Bayer]

- support for --enable-valgrind

With this enabled, gnugo will mark lots of memory as unitialized
(that is what the macro VALGRIND_MAKE_WRITABLE() means in this case).
This is useful for memory that is technically initialized, but should be
logically be considered as unitialized, such as the owl stack when a
new owl reading is started.

The hunk in worm.c is to silence a (harmless) complaint by valgrind about
overlapping source and dest in memcpy(), which made it unusable to find
other problems.

There is no change when compiled without --enable-valgrind, and gnugo
compiled "with" still runs without difference (only difference is a few
bytes bigger code size).

Arend


Index: config.h.in
===================================================================
RCS file: /cvsroot/gnugo/gnugo/config.h.in,v
retrieving revision 1.35
diff -u -p -r1.35 config.h.in
--- config.h.in 23 Apr 2004 13:21:53 -0000      1.35
+++ config.h.in 27 Apr 2004 16:58:22 -0000
@@ -133,6 +133,9 @@
 /* Break-in module. Enabled by default. */
 #undef USE_BREAK_IN
 
+/* Define special valgrind macros. */
+#undef USE_VALGRIND
+
 /* Version number of package */
 #undef VERSION
 
Index: config.vc
===================================================================
RCS file: /cvsroot/gnugo/gnugo/config.vc,v
retrieving revision 1.93
diff -u -p -r1.93 config.vc
--- config.vc   23 Apr 2004 16:11:42 -0000      1.93
+++ config.vc   27 Apr 2004 16:58:22 -0000
@@ -55,9 +55,11 @@
 /* Break-in module. Enabled by default. */
 #define USE_BREAK_IN 1
 
+/* Define special valgrind macros. */
+#define USE_VALGRIND 0
+
 /* Large scale captures. Disabled by default. */
 #define LARGE_SCALE 0
-
 
 /* Version number of package */
 #define PACKAGE "gnugo"
Index: config.vcin
===================================================================
RCS file: /cvsroot/gnugo/gnugo/config.vcin,v
retrieving revision 1.29
diff -u -p -r1.29 config.vcin
--- config.vcin 23 Apr 2004 13:21:53 -0000      1.29
+++ config.vcin 27 Apr 2004 16:58:22 -0000
@@ -55,9 +55,11 @@
 /* Break-in module. Enabled by default. */
 #define USE_BREAK_IN 1
 
+/* Define special valgrind macros. */
+#define USE_VALGRIND 0
+
 /* Large scale captures. Disabled by default. */
 #define LARGE_SCALE 0
-
 
 /* Version number of package */
 #define PACKAGE "gnugo"
Index: configure.in
===================================================================
RCS file: /cvsroot/gnugo/gnugo/configure.in,v
retrieving revision 1.103
diff -u -p -r1.103 configure.in
--- configure.in        23 Apr 2004 16:11:43 -0000      1.103
+++ configure.in        27 Apr 2004 16:58:22 -0000
@@ -508,6 +508,17 @@ else
    AC_DEFINE(OWL_THREATS, 0)
 fi
 
+dnl ------------ additional valgrind macros ------
+
+AH_TEMPLATE([USE_VALGRIND],
+[Define special valgrind macros.])
+
+if test "$enable_valgrind" = "yes" ; then
+   AC_DEFINE(USE_VALGRIND, 1)
+else 
+   AC_DEFINE(USE_VALGRIND, 0)
+fi
+
 dnl  ----------- special-case use of gcc ---------
 
 dnl Not sure if we are supposed to be accessing this variable, but...
Index: makevcdist.pl
===================================================================
RCS file: /cvsroot/gnugo/gnugo/makevcdist.pl,v
retrieving revision 1.18
diff -u -p -r1.18 makevcdist.pl
--- makevcdist.pl       27 Jun 2003 04:34:31 -0000      1.18
+++ makevcdist.pl       27 Apr 2004 16:58:22 -0000
@@ -22,7 +22,8 @@ my %defaults =
    OWL_THREATS => 0,
    USE_BREAK_IN => 1,
    COSMIC_GNUGO => 0,
-   ORACLE => 0);
+   ORACLE => 0,
+   USE_VALGRIND => 0);
 
 my @skip = qw/
   GNU_PACKAGE 
Index: engine/board.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/board.c,v
retrieving revision 1.95
diff -u -p -r1.95 board.c
--- engine/board.c      19 Apr 2004 18:24:45 -0000      1.95
+++ engine/board.c      27 Apr 2004 16:58:22 -0000
@@ -89,9 +89,12 @@ struct vertex_stack_entry {
 #define STACK_SIZE 80 * MAXSTACK
 
 
-#define CLEAR_STACKS()\
-(change_stack_pointer = change_stack, \
- vertex_stack_pointer = vertex_stack)
+#define CLEAR_STACKS() do { \
+  change_stack_pointer = change_stack; \
+  vertex_stack_pointer = vertex_stack; \
+  VALGRIND_MAKE_WRITABLE(change_stack, sizeof(change_stack)); \
+  VALGRIND_MAKE_WRITABLE(vertex_stack, sizeof(vertex_stack)); \
+} while (0)
 
 /* Begin a record : address == NULL */
 #define BEGIN_CHANGE_RECORD()\
@@ -3009,6 +3012,7 @@ new_position(void)
 
   memset(string, 0, sizeof(string));
   memset(ml, 0, sizeof(ml));
+  VALGRIND_MAKE_WRITABLE(next_stone, sizeof(next_stone));
 
   /* propagate_string relies on non-assigned stones to have
    * string_number -1.
Index: engine/board.h
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/board.h,v
retrieving revision 1.9
diff -u -p -r1.9 board.h
--- engine/board.h      12 Apr 2004 15:22:27 -0000      1.9
+++ engine/board.h      27 Apr 2004 16:58:22 -0000
@@ -24,6 +24,7 @@
 #define _BOARD_H_
 
 #include "sgftree.h"
+#include "config.h"
 #include <stdarg.h>
 
 /* local versions of absolute value, min and max */
@@ -406,6 +407,13 @@ void abortgo(const char *file, int line,
 #endif
 
 #define gg_assert(x) ASSERT1(x, NO_MOVE);
+
+/* Are we using valgrind memory checking? */
+#if USE_VALGRIND
+#include <valgrind/memcheck.h>
+#else
+#define VALGRIND_MAKE_WRITABLE(a, b)
+#endif
 
 #endif  /* _BOARD_H_ */
 
Index: engine/dragon.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/dragon.c,v
retrieving revision 1.135
diff -u -p -r1.135 dragon.c
--- engine/dragon.c     23 Apr 2004 01:51:29 -0000      1.135
+++ engine/dragon.c     27 Apr 2004 16:58:22 -0000
@@ -665,6 +665,7 @@ void
 initialize_dragon_data(void)
 {
   int str;
+  /* VALGRIND_MAKE_WRITABLE(dragon, BOARDMAX * sizeof(struct dragon_data)); */
   for (str = BOARDMIN; str < BOARDMAX; str++)
     if (ON_BOARD(str)) {
 
Index: engine/influence.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/influence.c,v
retrieving revision 1.99
diff -u -p -r1.99 influence.c
--- engine/influence.c  24 Jan 2004 04:04:56 -0000      1.99
+++ engine/influence.c  27 Apr 2004 16:58:22 -0000
@@ -1150,6 +1150,7 @@ compute_influence(int color, const char 
                  int move, const char *trace_message)
 {
   int save_debug = debug;
+  VALGRIND_MAKE_WRITABLE(q, sizeof(*q));
 
   q->is_territorial_influence = 1;
   q->color_to_move = color;
@@ -1774,6 +1775,7 @@ compute_escape_influence(int color, cons
   int k;
   int ii;
   int save_debug = debug;
+  VALGRIND_MAKE_WRITABLE(&escape_influence, sizeof(escape_influence));
 
   /* IMPORTANT: The caching relies on the fact that safe_stones[] and
    * strength[] will currently always be identical for identical board[]
Index: engine/owl.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/owl.c,v
retrieving revision 1.208
diff -u -p -r1.208 owl.c
--- engine/owl.c        23 Apr 2004 03:06:36 -0000      1.208
+++ engine/owl.c        27 Apr 2004 16:58:23 -0000
@@ -6348,6 +6348,7 @@ reduced_init_owl(struct local_owl_data *
 
   check_owl_stack_size();
   *owl = owl_stack[owl_stack_pointer];
+  VALGRIND_MAKE_WRITABLE(*owl, sizeof(struct local_owl_data));
 }
 
 
@@ -6393,6 +6394,8 @@ do_push_owl(struct local_owl_data **owl)
 {
   struct local_owl_data *new_owl = owl_stack[owl_stack_pointer];
 
+  /* Mark all the data in *new_owl as uninitialized. */
+  VALGRIND_MAKE_WRITABLE(new_owl, sizeof(struct local_owl_data));
   /* Copy the owl data. */
   memcpy(new_owl->goal, (*owl)->goal, sizeof(new_owl->goal));
   memcpy(new_owl->boundary, (*owl)->boundary, sizeof(new_owl->boundary));
Index: engine/readconnect.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/readconnect.c,v
retrieving revision 1.75
diff -u -p -r1.75 readconnect.c
--- engine/readconnect.c        23 Apr 2004 00:29:36 -0000      1.75
+++ engine/readconnect.c        27 Apr 2004 16:58:23 -0000
@@ -2752,6 +2752,7 @@ init_connection_data(int color, const ch
   char mark[BOARDMAX];
 
   memset(mark, 0, BOARDMAX);
+  VALGRIND_MAKE_WRITABLE(conn, sizeof(conn));
   clear_connection_data(conn);
 
   for (pos = BOARDMIN; pos < BOARDMAX; pos++) {
@@ -4143,7 +4144,6 @@ compute_connection_distances(int str, in
 
   spread_connection_distances(color, conn);
 }
-
 
 
 /* Print the connection distances in a struct connection_data. */
Index: engine/worm.c
===================================================================
RCS file: /cvsroot/gnugo/gnugo/engine/worm.c,v
retrieving revision 1.61
diff -u -p -r1.61 worm.c
--- engine/worm.c       12 Apr 2004 15:22:45 -0000      1.61
+++ engine/worm.c       27 Apr 2004 16:58:23 -0000
@@ -1223,7 +1223,8 @@ propagate_worm2(int str)
   ASSERT1(IS_STONE(worm[str].color), str);
 
   for (pos = BOARDMIN; pos < BOARDMAX; pos++)
-    if (board[pos] == board[str] && is_same_worm(pos, str))
+    if (board[pos] == board[str] && is_same_worm(pos, str)
+       && pos != str)
       worm[pos] = worm[str];
 }