https://github.com/labath created https://github.com/llvm/llvm-project/pull/140228
This reapplies https://github.com/llvm/llvm-project/pull/138892, which was reverted in https://github.com/llvm/llvm-project/commit/5fb9dca14aeaf12219ff149bf3a4f94c8dc58d8b due to failures on windows. Windows loads modules from the Process class, and it does that quite early, and it kinda makes sense which is why I'm moving the clearing code even earlier. The original commit message was: Minidump files contain explicit information about load addresses of modules, so it can load them itself. This works on other platforms, but fails on darwin because DynamicLoaderDarwin nukes the loaded module list on initialization (which happens after the core file plugin has done its work). This used to work until https://github.com/llvm/llvm-project/pull/109477, which enabled the dynamic loader plugins for minidump files in order to get them to provide access to TLS. Clearing the load list makes sense, but I think we could do it earlier in the process, so that both Process and DynamicLoader plugins get a chance to load modules. This patch does that by calling the function early in the launch/attach/load core flows. This fixes TestDynamicValue.py:test_from_core_file on darwin. >From 438daf635e8d57cdd9cf854c505ec9a434d11358 Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Wed, 14 May 2025 11:16:55 +0200 Subject: [PATCH 1/2] [lldb] Call Target::ClearAllLoadedSections earlier (#138892) Minidump files contain explicit information about load addresses of modules, so it can load them itself. This works on other platforms, but fails on darwin because DynamicLoaderDarwin nukes the loaded module list on initialization (which happens after the core file plugin has done its work). This used to work until #109477, which enabled the dynamic loader plugins for minidump files in order to get them to provide access to TLS. Clearing the load list makes sense, but I think we could do it earlier in the process, so that both Process and DynamicLoader plugins get a chance to load modules. This patch does that by calling the function early in the launch/attach/load core flows. This fixes TestDynamicValue.py:test_from_core_file on darwin. --- .../Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp | 1 - lldb/source/Target/Process.cpp | 4 ++++ lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py | 1 - 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp index 578ab12268ea3..1270d57423c7b 100644 --- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp +++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp @@ -872,7 +872,6 @@ void DynamicLoaderDarwin::PrivateInitialize(Process *process) { StateAsCString(m_process->GetState())); Clear(true); m_process = process; - m_process->GetTarget().ClearAllLoadedSections(); } // Member function that gets called when the process state changes. diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 13ff12b4ff953..7c5512598bbb6 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2763,6 +2763,7 @@ Status Process::LaunchPrivate(ProcessLaunchInfo &launch_info, StateType &state, } if (state == eStateStopped || state == eStateCrashed) { + GetTarget().ClearAllLoadedSections(); DidLaunch(); // Now that we know the process type, update its signal responses from the @@ -2799,6 +2800,7 @@ Status Process::LaunchPrivate(ProcessLaunchInfo &launch_info, StateType &state, } Status Process::LoadCore() { + GetTarget().ClearAllLoadedSections(); Status error = DoLoadCore(); if (error.Success()) { ListenerSP listener_sp( @@ -3094,6 +3096,8 @@ void Process::CompleteAttach() { Log *log(GetLog(LLDBLog::Process | LLDBLog::Target)); LLDB_LOGF(log, "Process::%s()", __FUNCTION__); + GetTarget().ClearAllLoadedSections(); + // Let the process subclass figure out at much as it can about the process // before we go looking for a dynamic loader plug-in. ArchSpec process_arch; diff --git a/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py b/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py index cd95a9ff3fe8c..faa35421ff60b 100644 --- a/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py +++ b/lldb/test/API/lang/cpp/dynamic-value/TestDynamicValue.py @@ -282,7 +282,6 @@ def test_from_forward_decl(self): @no_debug_info_test @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24663") - @expectedFailureDarwin # dynamic loader unloads modules @expectedFailureAll(archs=["arm"]) # Minidump saving not implemented def test_from_core_file(self): """Test fetching C++ dynamic values from core files. Specifically, test >From 9d6d2807284802e8b722b012c4b5b67a8be9ecba Mon Sep 17 00:00:00 2001 From: Pavel Labath <pa...@labath.sk> Date: Fri, 16 May 2025 11:17:25 +0200 Subject: [PATCH 2/2] [lldb] Call Target::ClearAllLoadedSections even earlier This reapplies #138892, which was reverted in 5fb9dca14aeaf12219ff149bf3a4f94c8dc58d8b due to failures on windows. Windows loads modules from the Process class, and it does that quite early, and it kinda makes sense which is why I'm moving the clearing code even earlier. --- lldb/source/Target/Process.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 7c5512598bbb6..c377feec86c16 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2675,6 +2675,7 @@ Status Process::LaunchPrivate(ProcessLaunchInfo &launch_info, StateType &state, m_jit_loaders_up.reset(); m_system_runtime_up.reset(); m_os_up.reset(); + GetTarget().ClearAllLoadedSections(); { std::lock_guard<std::mutex> guard(m_process_input_reader_mutex); @@ -2763,7 +2764,6 @@ Status Process::LaunchPrivate(ProcessLaunchInfo &launch_info, StateType &state, } if (state == eStateStopped || state == eStateCrashed) { - GetTarget().ClearAllLoadedSections(); DidLaunch(); // Now that we know the process type, update its signal responses from the @@ -2986,6 +2986,7 @@ Status Process::Attach(ProcessAttachInfo &attach_info) { m_jit_loaders_up.reset(); m_system_runtime_up.reset(); m_os_up.reset(); + GetTarget().ClearAllLoadedSections(); lldb::pid_t attach_pid = attach_info.GetProcessID(); Status error; @@ -3096,8 +3097,6 @@ void Process::CompleteAttach() { Log *log(GetLog(LLDBLog::Process | LLDBLog::Target)); LLDB_LOGF(log, "Process::%s()", __FUNCTION__); - GetTarget().ClearAllLoadedSections(); - // Let the process subclass figure out at much as it can about the process // before we go looking for a dynamic loader plug-in. ArchSpec process_arch; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
