I hacked up (somewhat literally) the attached 3 patches, to try to help
automate GC debugging for MoarVM. The intent is to be able to change how
the nursery memory management is done, to swap in approaches more suitable
for debugging. Ideally I'd also like to be able to debug with

1) the nursery size set to something pathologically small
2) force a tospace/fromspace swap very frequently

but right now there seem to be enough otherwise undetected GC errors that we
can't go that aggressive without the wheels falling off completely.

So, attached are 3 very rough patches that

1) encapsulate allocation, exchange and freeing of the to and fromspaces
2) offer an alternative approach that frees the fromspace once it's no longer
   needed. (Not useful yet, but once nursery size can be made small this makes
   it easier to use valgrind or similar to detect fromspace reads)
3) offer an alternative approach that makes fromspace unreadable

All 3 build Rakudo to the same point as master, except the last, where I need
to disable the mprotect to set unreadable.

That's because the last one has already found its first bug - a read from
fromspace in Lexotic.c


It's very hacky as is, without even a compile-time switch. However, I don't
think that that's the ideal way to go. I think that it ought to be possible
to switch approach via command line flag (or, technically, env var), early
in startup, before the first thread is created.

(Technically, it's actually possible to swap strategy at runtime, but the code
would need a bit more teasing apart).

I think also that it ought to be possible to set the nursery size as a
startup flag, but that's not essential yet. Heck, or even change it at
runtime, although that would need a bit more code.


I'm offering it here on the assumption that it will be useful to others, who
will be able to fix the problems that it discovers. (Until the build is
clean)

Nicholas Clark
>From edbb71149b3cf91cdf6c7c0a406ed39737ac7fb5 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Sun, 15 Dec 2013 13:54:00 +0100
Subject: [PATCH 1/3] Encapsulate nursery allocation, release and exchange behind functions.

This will permit exchanging the regular allocators with various debugging
allocators.
---
 build/Makefile.in        |    1 +
 src/core/threadcontext.c |    9 +++------
 src/core/threadcontext.h |   13 +++++++++++++
 src/gc/collect.c         |   12 +++---------
 src/gc/orchestrate.c     |    5 ++---
 src/gc/thingy.c          |   37 +++++++++++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 18 deletions(-)
 create mode 100644 src/gc/thingy.c

diff --git a/build/Makefile.in b/build/Makefile.in
index fc73691..d8394c1 100644
--- a/build/Makefile.in
+++ b/build/Makefile.in
@@ -75,6 +75,7 @@ OBJECTS = src/core/args@obj@ \
           src/gc/allocation@obj@ \
           src/gc/worklist@obj@ \
           src/gc/roots@obj@ \
+          src/gc/thingy@obj@ \
           src/io/fileops@obj@ \
           src/io/socketops@obj@ \
           src/io/dirops@obj@ \
