From 7810b4c37c7d03772215102a8db5e84d29fe2221 Mon Sep 17 00:00:00 2001 From: Ben Gamari <ben@smart-cactus.org> Date: Fri, 8 Mar 2024 11:58:58 -0500 Subject: [PATCH] rts/linker: Don't unload native objects when dlinfo isn't available To do so is unsafe as we have no way of identifying references to symbols provided by the object. Fixes #24513. Fixes #23993. --- rts/CheckUnload.c | 15 +++++++++++---- rts/Linker.c | 2 ++ rts/LinkerInternals.h | 10 +++++++++- rts/linker/Elf.c | 4 ++++ .../linker_unload_multiple_objs.c | 2 +- 5 files changed, 27 insertions(+), 6 deletions(-) diff --git a/rts/CheckUnload.c b/rts/CheckUnload.c index e7e04bcd388d..e787f2d68afb 100644 --- a/rts/CheckUnload.c +++ b/rts/CheckUnload.c @@ -492,8 +492,6 @@ void checkUnload(void) next = oc->next; ASSERT(oc->status == OBJECT_UNLOADED); - removeOCSectionIndices(s_indices, oc); - // Symbols should be removed by unloadObj_. // NB (osa): If this assertion doesn't hold then freeObjectCode below // will corrupt symhash as keys of that table live in ObjectCodes. If @@ -501,8 +499,17 @@ void checkUnload(void) // RTS) then it's probably because this assertion did not hold. ASSERT(oc->symbols == NULL); - freeObjectCode(oc); - n_unloaded_objects -= 1; + if (oc->unloadable) { + removeOCSectionIndices(s_indices, oc); + freeObjectCode(oc); + n_unloaded_objects -= 1; + } else { + // If we don't have enough information to + // accurately determine the reachability of + // the object then hold onto it. + oc->next = objects; + objects = oc; + } } old_objects = NULL; diff --git a/rts/Linker.c b/rts/Linker.c index 66fbf7907028..64f54cf0eaa2 100644 --- a/rts/Linker.c +++ b/rts/Linker.c @@ -1385,6 +1385,8 @@ mkOc( ObjectType type, pathchar *path, char *image, int imageSize, oc->prev = NULL; oc->next_loaded_object = NULL; oc->mark = object_code_mark_bit; + /* this will get cleared by the caller if object is not safely unloadable */ + oc->unloadable = true; oc->dependencies = allocHashSet(); #if defined(NEED_M32) diff --git a/rts/LinkerInternals.h b/rts/LinkerInternals.h index 271611a249da..cbfd3ef686c3 100644 --- a/rts/LinkerInternals.h +++ b/rts/LinkerInternals.h @@ -313,8 +313,14 @@ struct _ObjectCode { struct _ObjectCode *next_loaded_object; // Mark bit + // N.B. This is a full word as we CAS it. StgWord mark; + // Can this object be safely unloaded? Not true for + // dynamic objects when dlinfo is not available as + // we cannot determine liveness. + bool unloadable; + // Set of dependencies (ObjectCode*) of the object file. Traverse // dependencies using `iterHashTable`. // @@ -376,7 +382,9 @@ struct _ObjectCode { /* handle returned from dlopen */ void *dlopen_handle; - /* virtual memory ranges of loaded code */ + /* virtual memory ranges of loaded code. NULL if no range information is + * available (e.g. if dlinfo is unavailable on the current platform). + */ NativeCodeRange *nc_ranges; }; diff --git a/rts/linker/Elf.c b/rts/linker/Elf.c index b0acfcfb5469..f668cfe31338 100644 --- a/rts/linker/Elf.c +++ b/rts/linker/Elf.c @@ -2186,6 +2186,10 @@ void * loadNativeObj_ELF (pathchar *path, char **errmsg) copyErrmsg(errmsg, "dl_iterate_phdr failed to find obj"); goto dl_iterate_phdr_fail; } + nc->unloadable = true; +#else + nc->nc_ranges = NULL; + nc->unloadable = false; #endif /* defined (HAVE_DLINFO) */ insertOCSectionIndices(nc); diff --git a/testsuite/tests/rts/linker/unload_multiple_objs/linker_unload_multiple_objs.c b/testsuite/tests/rts/linker/unload_multiple_objs/linker_unload_multiple_objs.c index b076809a6b15..910663caa57a 100644 --- a/testsuite/tests/rts/linker/unload_multiple_objs/linker_unload_multiple_objs.c +++ b/testsuite/tests/rts/linker/unload_multiple_objs/linker_unload_multiple_objs.c @@ -64,7 +64,7 @@ void check_object_freed(char *obj_path) { OStatus st; st = getObjectLoadStatus(toPathchar(obj_path)); if (st != OBJECT_NOT_LOADED) { - errorBelch("object %s status != OBJECT_NOT_LOADED", obj_path); + errorBelch("object %s status != OBJECT_NOT_LOADED, is %d instead", obj_path, st); exit(1); } } -- GitLab