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