From 0199c34b2d01a7ca66d5ba4323556f9abcc03330 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Fri, 24 Jul 2015 00:18:02 +0200 Subject: [PATCH 1/7] add test for module collision detection --- test/shared/Makefile | 15 ++++++++++++++- test/shared/src/lib.d | 2 ++ test/shared/src/link_mod_collision.d | 5 +++++ test/shared/src/load_mod_collision.d | 11 +++++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 test/shared/src/link_mod_collision.d create mode 100644 test/shared/src/load_mod_collision.d diff --git a/test/shared/Makefile b/test/shared/Makefile index f25dd809a72..7e16456d5e6 100644 --- a/test/shared/Makefile +++ b/test/shared/Makefile @@ -4,15 +4,21 @@ include ../common.mak TESTS:=link load linkD linkDR loadDR host finalize TESTS+=link_linkdep load_linkdep link_loaddep load_loaddep load_13414 +TESTS+=link_mod_collision load_mod_collision .PHONY: all clean all: $(addprefix $(ROOT)/,$(addsuffix .done,$(TESTS))) $(ROOT)/loadDR.done: RUN_ARGS:=$(DRUNTIMESO) +$(ROOT)/%_mod_collision.done: $(ROOT)/%_mod_collision + @echo Testing $*_mod_collision + $(QUIET)($< $(RUN_ARGS) 2>&1 || true) | grep -qF 'already defined' + @touch $@ + $(ROOT)/%.done: $(ROOT)/% @echo Testing $* - $(QUIET)$(ROOT)/$* $(RUN_ARGS) + $(QUIET)$< $(RUN_ARGS) @touch $@ $(ROOT)/link: $(SRC)/link.d $(ROOT)/lib.so $(DRUNTIMESO) @@ -48,6 +54,13 @@ $(ROOT)/loadDR: $(SRC)/loadDR.c $(ROOT)/lib.so $(DRUNTIMESO) $(ROOT)/host: $(SRC)/host.c $(ROOT)/plugin1.so $(ROOT)/plugin2.so $(QUIET)$(CC) $(CFLAGS) -o $@ $< $(LDL) -pthread +$(ROOT)/link_mod_collision: $(ROOT)/%: $(SRC)/%.d $(ROOT)/lib.so $(DRUNTIMESO) + $(QUIET)$(DMD) $(DFLAGS) -of$@ $< -L$(ROOT)/lib.so + +$(ROOT)/load_mod_collision: $(ROOT)/%: $(SRC)/%.d $(ROOT)/lib.so $(DRUNTIMESO) +# use export dynamic so that Module in exe can interposes Module in lib.so + $(QUIET)$(DMD) $(DFLAGS) -of$@ $< $(LINKDL) -L--export-dynamic + $(ROOT)/liblinkdep.so: $(ROOT)/lib.so $(ROOT)/liblinkdep.so: DFLAGS+=-L$(ROOT)/lib.so diff --git a/test/shared/src/lib.d b/test/shared/src/lib.d index cd8cf6598c0..a3de8c9f739 100644 --- a/test/shared/src/lib.d +++ b/test/shared/src/lib.d @@ -1,3 +1,5 @@ +module lib; + // test EH void throwException() { diff --git a/test/shared/src/link_mod_collision.d b/test/shared/src/link_mod_collision.d new file mode 100644 index 00000000000..9c3d1c7b235 --- /dev/null +++ b/test/shared/src/link_mod_collision.d @@ -0,0 +1,5 @@ +module lib; // module collides with lib.so + +void main() +{ +} diff --git a/test/shared/src/load_mod_collision.d b/test/shared/src/load_mod_collision.d new file mode 100644 index 00000000000..310d6e5d88d --- /dev/null +++ b/test/shared/src/load_mod_collision.d @@ -0,0 +1,11 @@ +module lib; // module collides with lib.so + +import core.runtime, core.stdc.stdio, core.sys.posix.dlfcn; + +void main(string[] args) +{ + auto name = args[0]; + assert(name[$-19 .. $] == "/load_mod_collision"); + name = name[0 .. $-18] ~ "lib.so"; + auto lib = Runtime.loadLibrary(name); +} From 5f6c43131527749f6a3675ba4408bdfb90f2052b Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Thu, 23 Jul 2015 13:58:58 +0200 Subject: [PATCH 2/7] fix check for copy relocations when dynamically loading druntime - fixes Issue 14776 - shared library test - loadDR - segfaults on FreeBSD 10 - the executable might not define _end or __bss_start in which case they'll resolve to within the shared druntime library - skip copy relocation check when we can't determine the copy reloc section b/c druntime was dynamically loaded - copy relocations can't occur when dynamically loading a library anyhow - checkModuleCollisions only needed for version (Shared) druntime build --- src/rt/sections_elf_shared.d | 83 ++++++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/src/rt/sections_elf_shared.d b/src/rt/sections_elf_shared.d index 9c546f8cc93..fe0ff9e78cc 100644 --- a/src/rt/sections_elf_shared.d +++ b/src/rt/sections_elf_shared.d @@ -281,6 +281,12 @@ version (Shared) */ __gshared pthread_mutex_t _handleToDSOMutex; __gshared HashTab!(void*, DSO*) _handleToDSO; + + /* + * Section in executable that contains copy relocations. + * Might be null when druntime is dynamically loaded by a C host. + */ + __gshared const(void)[] _copyRelocSection; } else { @@ -331,7 +337,8 @@ extern(C) void _d_dso_registry(CompilerDSOData* data) // no backlink => register if (*data._slot is null) { - if (_loadedDSOs.empty) initLocks(); // first DSO + immutable firstDSO = _loadedDSOs.empty; + if (firstDSO) initLocks(); DSO* pdso = cast(DSO*).calloc(1, DSO.sizeof); assert(typeid(DSO).init().ptr is null); @@ -345,20 +352,21 @@ extern(C) void _d_dso_registry(CompilerDSOData* data) scanSegments(info, pdso); - checkModuleCollisions(info, pdso._moduleGroup.modules); - version (Shared) { - // the first loaded DSO is druntime itself - assert(!_loadedDSOs.empty || - /* We need a local symbol (rt_get_bss_start) or the function - * pointer might be a PLT address in the executable. - * data._slot is already local in the shared library - */ - handleForAddr(&rt_get_bss_start) == handleForAddr(data._slot)); + auto handle = handleForAddr(data._slot); + + if (firstDSO) + { + /// Assert that the first loaded DSO is druntime itself. Use a + /// local druntime symbol (rt_get_bss_start) to get the handle. + assert(handleForAddr(data._slot) == handleForAddr(&rt_get_bss_start)); + _copyRelocSection = getCopyRelocSection(); + } + checkModuleCollisions(info, pdso._moduleGroup.modules, _copyRelocSection); getDependencies(info, pdso._deps); - pdso._handle = handleForAddr(data._slot); + pdso._handle = handle; setDSOForHandle(pdso, pdso._handle); if (!_rtLoading) @@ -586,6 +594,14 @@ nothrow: return map; } + link_map* exeLinkMap(link_map* map) + { + assert(map); + while (map.l_prev !is null) + map = map.l_prev; + return map; + } + DSO* dsoForHandle(void* handle) { DSO* pdso; @@ -786,21 +802,52 @@ extern(C) void* rt_get_end() @nogc nothrow; } -nothrow -void checkModuleCollisions(in ref dl_phdr_info info, in immutable(ModuleInfo)*[] modules) +/// get the BSS section of the executable to check for copy relocations +const(void)[] getCopyRelocSection() nothrow +{ + auto bss_start = rt_get_bss_start(); + auto bss_end = rt_get_end(); + immutable bss_size = bss_end - bss_start; + + /** + Check whether __bss_start/_end both lie within the executable DSO.same DSO. + + When a C host program dynamically loads druntime, i.e. it isn't linked + against, __bss_start/_end might be defined in different DSOs, b/c the + linker creates those symbols only when they are used. + But as there are no copy relocations when dynamically loading a shared + library, we can simply return a null bss range in that case. + */ + if (bss_size <= 0) + return null; + + version (linux) + enum ElfW!"Addr" exeBaseAddr = 0; + else version (FreeBSD) + enum ElfW!"Addr" exeBaseAddr = 0; + + dl_phdr_info info = void; + findDSOInfoForAddr(bss_start, &info) || assert(0); + if (info.dlpi_addr != exeBaseAddr) + return null; + findDSOInfoForAddr(bss_end - 1, &info) || assert(0); + if (info.dlpi_addr != exeBaseAddr) + return null; + + return bss_start[0 .. bss_size]; +} + +void checkModuleCollisions(in ref dl_phdr_info info, in immutable(ModuleInfo)*[] modules, + in void[] copyRelocSection) nothrow in { assert(modules.length); } body { immutable(ModuleInfo)* conflicting; - auto bss_start = rt_get_bss_start(); - immutable bss_size = rt_get_end() - bss_start; - assert(bss_size >= 0); - foreach (m; modules) { auto addr = cast(const(void*))m; - if (cast(size_t)(addr - bss_start) < cast(size_t)bss_size) + if (cast(size_t)(addr - copyRelocSection.ptr) < copyRelocSection.length) { // Module is in .bss of the exe because it was copy relocated } From aa4376bcfa57c21c2be94d57bdb3b0930915da00 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Fri, 24 Jul 2015 14:17:27 +0200 Subject: [PATCH 3/7] add comment explaining module collision check --- src/rt/sections_elf_shared.d | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/rt/sections_elf_shared.d b/src/rt/sections_elf_shared.d index fe0ff9e78cc..61110be3e44 100644 --- a/src/rt/sections_elf_shared.d +++ b/src/rt/sections_elf_shared.d @@ -837,6 +837,13 @@ const(void)[] getCopyRelocSection() nothrow return bss_start[0 .. bss_size]; } +/** + * Check for module collisions. A module in a shared library collides + * with an existing module if it's ModuleInfo is interposed (search + * symbol interposition) by another DSO. Therefor two modules with the + * same name do not collide if their DSOs are in separate symbol resolution + * chains. + */ void checkModuleCollisions(in ref dl_phdr_info info, in immutable(ModuleInfo)*[] modules, in void[] copyRelocSection) nothrow in { assert(modules.length); } From 4659f966f92b6475ec9a16bf10ce492040d34be9 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Sat, 25 Jul 2015 01:42:28 +0200 Subject: [PATCH 4/7] workaround Bugzilla 14824 - A bug in FBSD's rtld will lead to a crash when calling a function over an PLT entry that was previously resolved to a now unloaded dynamic library. --- test/shared/Makefile | 2 +- test/shared/src/host.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/test/shared/Makefile b/test/shared/Makefile index 7e16456d5e6..4e846c6df9a 100644 --- a/test/shared/Makefile +++ b/test/shared/Makefile @@ -9,7 +9,7 @@ TESTS+=link_mod_collision load_mod_collision .PHONY: all clean all: $(addprefix $(ROOT)/,$(addsuffix .done,$(TESTS))) -$(ROOT)/loadDR.done: RUN_ARGS:=$(DRUNTIMESO) +$(ROOT)/loadDR.done $(ROOT)/host.done: RUN_ARGS:=$(DRUNTIMESO) $(ROOT)/%_mod_collision.done: $(ROOT)/%_mod_collision @echo Testing $*_mod_collision diff --git a/test/shared/src/host.c b/test/shared/src/host.c index 5928d26be81..81e896aa3d8 100644 --- a/test/shared/src/host.c +++ b/test/shared/src/host.c @@ -5,6 +5,12 @@ int main(int argc, char* argv[]) { +#if defined(__FreeBSD__) + // workaround for Bugzilla 14824 + void *druntime = dlopen(argv[1], RTLD_LAZY); // load druntime + assert(druntime); +#endif + const size_t pathlen = strrchr(argv[0], '/') - argv[0] + 1; char *name = malloc(pathlen + sizeof("plugin1.so")); memcpy(name, argv[0], pathlen); @@ -46,5 +52,9 @@ int main(int argc, char* argv[]) assert(dlclose(plugin1) == 0); free(name); + +#if defined(__FreeBSD__) + dlclose(druntime); +#endif return EXIT_SUCCESS; } From 8f41eb18b5ce4d8d825180fe00fb533931f8bd1f Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Fri, 24 Jul 2015 16:49:00 +0200 Subject: [PATCH 5/7] reenable druntime debug unittests --- posix.mak | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/posix.mak b/posix.mak index e7e2d35e6c8..7a93a468dad 100644 --- a/posix.mak +++ b/posix.mak @@ -196,9 +196,7 @@ ifeq (1,$(BUILD_WAS_SPECIFIED)) unittest : $(UT_MODULES) $(addsuffix /.run,$(ADDITIONAL_TESTS)) @echo done else -# The unit tests currently fail on FreeBSD in debug mode -#unittest : unittest-debug unittest-release -unittest : unittest-release +unittest : unittest-debug unittest-release unittest-%: $(MAKE) -f $(MAKEFILE) unittest OS=$(OS) MODEL=$(MODEL) DMD=$(DMD) BUILD=$* endif From 94d7132097df881199632624a4f4c5b8b7d42e88 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Sat, 25 Jul 2015 22:52:32 +0200 Subject: [PATCH 6/7] use _Exit(1) instead of assert(0) - to avoid a segfault/bus error --- src/rt/sections_elf_shared.d | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rt/sections_elf_shared.d b/src/rt/sections_elf_shared.d index 61110be3e44..041b7c1b011 100644 --- a/src/rt/sections_elf_shared.d +++ b/src/rt/sections_elf_shared.d @@ -878,7 +878,8 @@ body cast(int)loading.length, loading.ptr, cast(int)modname.length, modname.ptr, cast(int)existing.length, existing.ptr); - assert(0); + import core.stdc.stdlib : _Exit; + _Exit(1); } } From 5ea2b7bd80e799f64bd79d15346409728d706605 Mon Sep 17 00:00:00 2001 From: Martin Nowak Date: Sun, 26 Jul 2015 16:20:18 +0200 Subject: [PATCH 7/7] need --no-as-needed for some linux distributions (Ubuntu) - enforces the linking of lib.so even though the lib isn't used --- test/shared/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/shared/Makefile b/test/shared/Makefile index 4e846c6df9a..0b4a58b509c 100644 --- a/test/shared/Makefile +++ b/test/shared/Makefile @@ -55,7 +55,8 @@ $(ROOT)/host: $(SRC)/host.c $(ROOT)/plugin1.so $(ROOT)/plugin2.so $(QUIET)$(CC) $(CFLAGS) -o $@ $< $(LDL) -pthread $(ROOT)/link_mod_collision: $(ROOT)/%: $(SRC)/%.d $(ROOT)/lib.so $(DRUNTIMESO) - $(QUIET)$(DMD) $(DFLAGS) -of$@ $< -L$(ROOT)/lib.so +# use no-as-needed to enforce linking of unused lib.so + $(QUIET)$(DMD) $(DFLAGS) -of$@ $< -L--no-as-needed -L$(ROOT)/lib.so $(ROOT)/load_mod_collision: $(ROOT)/%: $(SRC)/%.d $(ROOT)/lib.so $(DRUNTIMESO) # use export dynamic so that Module in exe can interposes Module in lib.so