[sgen] Pass value of reference to major mark function, too.
authorMark Probst <mark.probst@gmail.com>
Wed, 2 Jan 2013 21:20:13 +0000 (22:20 +0100)
committerMark Probst <mark.probst@gmail.com>
Wed, 2 Jan 2013 21:20:13 +0000 (22:20 +0100)
The major mark function assumes that the reference to process
 is not NULL.  Its caller checks that it isn't, but the mark function loads
the reference again.  In the concurrent case this is a bug, because
the mutator might have changed the reference between the loads.

mono/metadata/sgen-major-copying.c
mono/metadata/sgen-major-scan-object.h
mono/metadata/sgen-marksweep.c

index ce9a7fc302c7d6ae853ce4810860c6bbfc0e6fad..496c3e4a889310c263a5ee8451ab86b710528ce9 100644 (file)
@@ -279,10 +279,10 @@ pin_major_object (char *obj, SgenGrayQueue *queue)
 #include "sgen-major-copy-object.h"
 
 static void
-major_copy_or_mark_object (void **obj_slot, SgenGrayQueue *queue)
+major_copy_or_mark_object (void **obj_slot, void *obj_void, SgenGrayQueue *queue)
 {
        char *forwarded;
-       char *obj = *obj_slot;
+       char *obj = obj_void;
        mword objsize;
 
        SGEN_ASSERT (9, current_collection_generation == GENERATION_OLD, "old gen parallel allocator called from a %d collection", current_collection_generation);
@@ -387,6 +387,12 @@ major_copy_or_mark_object (void **obj_slot, SgenGrayQueue *queue)
        *obj_slot = copy_object_no_checks (obj, queue);
 }
 
+static void
+major_copy_or_mark_object_canonical (void **ptr, SgenGrayQueue *queue)
+{
+       major_copy_or_mark_object (ptr, *ptr, queue);
+}
+
 #include "sgen-major-scan-object.h"
 
 /* FIXME: later reduce code duplication here with build_nursery_fragments().
@@ -692,7 +698,7 @@ sgen_copying_init (SgenMajorCollector *collector)
        collector->handle_gc_param = NULL;
        collector->print_gc_param_usage = NULL;
 
-       collector->major_ops.copy_or_mark_object = major_copy_or_mark_object;
+       collector->major_ops.copy_or_mark_object = major_copy_or_mark_object_canonical;
        collector->major_ops.scan_object = major_scan_object;
 }
 
index 5db30e22d272a1b14d59070ee43c16f61259eb17..725c76584fae754c11ac643a36a76f81e5de77f7 100644 (file)
@@ -39,7 +39,7 @@ extern long long stat_scan_object_called_major;
                void *__copy;                                           \
                if (__old && FOLLOW_OBJECT (__old)) {                   \
                        PREFETCH_DYNAMIC_HEAP (__old);                  \
-                       major_copy_or_mark_object ((ptr), queue);       \
+                       major_copy_or_mark_object ((ptr), __old, queue);        \
                        __copy = *(ptr);                                \
                        SGEN_COND_LOG (9, __old != __copy, "Overwrote field at %p with %p (was: %p)", (ptr), *(ptr), __old); \
                        if (G_UNLIKELY (sgen_ptr_in_nursery (__copy) && !sgen_ptr_in_nursery ((ptr)))) \
index 3cbc62a0df177c9c858cceef8b2f6a528b8c0500..95af5ca550eb50bb6819ae5cc9e948a5b1824a3e 100644 (file)
@@ -1077,9 +1077,8 @@ pin_major_object (char *obj, SgenGrayQueue *queue)
 
 #ifdef SGEN_PARALLEL_MARK
 static void
-major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
+major_copy_or_mark_object (void **ptr, void *obj, SgenGrayQueue *queue)
 {
-       void *obj = *ptr;
        mword objsize;
        MSBlockInfo *block;
        MonoVTable *vt;
@@ -1233,10 +1232,8 @@ major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
 #else
 #ifdef SGEN_CONCURRENT_MARK
 static void
-major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
+major_copy_or_mark_object (void **ptr, void *obj, SgenGrayQueue *queue)
 {
-       void *obj = *ptr;
-
        g_assert (!SGEN_OBJECT_IS_FORWARDED (obj));
 
        if (!sgen_ptr_in_nursery (obj)) {
@@ -1268,9 +1265,8 @@ major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
 }
 #else
 static void
-major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
+major_copy_or_mark_object (void **ptr, void *obj, SgenGrayQueue *queue)
 {
-       void *obj = *ptr;
        MSBlockInfo *block;
 
        HEAVY_STAT (++stat_copy_object_called_major);
@@ -1402,6 +1398,12 @@ major_copy_or_mark_object (void **ptr, SgenGrayQueue *queue)
 #endif
 #endif
 
+static void
+major_copy_or_mark_object_canonical (void **ptr, SgenGrayQueue *queue)
+{
+       major_copy_or_mark_object (ptr, *ptr, queue);
+}
+
 #ifdef SGEN_CONCURRENT_MARK
 static long long
 major_get_and_reset_num_major_objects_marked (void)
@@ -2349,7 +2351,7 @@ sgen_marksweep_init
        collector->is_valid_object = major_is_valid_object;
        collector->describe_pointer = major_describe_pointer;
 
-       collector->major_ops.copy_or_mark_object = major_copy_or_mark_object;
+       collector->major_ops.copy_or_mark_object = major_copy_or_mark_object_canonical;
        collector->major_ops.scan_object = major_scan_object;
 #ifdef SGEN_CONCURRENT_MARK
        collector->major_ops.scan_vtype = major_scan_vtype;