From 7d41ef96b2a1cf0ebd342de528204647d78c28dc Mon Sep 17 00:00:00 2001 From: ivmai Date: Thu, 10 Sep 2009 17:38:06 +0000 Subject: [PATCH] 2009-09-10 Ivan Maidanski (diff113) * include/gc.h (GC_has_static_roots_func): New typedef (user filter callback). * include/gc.h (GC_register_has_static_roots_callback): Use GC_has_static_roots_func type. * dyn_load.c (GC_has_static_roots, GC_register_has_static_roots_callback): Ditto. * dyn_load.c (GC_has_static_roots, GC_register_has_static_roots_callback): Define on all platforms. * dyn_load.c (GC_register_dynlib_callback, GC_register_dynamic_libraries, GC_init_dyld): Replace K&R-style functions definition with the ANSI C one. * dyn_load.c (GC_register_dynlib_callback): Use new local variable "callback" (initialized from GC_has_static_roots) to minimize data races. * dyn_load.c (GC_register_dynamic_libraries_dl_iterate_phdr, GC_cond_add_roots): Define as STATIC. * mark_rts.c (GC_remove_roots_inner): Ditto. * dyn_load.c (GC_dyld_image_add): Don't call GC_add_roots() for sections smaller than pointer size (just to avoid acquiring the lock unnecessarily). * dyn_load.c (GC_dyld_name_for_hdr): Define unconditionally (not only for DARWIN_DEBUG). * dyn_load.c (GC_dyld_image_add): Replace GC_add_roots() call with LOCK + GC_add_roots_inner() + UNLOCK. * dyn_load.c (GC_dyld_image_add): Call GC_has_static_roots() user callback (if set) holding the lock; if it returns 0 then don't call GC_add_roots_inner() for that region. * dyn_load.c (GC_register_has_static_roots_callback): Put "callback" value to GC_has_static_roots on all platforms. * dyn_load.c (GC_has_static_roots): Update the comments. * include/gc.h (GC_exclude_static_roots, GC_add_roots, GC_remove_roots, GC_register_has_static_roots_callback): Ditto. * include/private/gc_priv.h (struct roots): Ditto. * include/private/gc_priv.h (GC_remove_roots_inner): Move prototype to mark_rts.c and declare it as STATIC. * include/private/gc_priv.h (GC_exclude_static_roots_inner): New prototype. * dyn_load.c (GC_register_dynamic_libraries_dl_iterate_phdr): Use GC_exclude_static_roots_inner() instead of GC_exclude_static_roots. * misc.c (GC_init_inner): Ditto. * mark_rts.c (GC_exclude_static_roots_inner): New function (move all the code from GC_exclude_static_roots(); add the comment. * mark_rts.c (GC_add_roots_inner, GC_exclude_static_roots_inner): add alignment assertion for the lower bound; add assertion for the lower bound to be less than the upper one. * mark_rts.c (GC_add_roots_inner, GC_exclude_static_roots): Adjust the upper bound (round down to be of a pointer-aligned value); return in case of an empty range. * mark_rts.c (GC_exclude_static_roots): Acquire the lock and call GC_exclude_static_roots_inner(). * mark_rts.c (GC_remove_roots): Quickly check the bounds and return in case of a do-nothing case (before acquiring the lock). --- ChangeLog | 56 ++++++++++++++++++++++++++++ dyn_load.c | 78 ++++++++++++++++++++++----------------- include/gc.h | 32 ++++++++++++---- include/private/gc_priv.h | 6 +-- mark_rts.c | 36 +++++++++++++++++- misc.c | 8 ++-- 6 files changed, 165 insertions(+), 51 deletions(-) diff --git a/ChangeLog b/ChangeLog index e1d02da5..06eed467 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,60 @@ +2009-09-10 Ivan Maidanski + (diff113) + + * include/gc.h (GC_has_static_roots_func): New typedef (user filter + callback). + * include/gc.h (GC_register_has_static_roots_callback): Use + GC_has_static_roots_func type. + * dyn_load.c (GC_has_static_roots, + GC_register_has_static_roots_callback): Ditto. + * dyn_load.c (GC_has_static_roots, + GC_register_has_static_roots_callback): Define on all platforms. + * dyn_load.c (GC_register_dynlib_callback, + GC_register_dynamic_libraries, GC_init_dyld): Replace K&R-style + functions definition with the ANSI C one. + * dyn_load.c (GC_register_dynlib_callback): Use new local variable + "callback" (initialized from GC_has_static_roots) to minimize data + races. + * dyn_load.c (GC_register_dynamic_libraries_dl_iterate_phdr, + GC_cond_add_roots): Define as STATIC. + * mark_rts.c (GC_remove_roots_inner): Ditto. + * dyn_load.c (GC_dyld_image_add): Don't call GC_add_roots() for + sections smaller than pointer size (just to avoid acquiring the + lock unnecessarily). + * dyn_load.c (GC_dyld_name_for_hdr): Define unconditionally (not + only for DARWIN_DEBUG). + * dyn_load.c (GC_dyld_image_add): Replace GC_add_roots() call with + LOCK + GC_add_roots_inner() + UNLOCK. + * dyn_load.c (GC_dyld_image_add): Call GC_has_static_roots() user + callback (if set) holding the lock; if it returns 0 then don't call + GC_add_roots_inner() for that region. + * dyn_load.c (GC_register_has_static_roots_callback): Put + "callback" value to GC_has_static_roots on all platforms. + * dyn_load.c (GC_has_static_roots): Update the comments. + * include/gc.h (GC_exclude_static_roots, GC_add_roots, + GC_remove_roots, GC_register_has_static_roots_callback): Ditto. + * include/private/gc_priv.h (struct roots): Ditto. + * include/private/gc_priv.h (GC_remove_roots_inner): Move prototype + to mark_rts.c and declare it as STATIC. + * include/private/gc_priv.h (GC_exclude_static_roots_inner): New + prototype. + * dyn_load.c (GC_register_dynamic_libraries_dl_iterate_phdr): Use + GC_exclude_static_roots_inner() instead of GC_exclude_static_roots. + * misc.c (GC_init_inner): Ditto. + * mark_rts.c (GC_exclude_static_roots_inner): New function (move + all the code from GC_exclude_static_roots(); add the comment. + * mark_rts.c (GC_add_roots_inner, GC_exclude_static_roots_inner): + add alignment assertion for the lower bound; add assertion for the + lower bound to be less than the upper one. + * mark_rts.c (GC_add_roots_inner, GC_exclude_static_roots): Adjust + the upper bound (round down to be of a pointer-aligned value); + return in case of an empty range. + * mark_rts.c (GC_exclude_static_roots): Acquire the lock and call + GC_exclude_static_roots_inner(). + * mark_rts.c (GC_remove_roots): Quickly check the bounds and return + in case of a do-nothing case (before acquiring the lock). + 2009-09-10 Ivan Maidanski (diff112) diff --git a/dyn_load.c b/dyn_load.c index b46ba040..7960a53f 100644 --- a/dyn_load.c +++ b/dyn_load.c @@ -50,8 +50,15 @@ # undef GC_must_restore_redefined_dlopen # endif +/* A user-supplied routine (custom filter) that might be called to */ +/* determine whether a DSO really needs to be scanned by the GC. */ +/* 0 means no filter installed. May be unused on some platforms. */ +/* FIXME: Add filter support for more platforms. */ +STATIC GC_has_static_roots_func GC_has_static_roots = 0; + #if (defined(DYNAMIC_LOADING) || defined(MSWIN32) || defined(MSWINCE)) \ && !defined(PCR) + #if !defined(SOLARISDL) && !defined(IRIX5) && \ !defined(MSWIN32) && !defined(MSWINCE) && \ !(defined(ALPHA) && defined(OSF1)) && \ @@ -413,14 +420,8 @@ static int n_load_segs; # endif /* PT_GNU_RELRO */ -/* A user-supplied routine that is called to determine if a DSO must - be scanned by the gc. */ -static int (GC_CALLBACK * GC_has_static_roots)(const char *, void *, size_t); - -static int GC_register_dynlib_callback(info, size, ptr) - struct dl_phdr_info * info; - size_t size; - void * ptr; +static int GC_register_dynlib_callback(struct dl_phdr_info * info, + size_t size, void * ptr) { const ElfW(Phdr) * p; ptr_t start, end; @@ -468,12 +469,12 @@ static int GC_register_dynlib_callback(info, size, ptr) case PT_LOAD: { + GC_has_static_roots_func callback = GC_has_static_roots; if( !(p->p_flags & PF_W) ) break; start = ((char *)(p->p_vaddr)) + info->dlpi_addr; end = start + p->p_memsz; - if (GC_has_static_roots - && !GC_has_static_roots(info->dlpi_name, start, p->p_memsz)) + if (callback != 0 && !callback(info->dlpi_name, start, p->p_memsz)) break; # ifdef PT_GNU_RELRO if (n_load_segs >= MAX_LOAD_SEGS) ABORT("Too many PT_LOAD segs"); @@ -498,7 +499,7 @@ static int GC_register_dynlib_callback(info, size, ptr) /* Return TRUE if we succeed, FALSE if dl_iterate_phdr wasn't there. */ -GC_bool GC_register_dynamic_libraries_dl_iterate_phdr(void) +STATIC GC_bool GC_register_dynamic_libraries_dl_iterate_phdr(void) { if (dl_iterate_phdr) { int did_something = 0; @@ -507,8 +508,8 @@ GC_bool GC_register_dynamic_libraries_dl_iterate_phdr(void) static GC_bool excluded_segs = FALSE; n_load_segs = 0; if (!excluded_segs) { - GC_exclude_static_roots((ptr_t)load_segs, - (ptr_t)load_segs + sizeof(load_segs)); + GC_exclude_static_roots_inner((ptr_t)load_segs, + (ptr_t)load_segs + sizeof(load_segs)); excluded_segs = TRUE; } # endif @@ -797,7 +798,7 @@ void GC_register_dynamic_libraries(void) extern void GC_get_next_stack(char *start, char * limit, char **lo, char **hi); - void GC_cond_add_roots(char *base, char * limit) + STATIC void GC_cond_add_roots(char *base, char * limit) { char * curr_base = base; char * next_stack_lo; @@ -814,7 +815,7 @@ void GC_register_dynamic_libraries(void) if (curr_base < limit) GC_add_roots_inner(curr_base, limit, TRUE); } # else - void GC_cond_add_roots(char *base, char * limit) + STATIC void GC_cond_add_roots(char *base, char * limit) { char dummy; char * stack_top @@ -1034,7 +1035,7 @@ void GC_register_dynamic_libraries(void) extern char *sys_errlist[]; extern int sys_nerr; -void GC_register_dynamic_libraries() +void GC_register_dynamic_libraries(void) { int status; int index = 1; /* Ordinal position in shared library search list */ @@ -1144,7 +1145,6 @@ const static struct { { SEG_DATA, SECT_COMMON } }; -#ifdef DARWIN_DEBUG static const char *GC_dyld_name_for_hdr(const struct GC_MACH_HEADER *hdr) { unsigned long i,c; c = _dyld_image_count(); @@ -1152,25 +1152,37 @@ static const char *GC_dyld_name_for_hdr(const struct GC_MACH_HEADER *hdr) { return _dyld_get_image_name(i); return NULL; } -#endif /* This should never be called by a thread holding the lock */ static void GC_dyld_image_add(const struct GC_MACH_HEADER *hdr, intptr_t slide) { unsigned long start,end,i; const struct GC_MACH_SECTION *sec; + const char *name; + GC_has_static_roots_func callback = GC_has_static_roots; + DCL_LOCK_STATE; if (GC_no_dls) return; +# ifdef DARWIN_DEBUG + name = GC_dyld_name_for_hdr(hdr); +# else + name = callback != 0 ? GC_dyld_name_for_hdr(hdr) : NULL; +# endif for(i=0;isize == 0) continue; + if(sec == NULL || sec->size < sizeof(word)) continue; start = slide + sec->addr; end = start + sec->size; -# ifdef DARWIN_DEBUG - GC_printf("Adding section at %p-%p (%lu bytes) from image %s\n", - start,end,sec->size,GC_dyld_name_for_hdr(hdr)); -# endif - GC_add_roots((char*)start,(char*)end); + LOCK(); + /* The user callback is called holding the lock */ + if (callback == 0 || callback(name, (void*)start, (size_t)sec->size)) { +# ifdef DARWIN_DEBUG + GC_printf("Adding section at %p-%p (%lu bytes) from image %s\n", + start,end,sec->size,name); +# endif + GC_add_roots_inner((ptr_t)start, (ptr_t)end, FALSE); + } + UNLOCK(); } # ifdef DARWIN_DEBUG GC_print_static_roots(); @@ -1200,7 +1212,7 @@ static void GC_dyld_image_remove(const struct GC_MACH_HEADER *hdr, # endif } -void GC_register_dynamic_libraries() { +void GC_register_dynamic_libraries(void) { /* Currently does nothing. The callbacks are setup by GC_init_dyld() The dyld library takes it from there. */ } @@ -1211,7 +1223,7 @@ void GC_register_dynamic_libraries() { This should be called BEFORE any thread in created and WITHOUT the allocation lock held. */ -void GC_init_dyld() { +void GC_init_dyld(void) { static GC_bool initialized = FALSE; char *bind_fully_env = NULL; @@ -1301,7 +1313,7 @@ void GC_register_dynamic_libraries(void) #else /* !PCR */ -void GC_register_dynamic_libraries(){} +void GC_register_dynamic_libraries(void) {} #endif /* !PCR */ @@ -1315,13 +1327,11 @@ GC_bool GC_register_main_static_data(void) return TRUE; } +#endif /* HAVE_REGISTER_MAIN_STATIC_DATA */ + /* Register a routine to filter dynamic library registration. */ -GC_API void GC_CALL GC_register_has_static_roots_callback - (int (GC_CALLBACK * callback)(const char *, void *, size_t)) { -# ifdef HAVE_DL_ITERATE_PHDR +GC_API void GC_CALL GC_register_has_static_roots_callback( + GC_has_static_roots_func callback) +{ GC_has_static_roots = callback; -# endif } - -#endif /* HAVE_REGISTER_MAIN_STATIC_DATA */ - diff --git a/include/gc.h b/include/gc.h index c62b1485..3a0f95e2 100644 --- a/include/gc.h +++ b/include/gc.h @@ -374,6 +374,8 @@ GC_API void GC_CALL GC_set_max_heap_size(GC_word n); /* need not be scanned. This is sometimes important if the application */ /* maps large read/write files into the address space, which could be */ /* mistaken for dynamic library data segments on some systems. */ +/* The section (referred to by low_address) must be pointer-aligned. */ +/* low_address must not be greater than high_address_plus_1. */ GC_API void GC_CALL GC_exclude_static_roots(void * low_address, void * high_address_plus_1); @@ -381,10 +383,13 @@ GC_API void GC_CALL GC_exclude_static_roots(void * low_address, GC_API void GC_CALL GC_clear_roots(void); /* Add a root segment. Wizards only. */ +/* The segment (referred to by low_address) must be pointer-aligned. */ +/* low_address must not be greater than high_address_plus_1. */ GC_API void GC_CALL GC_add_roots(void * low_address, void * high_address_plus_1); /* Remove a root segment. Wizards only. */ +/* May be unimplemented on some platforms. */ GC_API void GC_CALL GC_remove_roots(void * low_address, void * high_address_plus_1); @@ -1079,14 +1084,25 @@ GC_API void * GC_CALL GC_malloc_many(size_t lb); #endif /* THREADS */ -/* Register a callback to control the scanning of dynamic libraries. - When the GC scans the static data of a dynamic library, it will - first call a user-supplied routine with filename of the library and - the address and length of the memory region. This routine should - return nonzero if that region should be scanned. */ -GC_API void GC_CALL GC_register_has_static_roots_callback - (int (GC_CALLBACK * callback)(const char *, void *, size_t)); - +/* A filter function to control the scanning of dynamic libraries. */ +/* If implemented, called by GC before registering a dynamic library */ +/* (discovered by GC) section as a static data root (called only as */ +/* a last reason not to register). The filename of the library, the */ +/* address and the length of the memory region (section) are passed. */ +/* This routine should return nonzero if that region should be scanned. */ +/* Always called with the allocation lock held. Depending on the */ +/* platform, might be called with the "world" stopped. */ +typedef int (GC_CALLBACK * GC_has_static_roots_func)( + const char * /* dlpi_name */, + void * /* section_start */, + size_t /* section_size */); + +/* Register a new callback (a user-supplied filter) to control the */ +/* scanning of dynamic libraries. Replaces any previously registered */ +/* callback. May be 0 (means no filtering). May be unused on some */ +/* platforms (if the filtering is unimplemented or inappropriate). */ +GC_API void GC_CALL GC_register_has_static_roots_callback( + GC_has_static_roots_func); #if defined(GC_WIN32_THREADS) && !defined(__CYGWIN32__) \ && !defined(__CYGWIN__) \ diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h index e2788a9c..3976ddd8 100644 --- a/include/private/gc_priv.h +++ b/include/private/gc_priv.h @@ -832,8 +832,8 @@ struct exclusion { /* Under Win32, we need to do a better job of filtering overlaps, so */ /* we resort to sequential search, and pay the price. */ struct roots { - ptr_t r_start; - ptr_t r_end; + ptr_t r_start;/* multiple of word size */ + ptr_t r_end; /* multiple of word size and greater than r_start */ # if !defined(MSWIN32) && !defined(MSWINCE) struct roots * r_next; # endif @@ -1470,7 +1470,7 @@ void GC_set_fl_marks(ptr_t p); /* set. Abort if not. */ #endif void GC_add_roots_inner(ptr_t b, ptr_t e, GC_bool tmp); -void GC_remove_roots_inner(ptr_t b, ptr_t e); +void GC_exclude_static_roots_inner(void *start, void *finish); GC_bool GC_is_static_root(ptr_t p); /* Is the address p in one of the registered static */ /* root sections? */ diff --git a/mark_rts.c b/mark_rts.c index 3554627a..830110fb 100644 --- a/mark_rts.c +++ b/mark_rts.c @@ -158,6 +158,12 @@ GC_API void GC_CALL GC_add_roots(void *b, void *e) void GC_add_roots_inner(ptr_t b, ptr_t e, GC_bool tmp) { struct roots * old; + + /* Adjust and check range boundaries for safety */ + GC_ASSERT((word)b % sizeof(word) == 0); + e = (ptr_t)((word)e & ~(sizeof(word) - 1)); + GC_ASSERT(b <= e); + if (b == e) return; /* nothing to do? */ # if defined(MSWIN32) || defined(MSWINCE) /* Spend the time to ensure that there are no overlapping */ @@ -299,9 +305,16 @@ STATIC void GC_remove_tmp_roots(void) #endif #if !defined(MSWIN32) && !defined(MSWINCE) +STATIC void GC_remove_roots_inner(ptr_t b, ptr_t e); + GC_API void GC_CALL GC_remove_roots(void *b, void *e) { DCL_LOCK_STATE; + + /* Quick check whether has nothing to do */ + if ((((word)b + (sizeof(word) - 1)) & ~(sizeof(word) - 1)) >= + ((word)e & ~(sizeof(word) - 1))) + return; LOCK(); GC_remove_roots_inner((ptr_t)b, (ptr_t)e); @@ -309,7 +322,7 @@ GC_API void GC_CALL GC_remove_roots(void *b, void *e) } /* Should only be called when the lock is held */ -void GC_remove_roots_inner(ptr_t b, ptr_t e) +STATIC void GC_remove_roots_inner(ptr_t b, ptr_t e) { int i; for (i = 0; i < n_root_sets; ) { @@ -396,11 +409,16 @@ STATIC struct exclusion * GC_next_exclusion(ptr_t start_addr) return GC_excl_table + low; } -GC_API void GC_CALL GC_exclude_static_roots(void *start, void *finish) +/* Should only be called when the lock is held. The range boundaries */ +/* should be properly aligned and valid. */ +void GC_exclude_static_roots_inner(void *start, void *finish) { struct exclusion * next; size_t next_index, i; + GC_ASSERT((word)start % sizeof(word) == 0); + GC_ASSERT(start < finish); + if (0 == GC_excl_table_entries) { next = 0; } else { @@ -429,6 +447,20 @@ GC_API void GC_CALL GC_exclude_static_roots(void *start, void *finish) ++GC_excl_table_entries; } +GC_API void GC_CALL GC_exclude_static_roots(void *b, void *e) +{ + DCL_LOCK_STATE; + + /* Adjust the upper boundary for safety (round down) */ + e = (void *)((word)e & ~(sizeof(word) - 1)); + + if (b == e) return; /* nothing to exclude? */ + + LOCK(); + GC_exclude_static_roots_inner(b, e); + UNLOCK(); +} + /* Invoke push_conditional on ranges that are not excluded. */ /*ARGSUSED*/ STATIC void GC_push_conditional_with_exclusions(ptr_t bottom, ptr_t top, diff --git a/misc.c b/misc.c index d8fc82b5..142b2766 100644 --- a/misc.c +++ b/misc.c @@ -650,11 +650,11 @@ void GC_init_inner(void) GC_obj_kinds[NORMAL].ok_descriptor = ((word)(-ALIGNMENT) | GC_DS_LENGTH); } GC_setpagesize(); - GC_exclude_static_roots(beginGC_arrays, endGC_arrays); - GC_exclude_static_roots(beginGC_obj_kinds, endGC_obj_kinds); + GC_exclude_static_roots_inner(beginGC_arrays, endGC_arrays); + GC_exclude_static_roots_inner(beginGC_obj_kinds, endGC_obj_kinds); # ifdef SEPARATE_GLOBALS - GC_exclude_static_roots(beginGC_objfreelist, endGC_objfreelist); - GC_exclude_static_roots(beginGC_aobjfreelist, endGC_aobjfreelist); + GC_exclude_static_roots_inner(beginGC_objfreelist, endGC_objfreelist); + GC_exclude_static_roots_inner(beginGC_aobjfreelist, endGC_aobjfreelist); # endif # ifdef MSWIN32 GC_init_win32(); -- 2.49.0