diff --git a/src/core/threadcontext.c b/src/core/threadcontext.c
index ceef989..7fabea9 100644
--- a/src/core/threadcontext.c
+++ b/src/core/threadcontext.c
@@ -10,10 +10,8 @@ MVMThreadContext * MVM_tc_create(MVMInstance *instance) {
     tc->instance = instance;
 
     /* Set up GC nursery. */
-    tc->nursery_fromspace   = calloc(1, MVM_NURSERY_SIZE);
-    tc->nursery_tospace     = calloc(1, MVM_NURSERY_SIZE);
-    tc->nursery_alloc       = tc->nursery_tospace;
-    tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
+    tc->nursery_thing = &default_nursery_thing;
+    tc->nursery_thing->create_nursery(tc);
 
     /* Set up temporary root handling. */
     tc->num_temproots   = 0;
@@ -58,8 +56,7 @@ void MVM_tc_destroy(MVMThreadContext *tc) {
     uv_run(tc->loop, UV_RUN_NOWAIT);
 
     /* Free the nursery. */
-    free(tc->nursery_fromspace);
-    free(tc->nursery_tospace);
+    tc->nursery_thing->free_nursery(tc);
 
     /* Destroy the second generation allocator. */
     MVM_gc_gen2_destroy(tc->instance, tc->gen2);
diff --git a/src/core/threadcontext.h b/src/core/threadcontext.h
index bcde6d0..c4cd006 100644
--- a/src/core/threadcontext.h
+++ b/src/core/threadcontext.h
@@ -44,6 +44,17 @@ typedef enum {
 #define MVMInitialFramePoolTableSize    64
 #define MVMFramePoolLengthLimit         64
 
+struct MVMThreadContext;
+
+struct MVMNurseryTHING {
+    void (* create_nursery)(MVMThreadContext *);
+    void *(* exchange_spaces)(MVMThreadContext *);
+    void (* disable_fromspace)(MVMThreadContext *);
+    void (* free_nursery)(MVMThreadContext *);
+};
+
+const struct MVMNurseryTHING default_nursery_thing;
+
 /* Information associated with an executing thread. */
 struct MVMThreadContext {
     /* The current allocation pointer, where the next object to be allocated
@@ -166,6 +177,8 @@ struct MVMThreadContext {
 
     /* Random number generator state. */
     MVMuint64 rand_state[2];
+
+    const struct MVMNurseryTHING *nursery_thing;
 };
 
 MVMThreadContext * MVM_tc_create(MVMInstance *instance);
diff --git a/src/gc/collect.c b/src/gc/collect.c
index 302121f..1c0fe76 100644
--- a/src/gc/collect.c
+++ b/src/gc/collect.c
@@ -51,15 +51,9 @@ void MVM_gc_collect(MVMThreadContext *tc, MVMuint8 what_to_do, MVMuint8 gen) {
     /* If we're starting a run (as opposed to just coming back here to do a
      * little more work we got after we first thought we were done...) */
     if (what_to_do != MVMGCWhatToDo_InTray) {
-        /* Swap fromspace and tospace. */
-        void * fromspace = tc->nursery_tospace;
-        void * tospace   = tc->nursery_fromspace;
-        tc->nursery_fromspace = fromspace;
-        tc->nursery_tospace   = tospace;
-
-        /* Reset nursery allocation pointers to the new tospace. */
-        tc->nursery_alloc       = tospace;
-        tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
+	/* Reset nursery allocation pointers to the new tospace. */
+	tc->nursery_alloc       = tc->nursery_thing->exchange_spaces(tc);
+	tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
 
         MVM_gc_worklist_add(tc, worklist, &tc->thread_obj);
         GCDEBUG_LOG(tc, MVM_GC_DEBUG_COLLECT, "Thread %d run %d : processing %d items from thread_obj\n", worklist->items);
diff --git a/src/gc/orchestrate.c b/src/gc/orchestrate.c
index 98d6105..52a6206 100644
--- a/src/gc/orchestrate.c
+++ b/src/gc/orchestrate.c
@@ -296,6 +296,7 @@ static void run_gc(MVMThreadContext *tc, MVMuint8 what_to_do) {
             MVM_gc_collect_free_gen2_unmarked(other);
         }
     }
+    tc->nursery_thing->disable_fromspace(tc);
 }
 
 /* This is called when the allocator finds it has run out of memory and wants
@@ -421,9 +422,7 @@ void MVM_gc_global_destruction(MVMThreadContext *tc) {
 
     /* Fake a nursery collection run by swapping the semi-
      * space nurseries. */
-    nursery_tmp = tc->nursery_fromspace;
-    tc->nursery_fromspace = tc->nursery_tospace;
-    tc->nursery_tospace = nursery_tmp;
+    tc->nursery_thing->exchange_spaces(tc);
 
     /* Run the objects' finalizers */
     MVM_gc_collect_free_nursery_uncopied(tc, tc->nursery_alloc);
diff --git a/src/gc/thingy.c b/src/gc/thingy.c
new file mode 100644
index 0000000..7ce20b6
--- /dev/null
+++ b/src/gc/thingy.c
@@ -0,0 +1,37 @@
+#include "moar.h"
+
+static void
+default_create_nursery(MVMThreadContext *tc) {
+    tc->nursery_fromspace   = calloc(1, MVM_NURSERY_SIZE);
+    tc->nursery_tospace     = calloc(1, MVM_NURSERY_SIZE);
+    tc->nursery_alloc       = tc->nursery_tospace;
+    tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
+}
+
+static void *
+default_exchange_spaces(MVMThreadContext *tc) {
+    /* Swap fromspace and tospace. */
+    void *const fromspace = tc->nursery_tospace;
+    void *const tospace   = tc->nursery_fromspace;
+    tc->nursery_fromspace = fromspace;
+    tc->nursery_tospace   = tospace;
+
+    return tospace;
+}
+
+static void
+default_disable_fromspace(MVMThreadContext *tc) {
+}
+
+static void
+default_free_nursery(MVMThreadContext *tc) {
+    free(tc->nursery_fromspace);
+    free(tc->nursery_tospace);
+}
+
+const struct MVMNurseryTHING default_nursery_thing = {
+    &default_create_nursery,
+    &default_exchange_spaces,
+    &default_disable_fromspace,
+    &default_free_nursery
+};
-- 
1.7.1

>From f74295fec953c0d3ea745b603fe9c3e038c6a152 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Sun, 15 Dec 2013 14:31:13 +0100
Subject: [PATCH 2/3] An alternative nursery strategy that frees the fromspace once it's cleared.

Combing this will valgrind will make it easy (or at least easier) to spot reads
from fromspace. With the caveat that it's going to be very slow, but the
mitigating feature that it will work without babysitting.
---
 src/core/threadcontext.c |    2 +-
 src/core/threadcontext.h |    1 +
 src/gc/thingy.c          |   28 ++++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/src/core/threadcontext.c b/src/core/threadcontext.c
index 7fabea9..9e9bfc4 100644
--- a/src/core/threadcontext.c
+++ b/src/core/threadcontext.c
@@ -10,7 +10,7 @@ MVMThreadContext * MVM_tc_create(MVMInstance *instance) {
     tc->instance = instance;
 
     /* Set up GC nursery. */
-    tc->nursery_thing = &default_nursery_thing;
+    tc->nursery_thing = &recycle_nursery_thing;
     tc->nursery_thing->create_nursery(tc);
 
     /* Set up temporary root handling. */
diff --git a/src/core/threadcontext.h b/src/core/threadcontext.h
index c4cd006..3c02df3 100644
--- a/src/core/threadcontext.h
+++ b/src/core/threadcontext.h
@@ -54,6 +54,7 @@ struct MVMNurseryTHING {
 };
 
 const struct MVMNurseryTHING default_nursery_thing;
+const struct MVMNurseryTHING recycle_nursery_thing;
 
 /* Information associated with an executing thread. */
 struct MVMThreadContext {
diff --git a/src/gc/thingy.c b/src/gc/thingy.c
index 7ce20b6..9913a27 100644
--- a/src/gc/thingy.c
+++ b/src/gc/thingy.c
@@ -35,3 +35,31 @@ const struct MVMNurseryTHING default_nursery_thing = {
     &default_disable_fromspace,
     &default_free_nursery
 };
+
+
+static void
+recycle_create_nursery(MVMThreadContext *tc) {
+    tc->nursery_fromspace   = NULL;
+    tc->nursery_tospace     = calloc(1, MVM_NURSERY_SIZE);
+    tc->nursery_alloc       = tc->nursery_tospace;
+    tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
+}
+
+static void *
+recycle_exchange_spaces(MVMThreadContext *tc) {
+    tc->nursery_fromspace = tc->nursery_tospace;
+    return tc->nursery_tospace = calloc(1, MVM_NURSERY_SIZE);
+}
+
+static void
+recycle_disable_fromspace(MVMThreadContext *tc) {
+    free(tc->nursery_fromspace);
+    tc->nursery_fromspace = NULL;
+}
+
+const struct MVMNurseryTHING recycle_nursery_thing = {
+    &recycle_create_nursery,
+    &recycle_exchange_spaces,
+    &recycle_disable_fromspace,
+    &default_free_nursery
+};
-- 
1.7.1

>From 1a5665932673ddf8c3db96cf89202b4c68fb90eb Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Sun, 15 Dec 2013 15:17:10 +0100
Subject: [PATCH 3/3] An alternative nursery strategy that marks fromspace as unreadable.

This causes any read from frompsace (outside of the GC) to SEGV, which flags
up problems rather rapidly.
---
 src/core/threadcontext.c |    2 +-
 src/core/threadcontext.h |    1 +
 src/gc/thingy.c          |   60 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 1 deletions(-)

diff --git a/src/core/threadcontext.c b/src/core/threadcontext.c
index 9e9bfc4..3b020e8 100644
--- a/src/core/threadcontext.c
+++ b/src/core/threadcontext.c
@@ -10,7 +10,7 @@ MVMThreadContext * MVM_tc_create(MVMInstance *instance) {
     tc->instance = instance;
 
     /* Set up GC nursery. */
-    tc->nursery_thing = &recycle_nursery_thing;
+    tc->nursery_thing = &mmap_nursery_thing;
     tc->nursery_thing->create_nursery(tc);
 
     /* Set up temporary root handling. */
diff --git a/src/core/threadcontext.h b/src/core/threadcontext.h
index 3c02df3..726ad09 100644
--- a/src/core/threadcontext.h
+++ b/src/core/threadcontext.h
@@ -55,6 +55,7 @@ struct MVMNurseryTHING {
 
 const struct MVMNurseryTHING default_nursery_thing;
 const struct MVMNurseryTHING recycle_nursery_thing;
+const struct MVMNurseryTHING mmap_nursery_thing;
 
 /* Information associated with an executing thread. */
 struct MVMThreadContext {
diff --git a/src/gc/thingy.c b/src/gc/thingy.c
index 9913a27..46f5750 100644
--- a/src/gc/thingy.c
+++ b/src/gc/thingy.c
@@ -63,3 +63,63 @@ const struct MVMNurseryTHING recycle_nursery_thing = {
     &recycle_disable_fromspace,
     &default_free_nursery
 };
+
+#include <sys/mman.h>
+
+static void *
+map(int prot) {
+    void *const addr = mmap(NULL, MVM_NURSERY_SIZE, prot, MAP_ANON|MAP_PRIVATE, -1, 0);
+    if (addr == MAP_FAILED) {
+	perror("mmap failed");
+	abort();
+    }
+    return addr;
+}
+
+static void
+mmap_create_nursery(MVMThreadContext *tc) {
+    tc->nursery_fromspace   = map(PROT_NONE);
+    tc->nursery_tospace     = map(PROT_READ|PROT_WRITE);
+    tc->nursery_alloc       = tc->nursery_tospace;
+    tc->nursery_alloc_limit = (char *)tc->nursery_alloc + MVM_NURSERY_SIZE;
+}
+
+static void *
+mmap_exchange_spaces(MVMThreadContext *tc) {
+    /* Swap fromspace and tospace. */
+    void *const tospace   = tc->nursery_fromspace;
+    tc->nursery_fromspace = tc->nursery_tospace;
+    tc->nursery_tospace   = tospace;
+
+    if (mprotect(tospace, MVM_NURSERY_SIZE, PROT_READ|PROT_WRITE)) {
+	perror("protect r/w for tospace failed");
+	abort();
+    }
+
+    return tospace;
+}
+
+static void
+mmap_disable_fromspace(MVMThreadContext *tc) {
+    if (mprotect(tc->nursery_fromspace, MVM_NURSERY_SIZE, PROT_NONE)) {
+	perror("protect none for fromspace failed");
+	abort();
+    }
+}
+
+static void
+mmap_free_nursery(MVMThreadContext *tc) {
+    if (munmap(tc->nursery_fromspace, MVM_NURSERY_SIZE)) {
+	perror("munmap for fromspace failed");
+    }
+    if (munmap(tc->nursery_tospace, MVM_NURSERY_SIZE)) {
+	perror("munmap for tospace failed");
+    }
+}
+
+const struct MVMNurseryTHING mmap_nursery_thing = {
+    &mmap_create_nursery,
+    &mmap_exchange_spaces,
+    &mmap_disable_fromspace,
+    &mmap_free_nursery
+};
-- 
1.7.1

Reply via email to