Debug improvements

configure.ac

@@ -246,6 +246,24 @@ AC_ARG_ENABLE([memory-checking],
 AC_MSG_CHECKING([whether to enable memory checking])
 AC_MSG_RESULT([$enable_memory_checking])
 
+AC_ARG_ENABLE([valgrind],
+    [AS_HELP_STRING([--enable-valgrind], [enable valgrind extensions to GASMAN])],
+    [AC_DEFINE([MEMORY_CANARY], [1], [define if building with valgrind extensions])],
+    [enable_valgrind=no]
+    )
+AC_MSG_CHECKING([whether to enable valgrind extensions to GASMAN])
+AC_MSG_RESULT([$enable_valgrind])
+
+if test "x$enable_valgrind" != "xno" -a "x$enable_memory_checking" != "xno"; then
+    AC_MSG_ERROR([--enable-valgrind and --enable-memory-checking cannot be used at the same time])
+fi
+
+if test "x$with_gc" != "xgasman"; then
+    if test "x$enable_valgrind" != "xno" -o "x$enable_memory_checking" != "xno"; then
+        AC_MSG_ERROR([--enable-valgrind and --enable-memory-checking are only compatible with GASMAN])
+    fi
+fi
+
 dnl
 dnl User setting: Enable -Werror (off by default)
 dnl

doc/dev/lib.xml

@@ -12,6 +12,69 @@ kernel among the developers.
 
 <Chapter Label="Chap-Lib">
 <Heading>Maintaining the &GAP; kernel and library</Heading>
+
+<Section Label="Sect-debugging">
+<Heading>Debugging the &GAP; Kernel</Heading>
+
+The &GAP; kernel supports a variety of options which can be enabled by running
+<C>configure</C> which provide various checks which can be useful when writing
+and debugging &GAP; kernel code.
+<P/>
+
+The first option, <C>--enable-debug</C>, is well supported can can be used
+whenever the &GAP; kernel, or kernel modules, are being developed (although
+developers should still check their code works without it).
+<P/>
+<C>--enable-debug</C> enables assertions throughout the kernel. These
+assertions are implemented using the <C>GAP_ASSERT</C> macro. These
+assertions are disabled when &GAP; is compiled without <C>--enable-debug</C>.
+When <C>--enable-debug</C> is enabled, <C>KernelDebug</C> will be shown in
+the line starting <C>Configuration:</C> when &GAP; is started.
+<P/>
+
+The next two options are more experimental, and are not designed to be constantly
+enabled. They are particularly designed to track down bugs relating to memory
+corruption relating to GASMAN (&GAP;'s memory manager). They cannot both be enabled
+at the same time.
+
+These options assume you are familiar somewhat familiar with the internals of GASMAN
+(see gasman.c for more information). In particular, GASMAN represents memory using
+an object of type Bag. The contents of these Bags can be moved during garbage collection.
+
+<P/>
+<C>--enable-memory-checking</C> makes &GAP; check for C pointers to the content of Bags
+being used after a new Bag has been created or a Bag is resized. GASMAN
+moves Bags during garbage collection, which can happen whenever a new Bag is created
+or a Bag is resized. However, as it is very unlikely that any single allocation
+will cause a garbage collection, these bugs trigger very rarely. Also the problems
+caused by these bugs are, if the C pointer is written to, a random other Bag,
+somewhere in &GAP;, is changed.
+<P/>
+
+After configuring, the memory checking must still be turned on. This can be done
+either by passing <C>--enableMemCheck</C> to &GAP;'s command line, or calling <C>GASMAN_MEM_CHECK(1)</C>.
+Note that enabling these tests makes &GAP; VERY, VERY slow. It can (depending on the
+machine and operating system) take &GAP; over a day to start, and load all standard
+packages. The recommending way to use this option is to start &GAP;, and then load
+small test files to try to isolate the problem. <C>MemCheck</C> will be shown in
+the line starting <C>Configuration:</C> when &GAP; is started.
+<P/>
+
+Known bugs: &GAP; will crash if <C>IO_fork</C> from the IO package is called while
+memory checking is enabled.
+<P/>
+
+<C>--enable-valgrind</C> makes changes to GASMAN so it is compatible with the
+<URL Text="valgrind">http://valgrind.org</URL> memory checking program.
+Without this, Valgrind will report many incorrect warnings relating to GASMAN
+and no useful warnings.
+<P/>
+
+At present, this <C>--enable-valgrind</C> only checks for invalid writes to the
+last bag which was created. Also, this option does not do anything unless &GAP;
+is run through Valgrind (for example by running <C>valgrind gap</C>.
+</Section>
+
 <Index>modules</Index>
 
 Modules, PROPOSALs, ...

src/gasman.c

@@ -271,40 +271,6 @@ Bag *                   AllocBags;
 UInt                    AllocSizeBags;
 Bag *                   EndBags;
 
-#if defined(MEMORY_CANARY)
-
-#include <valgrind/valgrind.h>
-#include <valgrind/memcheck.h>
-Int canary_size() {
-  Int bufsize = (Int)EndBags - (Int)AllocBags;
-  return bufsize<4096?bufsize:4096;
-}
-
-void ADD_CANARY() {
-  VALGRIND_MAKE_MEM_NOACCESS(AllocBags, canary_size());
-}
-void CLEAR_CANARY() {
-  VALGRIND_MAKE_MEM_DEFINED(AllocBags, canary_size());
-}
-#define CANARY_DISABLE_VALGRIND()  VALGRIND_DISABLE_ERROR_REPORTING
-#define CANARY_ENABLE_VALGRIND() VALGRIND_ENABLE_ERROR_REPORTING
-
-void CHANGED_BAG(Bag bag) {
-    CANARY_DISABLE_VALGRIND();
-    if (CONST_PTR_BAG(bag) <= YoungBags && LINK_BAG(bag) == bag) {
-        LINK_BAG(bag) = ChangedBags;
-        ChangedBags = bag;
-    }
-    CANARY_ENABLE_VALGRIND();
-}
-#else
-#define ADD_CANARY()
-#define CLEAR_CANARY()
-#define CANARY_DISABLE_VALGRIND()
-#define CANARY_ENABLE_VALGRIND()
-#endif
-
-
 /* These macros, are (a) for more readable code, but more importantly
    (b) to ensure that unsigned subtracts and divides are used (since
    we know the ordering of the pointers. This is needed on > 2GB
@@ -484,6 +450,44 @@ static inline UInt IS_BAG_BODY(void * ptr)
             ((UInt)ptr & (sizeof(Bag) - 1)) == 0);
 }
 
+#if defined(MEMORY_CANARY)
+
+#include <valgrind/valgrind.h>
+#include <valgrind/memcheck.h>
+static Int canary_size(void)
+{
+    Int bufsize = (Int)EndBags - (Int)AllocBags;
+    return bufsize < 4096 ? bufsize : 4096;
+}
+
+static void ADD_CANARY(void)
+{
+    VALGRIND_MAKE_MEM_NOACCESS(AllocBags, canary_size());
+}
+
+static void CLEAR_CANARY(void)
+{
+    VALGRIND_MAKE_MEM_DEFINED(AllocBags, canary_size());
+}
+#define CANARY_DISABLE_VALGRIND() VALGRIND_DISABLE_ERROR_REPORTING
+#define CANARY_ENABLE_VALGRIND() VALGRIND_ENABLE_ERROR_REPORTING
+
+void CHANGED_BAG(Bag bag)
+{
+    CANARY_DISABLE_VALGRIND();
+    if (CONST_PTR_BAG(bag) <= YoungBags && LINK_BAG(bag) == bag) {
+        LINK_BAG(bag) = ChangedBags;
+        ChangedBags = bag;
+    }
+    CANARY_ENABLE_VALGRIND();
+}
+#else
+#define ADD_CANARY()
+#define CLEAR_CANARY()
+#define CANARY_DISABLE_VALGRIND()
+#define CANARY_ENABLE_VALGRIND()
+#endif
+
 /****************************************************************************
 **
 *F  InitMsgsFuncBags(<msgs-func>) . . . . . . . . .  install message function