diff --git a/core/loader_shared.c b/core/loader_shared.c index 8c9233db577..8e7d68fb00a 100644 --- a/core/loader_shared.c +++ b/core/loader_shared.c @@ -178,6 +178,7 @@ loader_init_prologue(void) privmod_t *mod = privload_insert(NULL, privmod_static[i].base, privmod_static[i].size, privmod_static[i].name, privmod_static[i].path); + mod->is_top_level_client = true; mod->is_client = true; } @@ -520,6 +521,7 @@ privload_insert(privmod_t *after, app_pc base, size_t size, const char *name, } mod->ref_count = 1; mod->externally_loaded = false; + mod->is_top_level_client = false; /* up to caller to set later */ mod->is_client = false; /* up to caller to set later */ mod->called_proc_entry = false; mod->called_proc_exit = false; diff --git a/core/module_shared.h b/core/module_shared.h index 0cc997c8d8f..a9f64e23a11 100644 --- a/core/module_shared.h +++ b/core/module_shared.h @@ -413,7 +413,9 @@ typedef struct _privmod_t { char path[MAXIMUM_PATH]; uint ref_count; bool externally_loaded; - bool is_client; /* or Extension */ + /* XXX i#6982: Perhaps replace is_client with is_top_level_client. */ + bool is_top_level_client; /* set for command-line clients */ + bool is_client; /* set for command-line client or extension */ bool called_proc_entry; bool called_proc_exit; struct _privmod_t *next; diff --git a/core/unix/loader.c b/core/unix/loader.c index 2794dde0448..f011287725a 100644 --- a/core/unix/loader.c +++ b/core/unix/loader.c @@ -581,9 +581,9 @@ privload_process_imports(privmod_t *mod) SYSLOG_INTERNAL_WARNING( "private libpthread.so loaded but not fully supported (i#956)"); } - /* i#852: identify all libs that import from DR as client libs. - * XXX: this code seems stale as libdynamorio.so is already loaded - * (xref #3850). + /* i#852: Identify all libs that import from DR as client libs. + * XXX i#6982: The following condition is never true as + * libdynamorio.so has already been loaded (xref #3850). */ if (impmod->base == get_dynamorio_dll_start()) mod->is_client = true; diff --git a/core/unix/module_elf.c b/core/unix/module_elf.c index 75c943d3d8c..2a3ae3977cc 100644 --- a/core/unix/module_elf.c +++ b/core/unix/module_elf.c @@ -1104,7 +1104,6 @@ module_lookup_symbol(ELF_SYM_TYPE *sym, os_privmod_data_t *pd) { app_pc res; const char *name; - privmod_t *mod; bool is_ifunc; dcontext_t *dcontext = get_thread_private_dcontext(); @@ -1133,11 +1132,21 @@ module_lookup_symbol(ELF_SYM_TYPE *sym, os_privmod_data_t *pd) * FIXME: i#461 We do not tell weak/global, but return on the first we see. */ ASSERT_OWN_RECURSIVE_LOCK(true, &privload_lock); - mod = privload_first_module(); /* FIXME i#3850: Symbols are currently looked up following the dependency chain * depth-first instead of breadth-first. */ - while (mod != NULL) { + for (privmod_t *mod = privload_first_module(); mod != NULL; + mod = privload_next_module(mod)) { + /* Skip other client modules at this point because some will not be + * initialised and clients should be leaves of the dependency tree and + * not provide symbols for other modules. Skipping just the uninitialised + * client modules should also work but might introduce an element of + * unpredictability if we are unsure in what order modules will be + * initialised. Skipping all uninitialised modules should also work but + * might hide a more serious problem. See i#4501. + */ + if (mod->is_top_level_client) + continue; pd = mod->os_privmod_data; ASSERT(pd != NULL && name != NULL); @@ -1172,7 +1181,6 @@ module_lookup_symbol(ELF_SYM_TYPE *sym, os_privmod_data_t *pd) } return res; } - mod = privload_next_module(mod); } return NULL; } diff --git a/suite/tests/CMakeLists.txt b/suite/tests/CMakeLists.txt index dc55f029ab1..349eaf786a9 100644 --- a/suite/tests/CMakeLists.txt +++ b/suite/tests/CMakeLists.txt @@ -3647,6 +3647,19 @@ if (BUILD_SAMPLES) "CFLAGS=-m32;CXXFLAGS=-m32") endif () endif () + if (UNIX) + # XXX: Change this to go through torun() like the other tests to share + # output, multiplexing, _timeout, etc. features: not entirely + # straightforward because this test uses two clients. + # For now, "-s 90 -quiet -killpg" is added explicitly. + get_target_path_for_execution(drrun_path drrun "${location_suffix}") + get_client_path(client1 bbcount bbcount) + get_client_path(client2 opcodes opcodes) + get_target_path_for_execution(app_path "${ci_shared_app}" "${location_suffix}") + add_test(two_clients ${drrun_path} -s 90 -quiet -killpg + -client ${client1} 0 '' + -client ${client2} 1 '' ${dr_test_ops} -- ${app_path}) + endif (UNIX) endif (BUILD_SAMPLES) if (BUILD_CLIENTS)