From e6f85a9b9d56b2f76da6b934771928302571ab45 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Thu, 23 Jul 2015 16:29:02 -0400 Subject: [PATCH 1/2] Add jl_gc_find_taggedvalue_pool to check if a pointer belongs to the GC memory pool. --- src/gc-debug.c | 25 +++++++++++++++++++++++++ src/gc.c | 16 +++++++++++----- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/src/gc-debug.c b/src/gc-debug.c index c2355ce161d8b..38ab8967df662 100644 --- a/src/gc-debug.c +++ b/src/gc-debug.c @@ -1,5 +1,30 @@ // This file is a part of Julia. License is MIT: http://julialang.org/license +// Find the memory block in the pool that owns the byte pointed to by p. +// For end of object pointer (which is always the case for pointer to a +// singleton object), this usually returns the same pointer which points to +// the next object but it can also return NULL if the pointer is pointing to +// the end of the page. +DLLEXPORT jl_taggedvalue_t *jl_gc_find_taggedvalue_pool(char *p, + size_t *osize_p) +{ + region_t *r = find_region(p, 1); + if (!r) + return NULL; + char *page_begin = GC_PAGE_DATA(p) + GC_PAGE_OFFSET; + if (p < page_begin) + return NULL; + size_t ofs = p - page_begin; + int pg_idx = PAGE_INDEX(r, p); + gcpage_t *pagemeta = &r->meta[pg_idx]; + int osize = pagemeta->osize; + if (osize == 0) + return NULL; + if (osize_p) + *osize_p = osize; + return (jl_taggedvalue_t*)((char*)p - (ofs % osize)); +} + #ifdef GC_DEBUG_ENV #include #include diff --git a/src/gc.c b/src/gc.c index 7eff7eacf9f4f..4cebfc0fa7a70 100644 --- a/src/gc.c +++ b/src/gc.c @@ -259,6 +259,10 @@ static int check_timeout = 0; static gcpage_t *page_metadata(void *data); static void pre_mark(void); static void post_mark(arraylist_t *list, int dryrun); +static region_t *find_region(void *ptr, int maybe); +#define PAGE_INDEX(region, data) \ + ((GC_PAGE_DATA((data) - GC_PAGE_OFFSET) - \ + &(region)->pages[0][0])/GC_PAGE_SZ) #include "gc-debug.c" @@ -391,21 +395,23 @@ void jl_finalize(jl_value_t *o) (void)finalize_object(o); } -#define PAGE_INDEX(region, data) ((GC_PAGE_DATA((data) - GC_PAGE_OFFSET) - &(region)->pages[0][0])/GC_PAGE_SZ) -static region_t *find_region(void *ptr) +static region_t *find_region(void *ptr, int maybe) { // on 64bit systems we could probably use a single region and remove this loop for (int i = 0; i < REGION_COUNT && regions[i]; i++) { - if ((char*)ptr >= (char*)regions[i] && (char*)ptr <= (char*)regions[i] + sizeof(region_t)) + char *begin = ®ions[i]->pages[0][0]; + char *end = begin + sizeof(regions[i]->pages); + if ((char*)ptr >= begin && (char*)ptr <= end) return regions[i]; } - assert(0 && "find_region failed"); + (void)maybe; + assert(maybe && "find_region failed"); return NULL; } static gcpage_t *page_metadata(void *data) { - region_t *r = find_region(data); + region_t *r = find_region(data, 0); int pg_idx = PAGE_INDEX(r, (char*)data); return &r->meta[pg_idx]; } From fc0db0ec7e291abdf03b9a5d857d49e86c73b526 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Sun, 26 Jul 2015 09:27:33 -0400 Subject: [PATCH 2/2] Re-implement Oscar's GC scrub with a safer approach Instead of trashing pointer-like values on the stack, we fill dead objects that is possibly referred to from the stack with garbage. --- src/gc-debug.c | 26 ++++++++++++++++++++++++++ src/gc.c | 12 ++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/gc-debug.c b/src/gc-debug.c index 38ab8967df662..ba6a83a9085b4 100644 --- a/src/gc-debug.c +++ b/src/gc-debug.c @@ -287,8 +287,10 @@ static int gc_debug_alloc_check(jl_alloc_num_t *num) return ((num->num - num->min) % num->interv) == 0; } +static char *gc_stack_lo; static void gc_debug_init() { + gc_stack_lo = (char*)gc_get_stack_ptr(); char *env = getenv("JL_GC_NO_GENERATIONAL"); if (env && strcmp(env, "0") != 0) { gc_debug_env.sweep_mask = GC_MARKED; @@ -326,6 +328,25 @@ static inline void gc_debug_print() gc_debug_print_status(); } +static void gc_scrub_range(char *stack_lo, char *stack_hi) +{ + stack_lo = (char*)((uintptr_t)stack_lo & ~(uintptr_t)15); + for (char **stack_p = (char**)stack_lo; + stack_p > (char**)stack_hi;stack_p--) { + char *p = *stack_p; + size_t osize; + jl_taggedvalue_t *tag = jl_gc_find_taggedvalue_pool(p, &osize); + if (!tag || gc_marked(tag) || osize <= sizeof_jl_taggedvalue_t) + continue; + memset(tag, 0xff, osize); + } +} + +static void gc_scrub(char *stack_hi) +{ + gc_scrub_range(gc_stack_lo, stack_hi); +} + #else static inline int gc_debug_check_other() @@ -346,4 +367,9 @@ static inline void gc_debug_init() { } +static void gc_scrub(char *stack_hi) +{ + (void)stack_hi; +} + #endif diff --git a/src/gc.c b/src/gc.c index 4cebfc0fa7a70..fb33e1c785677 100644 --- a/src/gc.c +++ b/src/gc.c @@ -260,10 +260,20 @@ static gcpage_t *page_metadata(void *data); static void pre_mark(void); static void post_mark(arraylist_t *list, int dryrun); static region_t *find_region(void *ptr, int maybe); +jl_taggedvalue_t *jl_gc_find_taggedvalue_pool(char *p, size_t *osize_p); + #define PAGE_INDEX(region, data) \ ((GC_PAGE_DATA((data) - GC_PAGE_OFFSET) - \ &(region)->pages[0][0])/GC_PAGE_SZ) +NOINLINE static uintptr_t gc_get_stack_ptr() +{ + void *dummy = NULL; + // The mask is to suppress the compiler warning about returning + // address of local variable + return (uintptr_t)&dummy & ~(uintptr_t)15; +} + #include "gc-debug.c" int jl_in_gc; // referenced from switchto task.c @@ -1975,6 +1985,7 @@ void jl_gc_collect(int full) { if (!is_gc_enabled) return; if (jl_in_gc) return; + char *stack_hi = (char*)gc_get_stack_ptr(); gc_debug_print(); JL_SIGATOMIC_BEGIN(); jl_in_gc = 1; @@ -2076,6 +2087,7 @@ void jl_gc_collect(int full) #endif estimate_freed = live_bytes - scanned_bytes - perm_scanned_bytes + actual_allocd; + gc_scrub(stack_hi); gc_verify(); #if defined(MEMPROFILE)