On Sat, 15 Nov 2014 19:49:12 -0500 Brad Smith <[email protected]> wrote: > On 02/20/14 20:11, Brad Smith wrote: > > On 31/01/14 9:10 PM, Philip Guenther wrote: > >> On Wed, 29 Jan 2014, Sviatoslav Chagaev wrote: > >>> When unloading, ld.so removes elements from grpref list too soon, not > >>> allowing code which runs later to destroy objects in the list. Next time > >>> we dlopen, there are undead objects with e.g. freed elements in child > >>> list. Sooner or later something bad happens, in my case div by zero. > >>> > >>> Patch: > >> > >> Your patch reverts this commit: > >> > >> ---------------------------- > >> revision 1.34 > >> date: 2011/07/13 20:49:44; author: drahn; state: Exp; lines: +4 -2; > >> Delete items on grpreflist when walking them to decrement the count, > >> otherwise double decrement can occur. ok kurt@ timeout on other > >> reviewers. > >> ============================================================================= > >> > >> > >> I'm not sure if there's a regress for the issue Dale was fixing, but > >> simply reverting his fix and reintroducing the original problem isn't > >> much > >> better than where we are now. > >> > >> I'll stare at this some and see if I can see how to avoid leaving group > >> leaders on the global list... > > > > Any update on this? > > Any chance this issue could be looked into? As more projects are adding > support for or switching to SDL2 there has been more interest from > porters to use SDL2 but it is marked broken due to this issue. >
Hi, I propose to remove grpref_list altogether because it doesn't seem to have any real purpose. It's used in two places in code: for debug output (dlfcn.c:227) and when unloading objects to seemingly prevent unloading an object which is in use. The information it outputs can already be inferred from the rest of output and we already have opencount and refcount reference counters which will prevent unload of a used object. Tested on amd64 current, running Firefox, GIMP, Sylpheed, mplayer; the synthetic test doesn't crash anymore and I also built SDL2 from ports and ran a few simple programs which use it (https://github.com/S010/test/tree/master/sdl2), seems to work fine. Index: libexec/ld.so/dlfcn.c =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/dlfcn.c,v retrieving revision 1.89 diff -u -p -r1.89 dlfcn.c --- libexec/ld.so/dlfcn.c 13 Nov 2013 05:41:41 -0000 1.89 +++ libexec/ld.so/dlfcn.c 21 Nov 2014 02:27:48 -0000 @@ -91,7 +91,7 @@ dlopen(const char *libname, int flags) if (OBJECT_REF_CNT(object) > 1) { /* if opened but grpsym_list has not been created */ - if (OBJECT_DLREF_CNT(object) == 1) { + if (object->opencount == 1) { /* add first object manually */ _dl_link_grpsym(object, 1); _dl_cache_grpsym_list(object); @@ -238,9 +238,6 @@ dlctl(void *handle, int command, void *d TAILQ_FOREACH(m, &obj->child_list, next_sib) _dl_printf("\t[%s]\n", m->data->load_name); - _dl_printf(" grpref\n"); - TAILQ_FOREACH(m, &obj->grpref_list, next_sib) - _dl_printf("\t[%s]\n", m->data->load_name); _dl_printf("\n"); } retval = 0; @@ -400,9 +397,6 @@ _dl_tracefmt(int fd, elf_object_t *objec (void *)(object->load_base + object->load_size)); break; - case 'g': - _dl_fdprintf(fd, "%d", object->grprefcount); - break; case 'm': _dl_fdprintf(fd, "%d", object->sod.sod_major); break; @@ -472,12 +466,12 @@ _dl_show_objects(void) pad = ""; fmt1 = _dl_tracefmt1 ? _dl_tracefmt1 : - "\t%x %e %t %O %r %g %p\n"; + "\t%x %e %t %O %r %p\n"; fmt2 = _dl_tracefmt2 ? _dl_tracefmt2 : - "\t%x %e %t %O %r %g %p\n"; + "\t%x %e %t %O %r %p\n"; if (_dl_tracefmt1 == NULL && _dl_tracefmt2 == NULL) - _dl_fdprintf(outputfd, "\tStart %s End %s Type Open Ref GrpRef Name\n", + _dl_fdprintf(outputfd, "\tStart %s End %s Type Open Ref Name\n", pad, pad); if (_dl_tracelib) { Index: libexec/ld.so/library.c =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/library.c,v retrieving revision 1.70 diff -u -p -r1.70 library.c --- libexec/ld.so/library.c 10 Jul 2014 09:03:01 -0000 1.70 +++ libexec/ld.so/library.c 21 Nov 2014 02:27:48 -0000 @@ -66,8 +66,6 @@ _dl_unload_shlib(elf_object_t *object) object->status |= STAT_UNLOADED; TAILQ_FOREACH(n, &object->child_list, next_sib) _dl_unload_shlib(n->data); - TAILQ_FOREACH(n, &object->grpref_list, next_sib) - _dl_unload_shlib(n->data); DL_DEB(("unload_shlib unloading on %s\n", object->load_name)); _dl_load_list_free(object->load_list); _dl_munmap((void *)object->load_base, object->load_size); @@ -111,11 +109,6 @@ _dl_tryload_shlib(const char *libname, i _dl_close(libfile); if (_dl_loading_object == NULL) _dl_loading_object = object; - if (object->load_object != _dl_objects && - object->load_object != _dl_loading_object) { - _dl_link_grpref(object->load_object, - _dl_loading_object); - } return(object); } } Index: libexec/ld.so/library_mquery.c =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/library_mquery.c,v retrieving revision 1.47 diff -u -p -r1.47 library_mquery.c --- libexec/ld.so/library_mquery.c 10 Jul 2014 09:03:01 -0000 1.47 +++ libexec/ld.so/library_mquery.c 21 Nov 2014 02:27:48 -0000 @@ -72,8 +72,6 @@ _dl_unload_shlib(elf_object_t *object) object->status |= STAT_UNLOADED; TAILQ_FOREACH(n, &object->child_list, next_sib) _dl_unload_shlib(n->data); - TAILQ_FOREACH(n, &object->grpref_list, next_sib) - _dl_unload_shlib(n->data); DL_DEB(("unload_shlib unloading on %s\n", object->load_name)); _dl_load_list_free(object->load_list); _dl_remove_object(object); @@ -117,11 +115,6 @@ _dl_tryload_shlib(const char *libname, i _dl_close(libfile); if (_dl_loading_object == NULL) _dl_loading_object = object; - if (object->load_object != _dl_objects && - object->load_object != _dl_loading_object) { - _dl_link_grpref(object->load_object, - _dl_loading_object); - } return(object); } } Index: libexec/ld.so/library_subr.c =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/library_subr.c,v retrieving revision 1.41 diff -u -p -r1.41 library_subr.c --- libexec/ld.so/library_subr.c 10 Jul 2014 09:03:01 -0000 1.41 +++ libexec/ld.so/library_subr.c 21 Nov 2014 02:27:48 -0000 @@ -267,10 +267,6 @@ _dl_find_loaded_shlib(const char *req_na object->obj_flags |= flags & DF_1_GLOBAL; if (_dl_loading_object == NULL) _dl_loading_object = object; - if (object->load_object != _dl_objects && - object->load_object != _dl_loading_object) { - _dl_link_grpref(object->load_object, _dl_loading_object); - } } return (object); @@ -420,7 +416,7 @@ _dl_link_dlopen(elf_object_t *dep) dep->opencount++; - if (OBJECT_DLREF_CNT(dep) > 1) + if (dep->opencount > 1) return; n = _dl_malloc(sizeof *n); @@ -452,15 +448,6 @@ _dl_notify_unload_shlib(elf_object_t *ob if (OBJECT_REF_CNT(object) == 0) TAILQ_FOREACH(n, &object->child_list, next_sib) _dl_child_refcnt_decrement(n->data); - - if (OBJECT_DLREF_CNT(object) == 0) { - while ((n = TAILQ_FIRST(&object->grpref_list)) != NULL) { - TAILQ_REMOVE(&object->grpref_list, n, next_sib); - n->data->grprefcount--; - _dl_notify_unload_shlib(n->data); - _dl_free(n); - } - } } void @@ -479,19 +466,6 @@ _dl_unload_dlopen(void) _dl_run_all_dtors(); } } -} - -void -_dl_link_grpref(elf_object_t *load_group, elf_object_t *load_object) -{ - struct dep_node *n; - - n = _dl_malloc(sizeof *n); - if (n == NULL) - _dl_exit(7); - n->data = load_group; - TAILQ_INSERT_TAIL(&load_object->grpref_list, n, next_sib); - load_group->grprefcount++; } void Index: libexec/ld.so/resolve.c =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/resolve.c,v retrieving revision 1.67 diff -u -p -r1.67 resolve.c --- libexec/ld.so/resolve.c 10 Jul 2014 09:03:01 -0000 1.67 +++ libexec/ld.so/resolve.c 21 Nov 2014 02:27:48 -0000 @@ -342,13 +342,11 @@ _dl_finalize_object(const char *objname, object->refcount = 0; TAILQ_INIT(&object->child_list); object->opencount = 0; /* # dlopen() & exe */ - object->grprefcount = 0; /* default dev, inode for dlopen-able objects. */ object->dev = 0; object->inode = 0; object->lastlookup = 0; TAILQ_INIT(&object->grpsym_list); - TAILQ_INIT(&object->grpref_list); if (object->dyn.rpath) { object->rpath = _dl_split_path(object->dyn.rpath); @@ -384,7 +382,7 @@ _dl_cleanup_objects() n = TAILQ_FIRST(&_dlopened_child_list); while (n != NULL) { next = TAILQ_NEXT(n, next_sib); - if (OBJECT_DLREF_CNT(n->data) == 0) { + if (n->data->opencount == 0) { TAILQ_REMOVE(&_dlopened_child_list, n, next_sib); _dl_free(n); } @@ -402,7 +400,6 @@ _dl_cleanup_objects() _dl_free_path(head->rpath); _dl_tailq_free(TAILQ_FIRST(&head->grpsym_list)); _dl_tailq_free(TAILQ_FIRST(&head->child_list)); - _dl_tailq_free(TAILQ_FIRST(&head->grpref_list)); nobj = head->next; _dl_free(head); head = nobj; Index: libexec/ld.so/resolve.h =================================================================== RCS file: /OpenBSD/src/libexec/ld.so/resolve.h,v retrieving revision 1.70 diff -u -p -r1.70 resolve.h --- libexec/ld.so/resolve.h 13 Nov 2013 05:41:42 -0000 1.70 +++ libexec/ld.so/resolve.h 21 Nov 2014 02:27:48 -0000 @@ -129,15 +129,11 @@ struct elf_object { TAILQ_HEAD(,dep_node) child_list; /* direct dep libs of object */ TAILQ_HEAD(,dep_node) grpsym_list; /* ordered complete dep list */ - TAILQ_HEAD(,dep_node) grpref_list; /* refs to other load groups */ int refcount; /* dep libs only */ int opencount; /* # dlopen() & exe */ - int grprefcount; /* load group refs */ #define OBJECT_REF_CNT(object) \ - ((object->refcount + object->opencount + object->grprefcount)) -#define OBJECT_DLREF_CNT(object) \ - ((object->opencount + object->grprefcount)) + ((object->refcount + object->opencount)) /* object that caused this module to be loaded, used in symbol lookup */ elf_object_t *load_object; @@ -218,7 +214,6 @@ void _dl_link_child(elf_object_t *dep, e void _dl_link_grpsym(elf_object_t *object, int checklist); void _dl_cache_grpsym_list(elf_object_t *object); void _dl_cache_grpsym_list_setup(elf_object_t *object); -void _dl_link_grpref(elf_object_t *load_group, elf_object_t *load_object); void _dl_link_dlopen(elf_object_t *dep); void _dl_unlink_dlopen(elf_object_t *dep); void _dl_notify_unload_shlib(elf_object_t *object);
