Remove druntime's libunwind dependency (#4691)

* Reuse Darwin backtracing code for DRuntime_Use_Libunwind and use same code path as libexec-based backtracing. * Remove linking with libunwind * Run two tests manually for musl.
ldc-developers · Jul 3, 2024 · 7ad1a98 · 7ad1a98
1 parent a0318c2
commit 7ad1a98
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 209 deletions.
diff --git a/.github/workflows/alpine_musl.yml b/.github/workflows/alpine_musl.yml
@@ -21,7 +21,7 @@ jobs:
         uses: jirutka/setup-alpine@v1
         with:
           branch: v3.20
-          packages: ldc git g++ cmake ninja llvm-dev llvm-static compiler-rt libunwind-static libxml2-static zstd-static zlib-static bash grep diffutils make
+          packages: ldc git g++ cmake ninja llvm-dev llvm-static compiler-rt libxml2-static zstd-static zlib-static bash grep diffutils make
 
       - name: Build LDC bootstrap
         shell: alpine.sh {0}
@@ -73,7 +73,16 @@ jobs:
       - name: Run DMD testsuite
         if: success() || failure()
         shell: alpine.sh {0}
-        run: ctest -V -R "dmd-testsuite"
+        run: |
+          # These two tests require extra flags "-link-defaultlib-debug -frame-pointer=all", https://github.com/ldc-developers/ldc/issues/4694.
+          # Run them separately with these flags, and then remove them before running the full testsuite.
+          bin/ldc2    -g -L-export-dynamic -link-defaultlib-debug -frame-pointer=all -run tests/dmd/runnable/test17559.d
+          bin/ldc2 -O -g -L-export-dynamic -link-defaultlib-debug -frame-pointer=all -run tests/dmd/runnable/test17559.d
+          bin/ldc2    -g -L-export-dynamic -link-defaultlib-debug -frame-pointer=all -run tests/dmd/runnable/test19086.d
+          bin/ldc2 -O -g -L-export-dynamic -link-defaultlib-debug -frame-pointer=all -run tests/dmd/runnable/test19086.d
+          rm tests/dmd/runnable/test17559.d
+          rm tests/dmd/runnable/test19086.d
+          ctest -V -R "dmd-testsuite"
 
       - name: Run defaultlib unittests & druntime integration tests
         if: success() || failure()

diff --git a/driver/linker-gcc.cpp b/driver/linker-gcc.cpp
@@ -694,9 +694,6 @@ void ArgsBuilder::addDefaultPlatformLibs() {
       args.push_back("-lm");
       break;
     }
-    if (triple.isMusl() && !global.params.betterC) {
-      args.push_back("-lunwind"); // for druntime backtrace
-    }
     args.push_back("-lrt");
     args.push_back("-ldl");
   // fallthrough

diff --git a/runtime/DRuntimeIntegrationTests.cmake b/runtime/DRuntimeIntegrationTests.cmake
@@ -42,10 +42,6 @@ set(linkdl "")
 if("${TARGET_SYSTEM}" MATCHES "Linux")
     set(linkdl "LINKDL=-L-ldl")
 endif()
-set(linkunwind "")
-if("${TARGET_SYSTEM}" MATCHES "musl")
-    set(linkunwind "LINKUNWIND=-L-lunwind")
-endif()
 
 get_subdirs(testnames ${PROJECT_SOURCE_DIR}/druntime/test)
 if(${BUILD_SHARED_LIBS} STREQUAL "OFF")
@@ -78,7 +74,7 @@ foreach(name ${testnames})
             COMMAND ${GNU_MAKE_BIN} -C ${PROJECT_SOURCE_DIR}/druntime/test/${name}
                 ROOT=${outdir} DMD=${LDMD_EXE_FULL} BUILD=${build}
                 DRUNTIME=${druntime_path_build} DRUNTIMESO=${shared_druntime_path_build}
