[lldb] Call Target::ClearAllLoadedSections even earlier #140228
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 llvm#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.
This reapplies llvm#138892, which was reverted in 5fb9dca 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.
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesThis reapplies #138892, which was reverted in Windows loads modules from the Process class, and it does that quite The original commit message was: Minidump files contain explicit information about load addresses of This used to work until #109477, which enabled the dynamic loader Clearing the load list makes sense, but I think we could do it earlier This fixes TestDynamicValue.py:test_from_core_file on darwin. Full diff: https://github.com/llvm/llvm-project/pull/140228.diff 3 Files Affected:
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..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);
@@ -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(
@@ -2984,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;
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
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reapplies #138892, which was reverted in
5fb9dca 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 #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.