As part of being terribly greedy by trying to deny everyone else the pleasure
of finding an GC bugs I've been running with a somewhat stronger write
barrier, specifically checking that the assignee pointer is not in fromspace.
(and hence stale)

I actually made it abort(), because when I tried using
MVM_ASSERT_NOT_FROMSPACE() somewhere, IIRC it made something pass, because
IIRC MVM_exception_throw_adhoc() is trapable, and was trapped (and eaten)


The other part of this was that I figured that what I needed was beyond the
sane use of macros to emulate inline functions. Given that I only need this
to work on current gcc, I just straight out used inline functions.

But I think it would be good for MoarVM to use inline functions, by stealing
how Perl 5 does it (and still works on cranky C89 only compilers)

Specifically, there is probing code in Perl 5's Configure
(search for perl_static_inline) that tries test compiles for all of
'static inline', 'static __inline__', 'static __inline' and 'static _inline'
with a couple of small programs until it finds one that inlines, without
generating an external symbol. At least one of which works on pretty much
all compilers, and for those few that don't, the fall back is 'static'.

I think that it would be cool if someone ported the code the Configure.pl
system, because

1) it will make the MoarVM source code easier to read
2) it will avoid all sorts of subtle bugs from arguments with side effects
   passed to "functions" that happens if the "functions" are actually macros
3) it makes it easier to debug in toolchains that don't have something like
   gcc's -g3


(No, I'm not volunteering myself. I'm already busy looking for GC bugs)

Nicholas Clark

commit f9612859e6f29af1dfb3983c6d2f08bab021b7d3
Author: Nicholas Clark <n...@ccl4.org>
Date:   Sun Dec 22 21:21:46 2013 +0100

    When debugging, assigning pointers to fromspace is thoughtcrime.
    
    Encapsulate the fromspace check into MVM_in_fromspace() and the calculation 
for
    the end of fromspace into MVM_nursery_end(). The latter is so named because
    soon we will have the possibility of the address space reserved for the 
nursery
    being larger than tospace and fromspace.

diff --git a/src/gc/collect.h b/src/gc/collect.h
index d6fd456..a02785e 100644
--- a/src/gc/collect.h
+++ b/src/gc/collect.h
@@ -45,6 +45,17 @@ struct MVMGCPassedWork {
     MVMint32         num_items;
 };
 
+static inline const char *
+MVM_nursery_end(MVMThreadContext *tc) {
+    return (char *)(tc)->nursery_fromspace + MVM_NURSERY_SIZE;
+}
+
+static inline MVMuint8
+MVM_in_fromspace(MVMThreadContext *tc, const void *ref) {
+    return (char *)ref >= (char *)tc->nursery_fromspace
+        && (char *)ref < MVM_nursery_end(tc);
+}
+
 /* Functions. */
 void MVM_gc_collect(MVMThreadContext *tc, MVMuint8 what_to_do, MVMuint8 gen);
 void MVM_gc_collect_free_nursery_uncopied(MVMThreadContext *tc, void *limit);
diff --git a/src/gc/debug.h b/src/gc/debug.h
index 69465bb..aab85da 100644
--- a/src/gc/debug.h
+++ b/src/gc/debug.h
@@ -2,7 +2,6 @@
  * debugging. */
 
 #define MVM_ASSERT_NOT_FROMSPACE(tc, c) do { \
-        if ((char *)(c) >= (char *)tc->nursery_fromspace && \
-                (char *)(c) < (char *)tc->nursery_fromspace + 
MVM_NURSERY_SIZE) \
+        if (MVM_in_fromspace(tc, c))                                    \
             MVM_exception_throw_adhoc(tc, "Collectable in fromspace 
accessed"); \
     } while (0);
diff --git a/src/gc/wb.h b/src/gc/wb.h
index f77e7c1..8daae15 100644
--- a/src/gc/wb.h
+++ b/src/gc/wb.h
@@ -16,6 +16,11 @@
 #define MVM_ASSIGN_REF(tc, update_root, update_addr, referenced) \
     { \
         void *_r = referenced; \
+        if (MVM_in_fromspace(tc, _r)) {                                 \
+            fprintf(stderr, "In fromspace: %p <= %p < %p\n",            \
+                    tc->nursery_fromspace, _r, MVM_nursery_end(tc));    \
+            abort();                                                    \
+        }                                                               \
         MVM_WB(tc, update_root, _r); \
         update_addr = _r; \
     }

Reply via email to