From 8eb6f8d93fe7cb922969e47ec343e8dafb5cbb2b Mon Sep 17 00:00:00 2001 From: Ivan Maidanski Date: Sat, 23 Jun 2018 00:42:30 +0300 Subject: [PATCH] Add GC_reachable_here after GC_dirty in GC source (fix of commits 73d30d2b4, e5fb574cf) * README.md (Incremental Collection): Add note about bugs caused by a missing GC_reachable_here call. * doc/gcdescr.md (Generational Collection): Mention GC_reachable_here for MANUAL_VDB mode. * finalize.c (GC_register_disappearing_link_inner, GC_register_finalizer_inner): Move GC_dirty(new_dl) call to be before unlocking (so that to ensure no collection occurs between initialization of new_dl and GC_dirty() call). * finalize.c (GC_finalize): Call GC_dirty() immediately after updating GC_fnlz_roots.fo_head (instead of setting needs_barrier) if GC_object_finalized_proc is set. * gcj_mlc.c (GC_gcj_malloc, GC_debug_gcj_malloc, GC_gcj_malloc_ignore_off_page): Call REACHABLE_AFTER_DIRTY(ptr_to_struct_containing_descr) after GC_dirty(op). * include/gc.h (GC_end_stubborn_change): Mention GC_reachable_here in comment. * include/gc_inline.h (GC_FAST_MALLOC_GRANS): Call GC_reachable_here(next) after GC_end_stubborn_change(my_fl); remove GC_end_stubborn_change() call when a non-pointer is stored to my_fl; remove GC_end_stubborn_change() after GC_generic_malloc_many() call. * include/gc_inline.h (GC_CONS): Call GC_reachable_here for the stored pointers after GC_end_stubborn_change call. * include/private/gc_priv.h (REACHABLE_AFTER_DIRTY): New macro. * mallocx.c [MANUAL_VDB] (GC_generic_malloc_many): If GC_is_heap_ptr(result) then call GC_dirty(result) and REACHABLE_AFTER_DIRTY(op) after storing op pointer. * typd_mlc.c (GC_make_sequence_descriptor): Call REACHABLE_AFTER_DIRTY for the stored pointers after GC_dirty(result). * typd_mlc.c (GC_malloc_explicitly_typed, GC_malloc_explicitly_typed_ignore_off_page, GC_calloc_explicitly_typed): Call REACHABLE_AFTER_DIRTY(d) after GC_dirty(op). * win32_threads.c (GC_CreateThread, GC_beginthreadex, GC_pthread_create): Call REACHABLE_AFTER_DIRTY for the stored pointer after GC_dirty. --- README.md | 5 +++-- doc/gcdescr.md | 3 ++- finalize.c | 10 +++++++--- gcj_mlc.c | 7 +++++-- include/gc.h | 2 ++ include/gc_inline.h | 9 ++++++--- include/private/gc_priv.h | 2 ++ mallocx.c | 6 ++++++ typd_mlc.c | 6 ++++++ win32_threads.c | 3 +++ 10 files changed, 42 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 6b4ae5c..e807868 100644 --- a/README.md +++ b/README.md @@ -498,8 +498,9 @@ of information: 2. Information supplied by the programmer. The object is considered dirty after a call to `GC_end_stubborn_change` provided the library has been compiled suitably. It is typically not worth using for short-lived objects. - Note that bugs caused by a missing `GC_end_stubborn_change` call are - likely to be observed very infrequently and hard to trace. + Note that bugs caused by a missing `GC_end_stubborn_change` or + `GC_reachable_here` call are likely to be observed very infrequently and + hard to trace. ## Bugs diff --git a/doc/gcdescr.md b/doc/gcdescr.md index c9d37fc..881a961 100644 --- a/doc/gcdescr.md +++ b/doc/gcdescr.md @@ -398,7 +398,8 @@ We keep track of modified pages using one of several distinct mechanisms: * (`PCR_VDB`) By relying on an external dirty bit implementation, in this case the one in Xerox PCR. * (`MANUAL_VDB`) Through explicit mutator cooperation. This requires the - client code to call `GC_end_stubborn_change`, and is rarely used. + client code to call `GC_end_stubborn_change` (followed by a number of + `GC_reachable_here` calls), and is rarely used. * (`DEFAULT_VDB`) By treating all pages as dirty. This is the default if none of the other techniques is known to be usable. (Practical only for testing.) diff --git a/finalize.c b/finalize.c index c73febe..a80c6c2 100644 --- a/finalize.c +++ b/finalize.c @@ -236,11 +236,11 @@ STATIC int GC_register_disappearing_link_inner( new_dl -> dl_hidden_obj = GC_HIDE_POINTER(obj); new_dl -> dl_hidden_link = GC_HIDE_POINTER(link); dl_set_next(new_dl, dl_hashtbl -> head[index]); + GC_dirty(new_dl); dl_hashtbl -> head[index] = new_dl; dl_hashtbl -> entries++; GC_dirty(dl_hashtbl->head + index); UNLOCK(); - GC_dirty(new_dl); return GC_SUCCESS; } @@ -814,11 +814,11 @@ STATIC void GC_register_finalizer_inner(void * obj, new_fo -> fo_object_size = hhdr -> hb_sz; new_fo -> fo_mark_proc = mp; fo_set_next(new_fo, GC_fnlz_roots.fo_head[index]); + GC_dirty(new_fo); GC_fo_entries++; GC_fnlz_roots.fo_head[index] = new_fo; GC_dirty(GC_fnlz_roots.fo_head + index); UNLOCK(); - GC_dirty(new_fo); } GC_API void GC_CALL GC_register_finalizer(void * obj, @@ -1069,7 +1069,11 @@ GC_INNER void GC_finalize(void) next_fo = fo_next(curr_fo); if (NULL == prev_fo) { GC_fnlz_roots.fo_head[i] = next_fo; - needs_barrier = TRUE; + if (GC_object_finalized_proc) { + GC_dirty(GC_fnlz_roots.fo_head + i); + } else { + needs_barrier = TRUE; + } } else { fo_set_next(prev_fo, next_fo); GC_dirty(prev_fo); diff --git a/gcj_mlc.c b/gcj_mlc.c index 14d3813..56d940f 100644 --- a/gcj_mlc.c +++ b/gcj_mlc.c @@ -194,7 +194,8 @@ static void maybe_finalize(void) *(void **)op = ptr_to_struct_containing_descr; UNLOCK(); GC_dirty(op); - return((void *) op); + REACHABLE_AFTER_DIRTY(ptr_to_struct_containing_descr); + return (void *)op; } /* Similar to GC_gcj_malloc, but add debug info. This is allocated */ @@ -226,6 +227,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_debug_gcj_malloc(size_t lb, result = GC_store_debug_info_inner(result, (word)lb, s, i); UNLOCK(); GC_dirty(result); + REACHABLE_AFTER_DIRTY(ptr_to_struct_containing_descr); return result; } @@ -268,7 +270,8 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_gcj_malloc_ignore_off_page(size_t lb, *(void **)op = ptr_to_struct_containing_descr; UNLOCK(); GC_dirty(op); - return((void *) op); + REACHABLE_AFTER_DIRTY(ptr_to_struct_containing_descr); + return (void *)op; } #endif /* GC_GCJ_SUPPORT */ diff --git a/include/gc.h b/include/gc.h index e43f7f7..6db165c 100644 --- a/include/gc.h +++ b/include/gc.h @@ -526,6 +526,8 @@ GC_API GC_ATTR_DEPRECATED void GC_CALL GC_change_stubborn(const void *); /* Only non-NULL pointer stores into the object are considered to be */ /* changes. Matters only if the library has been compiled with */ /* MANUAL_VDB defined (otherwise the function does nothing). */ +/* Should be followed typically by GC_reachable_here called for each */ +/* of the stored pointers. */ GC_API void GC_CALL GC_end_stubborn_change(const void *) GC_ATTR_NONNULL(1); /* Return a pointer to the base (lowest address) of an object given */ diff --git a/include/gc_inline.h b/include/gc_inline.h index 4335a4e..0117dd3 100644 --- a/include/gc_inline.h +++ b/include/gc_inline.h @@ -128,7 +128,10 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL GC_FAST_M_AO_STORE(my_fl, next); \ init; \ GC_PREFETCH_FOR_WRITE(next); \ - if ((kind) != GC_I_PTRFREE) GC_end_stubborn_change(my_fl); \ + if ((kind) != GC_I_PTRFREE) { \ + GC_end_stubborn_change(my_fl); \ + GC_reachable_here(next); \ + } \ GC_ASSERT(GC_size(result) >= (granules)*GC_GRANULE_BYTES); \ GC_ASSERT((kind) == GC_I_PTRFREE \ || ((GC_word *)result)[1] == 0); \ @@ -141,7 +144,6 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL /* Small counter value, not NULL */ \ GC_FAST_M_AO_STORE(my_fl, (char *)my_entry \ + (granules) + 1); \ - if ((kind) != GC_I_PTRFREE) GC_end_stubborn_change(my_fl); \ result = (default_expr); \ break; \ } else { \ @@ -149,7 +151,6 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL GC_generic_malloc_many(((granules) == 0? GC_GRANULE_BYTES : \ GC_RAW_BYTES_FROM_INDEX(granules)), \ kind, my_fl); \ - GC_end_stubborn_change(my_fl); \ my_entry = *my_fl; \ if (my_entry == 0) { \ result = (*GC_get_oom_fn())((granules)*GC_GRANULE_BYTES); \ @@ -193,6 +194,8 @@ GC_API GC_ATTR_MALLOC GC_ATTR_ALLOC_SIZE(1) void * GC_CALL *(void **)(result) = (void *)(first); \ ((void **)(result))[1] = (void *)(second); \ GC_end_stubborn_change(result); \ + GC_reachable_here(first); \ + GC_reachable_here(second); \ } \ } while (0) diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index 823d2bf..1b739ec 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -2194,8 +2194,10 @@ GC_EXTERN GC_bool GC_print_back_height; #ifdef MANUAL_VDB GC_INNER void GC_dirty_inner(const void *p); /* does not require locking */ # define GC_dirty(p) (GC_incremental ? GC_dirty_inner(p) : (void)0) +# define REACHABLE_AFTER_DIRTY(p) GC_reachable_here(p) #else # define GC_dirty(p) (void)(p) +# define REACHABLE_AFTER_DIRTY(p) (void)(p) #endif /* Same as GC_base but excepts and returns a pointer to const object. */ diff --git a/mallocx.c b/mallocx.c index 8774a3e..6888609 100644 --- a/mallocx.c +++ b/mallocx.c @@ -323,6 +323,12 @@ GC_API void GC_CALL GC_generic_malloc_many(size_t lb, int k, void **result) if (EXPECT(0 != op, TRUE)) obj_link(op) = 0; *result = op; +# ifdef MANUAL_VDB + if (GC_is_heap_ptr(result)) { + GC_dirty(result); + REACHABLE_AFTER_DIRTY(op); + } +# endif return; } GC_ASSERT(k < MAXOBJKINDS); diff --git a/typd_mlc.c b/typd_mlc.c index 823447b..eb40b0e 100644 --- a/typd_mlc.c +++ b/typd_mlc.c @@ -326,6 +326,8 @@ GC_make_sequence_descriptor(complex_descriptor *first, result -> sd_first = first; result -> sd_second = second; GC_dirty(result); + REACHABLE_AFTER_DIRTY(first); + REACHABLE_AFTER_DIRTY(second); } return((complex_descriptor *)result); } @@ -604,6 +606,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_malloc_explicitly_typed(size_t lb, lg = BYTES_TO_GRANULES(GC_size(op)); op[GRANULES_TO_WORDS(lg) - 1] = d; GC_dirty(op + GRANULES_TO_WORDS(lg) - 1); + REACHABLE_AFTER_DIRTY(d); return op; } @@ -645,6 +648,7 @@ GC_API GC_ATTR_MALLOC void * GC_CALL } ((word *)op)[GRANULES_TO_WORDS(lg) - 1] = d; GC_dirty(op + GRANULES_TO_WORDS(lg) - 1); + REACHABLE_AFTER_DIRTY(d); return op; } @@ -699,6 +703,8 @@ GC_API GC_ATTR_MALLOC void * GC_CALL GC_calloc_explicitly_typed(size_t n, op[lw - 1] = (word)complex_descr; GC_dirty(op + lw - 1); + REACHABLE_AFTER_DIRTY(complex_descr); + /* Make sure the descriptor is cleared once there is any danger */ /* it may have been collected. */ if (EXPECT(GC_general_register_disappearing_link( diff --git a/win32_threads.c b/win32_threads.c index e0cc09a..6434497 100644 --- a/win32_threads.c +++ b/win32_threads.c @@ -2267,6 +2267,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit, args -> start = lpStartAddress; args -> param = lpParameter; GC_dirty(args); + REACHABLE_AFTER_DIRTY(lpParameter); set_need_to_lock(); thread_h = CreateThread(lpThreadAttributes, dwStackSize, GC_win32_start, @@ -2320,6 +2321,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit, args -> start = (LPTHREAD_START_ROUTINE)start_address; args -> param = arglist; GC_dirty(args); + REACHABLE_AFTER_DIRTY(arglist); set_need_to_lock(); thread_h = _beginthreadex(security, stack_size, @@ -2613,6 +2615,7 @@ GC_INNER void GC_thr_init(void) si -> start_routine = start_routine; si -> arg = arg; GC_dirty(si); + REACHABLE_AFTER_DIRTY(arg); if (attr != 0 && pthread_attr_getdetachstate(attr, &si->detached) == PTHREAD_CREATE_DETACHED) { -- 2.7.4