-                SHARED=1 ${cflags_base} ${linkdl} ${linkunwind}
+                SHARED=1 ${cflags_base} ${linkdl}
         )
         set_tests_properties(${fullname} PROPERTIES DEPENDS clean-${fullname})
     endforeach()

diff --git a/runtime/druntime/src/core/internal/backtrace/handler.d b/runtime/druntime/src/core/internal/backtrace/handler.d
diff --git a/runtime/druntime/src/core/internal/backtrace/unwind.d b/runtime/druntime/src/core/internal/backtrace/unwind.d
@@ -150,7 +150,7 @@ version (LDC) // simplify runtime function forward declaration
 else
     void _Unwind_Resume(_Unwind_Exception* exception_object);
 _Unwind_Reason_Code _Unwind_Resume_or_Rethrow(_Unwind_Exception* exception_object);
-_Unwind_Reason_Code _Unwind_Backtrace(_Unwind_Trace_Fn, void*);
+_Unwind_Reason_Code _Unwind_Backtrace(_Unwind_Trace_Fn, void*) nothrow;
 
 version (ARM_EABI_UNWINDER)
 {
@@ -160,46 +160,37 @@ version (ARM_EABI_UNWINDER)
     // On ARM, these are macros resp. not visible (static inline). To avoid
     // an unmaintainable amount of dependencies on implementation details,
     // just use a C shim (in ldc/arm_unwind.c).
-    _Unwind_Word _d_eh_GetGR(_Unwind_Context* context, int index);
+    _Unwind_Word _d_eh_GetGR(_Unwind_Context* context, int index) nothrow;
     alias _Unwind_GetGR = _d_eh_GetGR;
 
-    void _d_eh_SetGR(_Unwind_Context* context, int index, _Unwind_Word new_value);
+    void _d_eh_SetGR(_Unwind_Context* context, int index, _Unwind_Word new_value) nothrow;
     alias _Unwind_SetGR = _d_eh_SetGR;
 
-    _Unwind_Ptr _d_eh_GetIP(_Unwind_Context* context);
+    _Unwind_Ptr _d_eh_GetIP(_Unwind_Context* context) nothrow;
     alias _Unwind_GetIP = _d_eh_GetIP;
 
-    _Unwind_Ptr _d_eh_GetIPInfo(_Unwind_Context* context, int*);
+    _Unwind_Ptr _d_eh_GetIPInfo(_Unwind_Context* context, int*) nothrow;
     alias _Unwind_GetIPInfo = _d_eh_GetIPInfo;
 
-    void _d_eh_SetIP(_Unwind_Context* context, _Unwind_Ptr new_value);
+    void _d_eh_SetIP(_Unwind_Context* context, _Unwind_Ptr new_value) nothrow;
     alias _Unwind_SetIP = _d_eh_SetIP;
 }
 else
 {
-    _Unwind_Word _Unwind_GetGR(_Unwind_Context* context, int index);
-    void _Unwind_SetGR(_Unwind_Context* context, int index, _Unwind_Word new_value);
-    _Unwind_Ptr _Unwind_GetIP(_Unwind_Context* context);
-    _Unwind_Ptr _Unwind_GetIPInfo(_Unwind_Context* context, int*);
-    void _Unwind_SetIP(_Unwind_Context* context, _Unwind_Ptr new_value);
+    _Unwind_Word _Unwind_GetGR(_Unwind_Context* context, int index) nothrow;
+    void _Unwind_SetGR(_Unwind_Context* context, int index, _Unwind_Word new_value) nothrow;
+    _Unwind_Ptr _Unwind_GetIP(_Unwind_Context* context) nothrow;
+    _Unwind_Ptr _Unwind_GetIPInfo(_Unwind_Context* context, int*) nothrow;
+    void _Unwind_SetIP(_Unwind_Context* context, _Unwind_Ptr new_value) nothrow;
 }
-_Unwind_Word _Unwind_GetCFA(_Unwind_Context*);
-_Unwind_Word _Unwind_GetBSP(_Unwind_Context*);
-void* _Unwind_GetLanguageSpecificData(_Unwind_Context*);
-_Unwind_Ptr _Unwind_GetRegionStart(_Unwind_Context* context);
-void* _Unwind_FindEnclosingFunction(void* pc);
+_Unwind_Word _Unwind_GetCFA(_Unwind_Context*) nothrow;
+_Unwind_Word _Unwind_GetBSP(_Unwind_Context*) nothrow;
+void* _Unwind_GetLanguageSpecificData(_Unwind_Context*) nothrow;
+_Unwind_Ptr _Unwind_GetRegionStart(_Unwind_Context* context) nothrow;
+void* _Unwind_FindEnclosingFunction(void* pc) nothrow;
 
-version (X68_64)
+version (X86_64)
 {
-    _Unwind_Ptr _Unwind_GetDataRelBase(_Unwind_Context* context)
-    {
-        return _Unwind_GetGR(context, 1);
-    }
-
-    _Unwind_Ptr _Unwind_GetTextRelBase(_Unwind_Context* context)
-    {
-        assert(0);
-    }
 }
 else
 {

diff --git a/runtime/druntime/src/core/runtime.d b/runtime/druntime/src/core/runtime.d
@@ -19,9 +19,17 @@ else version (TVOS)
 else version (WatchOS)
     version = Darwin;
 
+version(LDC) version (Darwin)
+{
+    // Use our own backtrace() based on _Unwind_Backtrace(), as the former (from
+    // execinfo) doesn't seem to handle missing frame pointers too well.
+    version = DefineBacktrace_using_UnwindBacktrace;
+}
+
 version (DRuntime_Use_Libunwind)
 {
-    import core.internal.backtrace.libunwind;
+    version = DefineBacktrace_using_UnwindBacktrace;
+
     // This shouldn't be necessary but ensure that code doesn't get mixed
     // It does however prevent the unittest SEGV handler to be installed,
     // which is desireable as it uses backtrace directly.
@@ -670,24 +678,11 @@ extern (C) UnitTestResult runModuleUnitTests()
     return results;
 }
 
-version (LDC) version (Darwin)
+version (DefineBacktrace_using_UnwindBacktrace)
 {
-    nothrow:
-
-    extern (C)
-    {
-        enum _URC_NO_REASON = 0;
-        enum _URC_END_OF_STACK = 5;
+    import core.internal.backtrace.unwind;
 
-        alias _Unwind_Context_Ptr = void*;
-        alias _Unwind_Trace_Fn = int function(_Unwind_Context_Ptr, void*);
-        int _Unwind_Backtrace(_Unwind_Trace_Fn, void*);
-        ptrdiff_t _Unwind_GetIP(_Unwind_Context_Ptr context);
-    }
-
-    // Use our own backtrce() based on _Unwind_Backtrace(), as the former (from
-    // execinfo) doesn't seem to handle missing frame pointers too well.
-    private int backtrace(void** buffer, int maxSize)
+    private int backtrace(void** buffer, int maxSize) nothrow
     {
         if (maxSize < 0) return 0;
 
@@ -698,7 +693,7 @@ version (LDC) version (Darwin)
             int entriesWritten = 0;
         }
 
-        static extern(C) int handler(_Unwind_Context_Ptr context, void* statePtr)
+        static extern(C) int handler(_Unwind_Context* context, void* statePtr)
         {
             auto state = cast(State*)statePtr;
             if (state.entriesWritten >= state.maxSize) return _URC_END_OF_STACK;
@@ -819,14 +814,8 @@ void defaultTraceDeallocator(Throwable.TraceInfo info) nothrow
     free(cast(void *)obj);
 }
 
-version (DRuntime_Use_Libunwind)
-{
-    import core.internal.backtrace.handler;
-
-    alias DefaultTraceInfo = LibunwindHandler;
-}
 /// Default implementation for most POSIX systems
-else version (Posix) private class DefaultTraceInfo : Throwable.TraceInfo
+version (Posix) private class DefaultTraceInfo : Throwable.TraceInfo
 {
     import core.demangle;
     import core.stdc.stdlib : free;

diff --git a/runtime/druntime/test/common.mak b/runtime/druntime/test/common.mak
@@ -12,7 +12,6 @@ DMD:=
 DRUNTIME:=
 DRUNTIMESO:=
 LINKDL:=
-LINKUNWIND:=
 QUIET:=
 TIMELIMIT:=
 PIC:=
@@ -26,7 +25,6 @@ ifeq (,$(findstring ldmd2,$(DMD)))
 endif
 
 LDL:=$(subst -L,,$(LINKDL)) # -ldl
-LUNWIND:=$(subst -L,,$(LINKUNWIND)) # -lunwind
 SRC:=src
 GENERATED:=./generated
 ROOT:=$(GENERATED)/$(OS)/$(BUILD)/$(MODEL)

diff --git a/runtime/druntime/test/stdcpp/Makefile b/runtime/druntime/test/stdcpp/Makefile
@@ -98,22 +98,22 @@ $(ROOT)/%_old.done: $(ROOT)/%_old$(DOTEXE)
 $(ROOT)/%$(DOTEXE): $(SRC)/%.cpp $(SRC)/%_test.d
 	@mkdir -p $(dir $@)
 	$(QUIET)$(DMD) $(DFLAGS) -extern-std=c++98 -main -unittest -version=CoreUnittest -c -of=$(ROOT)/$*_d$(DOTOBJ) $(SRC)/$*_test.d
-	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++98 -o $@ $< $(ROOT)/$*_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL) $(LUNWIND)
+	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++98 -o $@ $< $(ROOT)/$*_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL)
 # build C++11 tests
 $(ROOT)/%_11$(DOTEXE): $(SRC)/%.cpp $(SRC)/%_test.d
 	@mkdir -p $(dir $@)
 	$(QUIET)$(DMD) $(DFLAGS) -extern-std=c++11 -main -unittest -version=CoreUnittest -c -of=$(ROOT)/$*_11_d$(DOTOBJ) $(SRC)/$*_test.d
-	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++11 -o $@ $< $(ROOT)/$*_11_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL) $(LUNWIND)
+	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++11 -o $@ $< $(ROOT)/$*_11_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL)
 # build C++17 tests
 $(ROOT)/%_17$(DOTEXE): $(SRC)/%.cpp $(SRC)/%_test.d
 	@mkdir -p $(dir $@)
 	$(QUIET)$(DMD) $(DFLAGS) -extern-std=c++17 -main -unittest -version=CoreUnittest -c -of=$(ROOT)/$*_17_d$(DOTOBJ) $(SRC)/$*_test.d
-	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++17 -o $@ $< $(ROOT)/$*_17_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL) $(LUNWIND)
+	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -std=c++17 -o $@ $< $(ROOT)/$*_17_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL)
 # build libstdc++ _GLIBCXX_USE_CXX11_ABI=0 tests
 $(ROOT)/%_old$(DOTEXE): $(SRC)/%.cpp $(SRC)/%_test.d
 	@mkdir -p $(dir $@)
 	$(QUIET)$(DMD) $(DFLAGS) -version=_GLIBCXX_USE_CXX98_ABI -main -unittest -version=CoreUnittest -c -of=$(ROOT)/$*_old_d$(DOTOBJ) $(SRC)/$*_test.d
-	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -D_GLIBCXX_USE_CXX11_ABI=0 -o $@ $< $(ROOT)/$*_old_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL) $(LUNWIND)
+	$(QUIET)$(CXX) $(CXXFLAGS_BASE) -D_GLIBCXX_USE_CXX11_ABI=0 -o $@ $< $(ROOT)/$*_old_d$(DOTOBJ) $(DRUNTIME) -lpthread $(LDL)
 
 endif # end Posix