jasonmolenda created this revision.
jasonmolenda added a reviewer: tatyana-krasnukha.
jasonmolenda added a project: LLDB.
Herald added subscribers: JDevlieghere, GorNishanov.
jasonmolenda requested review of this revision.
Part of Tatyana's patch in https://reviews.llvm.org/D92164 added a stack of
ExecutionContexts to the CommandInterpreter (pushed with
OverrideExecutionContext, popped with RestoreExecutionContext) and when we need
to retrieve the currently selected ExecutionContext via CommandInterpeter::
GetExecutionContext(), if the stack has an entry, we return the exe_ctx from
the top of the stack, else we fall back to the
Debugger::GetSelectedExecutionContext() which gives you an exe_ctx with the
currently selected Target.
This patch is fixing the fact that the lldb command driver during startup,
pushes the current exe_ctx on the stack here,
-> 2874
OverrideExecutionContext(m_debugger.GetSelectedExecutionContext());
2875 auto finalize = llvm::make_scope_exit([this]() {
2876 RestoreExecutionContext();
0 0x00010d9fc80d
LLDB`lldb_private::CommandInterpreter::IOHandlerInputComplete + 285
CommandInterpreter.cpp:2874
1 0x00010d81573d LLDB`lldb_private::IOHandlerEditline::Run + 349
IOHandler.cpp:582
2 0x00010d7ccd71 LLDB`lldb_private::Debugger::RunIOHandlers + 81
Debugger.cpp:879
3 0x00010d9fe0d4
LLDB`lldb_private::CommandInterpreter::RunCommandInterpreter + 212
CommandInterpreter.cpp:3126
4 0x00010d268370 LLDB`lldb::SBDebugger::RunCommandInterpreter + 544
SBDebugger.cpp:1238
5 0x00010854b8a2 lldb`Driver::MainLoop + 3298 Driver.cpp:678
There is no Target at this point, so we have an ExecutionContext with a nullptr
for a target (and all other fields).
This Target won't get popped off the stack, it remains there as the first
entry. When loading a corefile, which loads a binary and dSYM, and the dSYM has
Python code in it, and the SB API tries to set some settings via
SBCommandInterpreter::HandleCommand, if those commands need the current Target
to set something, they will silently fail to do anything. Target settings that
are set in the template target will work, but those that only take effect in
the current target are ignored. It can be a little tricky to spot.
Complicating the test case writing, API tests don't have this same "push a
nullptr target on to the stack" issue, so they won't reproduce the problem. I
started writing an API test and I kept it in this patch because it might find
regressions in the future. Jonas helped rewrite the test into a shell test that
uses the driver program to show the issue and confirm the patch fixes it.
I can imagine we might iterate on exactly how I avoid pushing a nullptr
ExecutionContext, but I think the basic safeguard is pretty straightforward -
it's a pretty unusual situation to have no selected Targets, this only happens
during early startup I suspect. Passing an arg to the make_scope_exit to avoid
popping the stack seems especially unpretty. I wasn't worried about popping
too many elements off the stack, the code will ignore that case, but I was
worried that it might be possible to have a real ExecutionContext on the stack,
then a nullptr ExecutionContext is pushed, and I didn't want to pop the real
exectx later on.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111209
Files:
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/test/API/macosx/dsym-script-settings-corefile/Makefile
lldb/test/API/macosx/dsym-script-settings-corefile/TestdSYMScriptSettingsCorefile.py
lldb/test/API/macosx/dsym-script-settings-corefile/a_out.py
lldb/test/API/macosx/dsym-script-settings-corefile/main.c
lldb/test/API/macosx/dsym-script-settings-corefile/operating_system.py
lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/a_out.py
lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/main.c
lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/operating_system.py
lldb/test/Shell/ScriptInterpreter/Python/OS/os.test
Index: lldb/test/Shell/ScriptInterpreter/Python/OS/os.test
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/OS/os.test
@@ -0,0 +1,23 @@
+# REQUIRES: system-darwin
+
+# RUN: %clang_host %S/Inputs/main.c -g -o %t.out
+# RUN: mkdir -p %t.out.dSYM/Contents/Resources/Python
+# RUN: cp %S/Inputs/a_out.py %t.out.dSYM/Contents/Resources/Python/os_test_tmp_out.py
+# RUN: cp %S/Inputs/operating_system.py %t.out.dSYM/Contents/Resources/Python/
+
+# RUN: %lldb -x %t.out \
+# RUN: -o 'br s -p break' \
+# RUN: -o 'r' \
+# RUN: -o 'pro save-core -s stack %t.corefile' \
+# RUN: -o 'pro kill' \
+# RUN: -o 'tar del' \
+# RUN: -o 'settings set target.load-script-from-symbol-file true' \
+# RUN: -o 'tar cr -c %t.corefile' \
+# RUN: -o 'sett show target.process.memory-cache-line-size target.default-arch target.prefer-dynamic-value target.process.python-os-plugin-path' \
+# RUN: -o 'thread list' | FileCheck %s
+
+# CHECK: target.process.memory-cache-line-size (unsigned) = 16
+# CHECK: target.default-arch (arch) = x86_64
+# CHECK: target.prefer-dynamic-value (enum) = run-target
+# CHECK: name = 'one', queue = 'queue1'
+# CHECK: name = 'two', queue = 'queue2'
Index: lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/operating_system.py
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/operating_system.py
@@ -0,0 +1,45 @@
+import lldb
+
+
+class OperatingSystemPlugIn(object):
+ """Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class"""
+
+ def __init__(self, process):
+ '''Initialization needs a valid.SBProcess object.
+
+ This plug-in will get created after a live process is valid and has stopped for the first time.
+ '''
+ self.process = None
+ self.registers = None
+ self.threads = None
+ if isinstance(process, lldb.SBProcess) and process.IsValid():
+ self.process = process
+ self.threads = None # Will be an dictionary containing info for each thread
+
+ def get_target(self):
+ return self.process.target
+
+ def get_thread_info(self):
+ if not self.threads:
+ # FIXME: LLDB is not actually parsing thread stop reasons.
+ self.threads = [{
+ 'tid': 0x111111111,
+ 'name': 'one',
+ 'queue': 'queue1',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed'
+ }, {
+ 'tid': 0x222222222,
+ 'name': 'two',
+ 'queue': 'queue2',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed'
+ }, {
+ 'tid': 0x333333333,
+ 'name': 'three',
+ 'queue': 'queue3',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed - should be "sigstop" though',
+ 'core': 0
+ }]
+ return self.threads
Index: lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/main.c
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/main.c
@@ -0,0 +1,4 @@
+#include <stdio.h>
+int main() {
+ puts ("Hello, corefile"); // break here
+}
Index: lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/a_out.py
===================================================================
--- /dev/null
+++ lldb/test/Shell/ScriptInterpreter/Python/OS/Inputs/a_out.py
@@ -0,0 +1,34 @@
+def __lldb_init_module(debugger, internal_dict):
+ print("")
+ print("")
+ # Process settings will be set in the template
+ set1 = "settings set target.process.memory-cache-line-size 16"
+ print("Process setting:")
+ print(set1)
+ debugger.HandleCommand(set1)
+
+ # Global target settings will be set in the template
+ set2 = "sett set target.default-arch x86_64"
+ print("")
+ print("Global target setting:")
+ print(set2)
+ debugger.HandleCommand(set2)
+
+ # Target specific settings need a Target to set in
+ set3 = "settings set target.prefer-dynamic-value run-target"
+ print("")
+ print("Target specific setting:")
+ print(set3)
+ debugger.HandleCommand(set3)
+
+ self_path = "{}".format(__file__)
+ base_dir_name = self_path[:self_path.rfind("/")]
+ os_plugin = base_dir_name + "/operating_system.py"
+ set4 = "settings set target.process.python-os-plugin-path \"%s\"" % os_plugin
+ print("")
+ print("OS Plugin setting:")
+ print(set4)
+ debugger.HandleCommand(set4)
+
+ print("")
+ print("")
Index: lldb/test/API/macosx/dsym-script-settings-corefile/operating_system.py
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/dsym-script-settings-corefile/operating_system.py
@@ -0,0 +1,45 @@
+import lldb
+
+
+class OperatingSystemPlugIn(object):
+ """Class that provides data for an instance of a LLDB 'OperatingSystemPython' plug-in class"""
+
+ def __init__(self, process):
+ '''Initialization needs a valid.SBProcess object.
+
+ This plug-in will get created after a live process is valid and has stopped for the first time.
+ '''
+ self.process = None
+ self.registers = None
+ self.threads = None
+ if isinstance(process, lldb.SBProcess) and process.IsValid():
+ self.process = process
+ self.threads = None # Will be an dictionary containing info for each thread
+
+ def get_target(self):
+ return self.process.target
+
+ def get_thread_info(self):
+ if not self.threads:
+ # FIXME: LLDB is not actually parsing thread stop reasons.
+ self.threads = [{
+ 'tid': 0x111111111,
+ 'name': 'one',
+ 'queue': 'queue1',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed'
+ }, {
+ 'tid': 0x222222222,
+ 'name': 'two',
+ 'queue': 'queue2',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed'
+ }, {
+ 'tid': 0x333333333,
+ 'name': 'three',
+ 'queue': 'queue3',
+ 'state': 'stopped',
+ 'stop_reason': 'not parsed - should be "sigstop" though',
+ 'core': 0
+ }]
+ return self.threads
Index: lldb/test/API/macosx/dsym-script-settings-corefile/main.c
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/dsym-script-settings-corefile/main.c
@@ -0,0 +1,4 @@
+#include <stdio.h>
+int main() {
+ puts ("HI corefile"); // break here
+}
Index: lldb/test/API/macosx/dsym-script-settings-corefile/a_out.py
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/dsym-script-settings-corefile/a_out.py
@@ -0,0 +1,34 @@
+def __lldb_init_module(debugger, internal_dict):
+ print("")
+ print("")
+ # Process settings will be set in the template
+ set1 = "settings set target.process.memory-cache-line-size 16"
+ print("Process setting:")
+ print(set1)
+ debugger.HandleCommand(set1)
+
+ # Global target settings will be set in the template
+ set2 = "sett set target.default-arch x86_64"
+ print("")
+ print("Global target setting:")
+ print(set2)
+ debugger.HandleCommand(set2)
+
+ # Target specific settings need a Target to set in
+ set3 = "settings set target.prefer-dynamic-value run-target"
+ print("")
+ print("Target specific setting:")
+ print(set3)
+ debugger.HandleCommand(set3)
+
+ self_path = "{}".format(__file__)
+ base_dir_name = self_path[:self_path.rfind("/")]
+ os_plugin = base_dir_name + "/operating_system.py"
+ set4 = "settings set target.process.python-os-plugin-path \"%s\"" % os_plugin
+ print("")
+ print("OS Plugin setting:")
+ print(set4)
+ debugger.HandleCommand(set4)
+
+ print("")
+ print("")
Index: lldb/test/API/macosx/dsym-script-settings-corefile/TestdSYMScriptSettingsCorefile.py
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/dsym-script-settings-corefile/TestdSYMScriptSettingsCorefile.py
@@ -0,0 +1,95 @@
+"""Test that settings in dSYM Python scripts are registered when read from corefile."""
+
+import os
+import re
+import subprocess
+import shutil
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestdSYMScriptSettingsCorefile(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @no_debug_info_test
+ @skipUnlessDarwin
+ @skipIfOutOfTreeDebugserver # newer debugserver required for these qMemoryRegionInfo types
+ def test(self):
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ dsym = self.getBuildArtifact("a.out.dSYM")
+ python_resources_dir = os.path.join(dsym, "Contents", "Resources", "Python")
+ python_file = os.path.join(self.getSourceDir(), "a_out.py")
+ osplugin_python_file = os.path.join(self.getSourceDir(), "operating_system.py")
+ lldbutil.mkdir_p(python_resources_dir)
+ shutil.copyfile(python_file, os.path.join(python_resources_dir, "a_out.py"))
+ shutil.copyfile(osplugin_python_file, os.path.join(python_resources_dir, "operating_system.py"))
+
+ corefile = self.getBuildArtifact("process.core")
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "// break here", lldb.SBFileSpec("main.c"))
+
+ self.runCmd("process save-core -s stack " + corefile)
+
+ ci = self.dbg.GetCommandInterpreter()
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.process.memory-cache-line-size", result)
+ init_value_memory_cache_line = re.search(" = ([0-9]+)\s*$", result.GetOutput()).group(1)
+ self.assertEqual(init_value_memory_cache_line, "512")
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.default-arch", result)
+ self.assertEqual(re.search(" = (.+)\s*$", result.GetOutput()), None)
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.prefer-dynamic-value", result)
+ init_value_prefer_dynamic_value = re.search(" = (.+)\s*$", result.GetOutput()).group(1)
+ # command driver lldb will have no-run-target;
+ # SB API test will have no-dynamic-values.
+ # the custom python sets it to run-target, so just check that
+ # it's not equal to that.
+ self.assertNotEqual(init_value_prefer_dynamic_value, "run-target")
+
+ process.Kill()
+ self.dbg.DeleteTarget(target)
+
+
+ self.runCmd("settings set target.load-script-from-symbol-file true", check=False)
+ def cleanup():
+ self.runCmd("settings set target.load-script-from-symbol-file false", check=False)
+ self.runCmd("settings set target.process.memory-cache-line-size " + init_value_memory_cache_line)
+ self.runCmd("settings clear target.default-arch")
+ self.runCmd("settings set target.prefer-dynamic-value " + init_value_prefer_dynamic_value)
+ self.runCmd("settings clear target.process.python-os-plugin-path")
+ self.addTearDownHook(cleanup)
+
+ if self.TraceOn():
+ self.runCmd("settings show target.process.memory-cache-line-size target.default-arch target.prefer-dynamic-value target.process.python-os-plugin-path")
+
+ # Now load the corefile
+ target = self.dbg.CreateTarget('')
+ process = target.LoadCore(corefile)
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.process.memory-cache-line-size", result)
+ new_init_value_memory_cache_line = re.search(" = ([0-9]+)\s*$", result.GetOutput()).group(1)
+ self.assertEqual(new_init_value_memory_cache_line, "16")
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.default-arch", result)
+ new_default_arch = re.search(" = (.+)\s*$", result.GetOutput()).group(1)
+ self.assertEqual(new_default_arch, "x86_64")
+
+ result = lldb.SBCommandReturnObject()
+ ci.HandleCommand("settings show target.prefer-dynamic-value", result)
+ new_prefer_dynamic_value = re.search(" = (.+)\s*$", result.GetOutput()).group(1)
+ self.assertEqual(new_prefer_dynamic_value, "run-target")
+
+ if self.TraceOn():
+ self.runCmd("settings show target.process.memory-cache-line-size target.default-arch target.prefer-dynamic-value target.process.python-os-plugin-path")
+ self.runCmd("thread list")
+
Index: lldb/test/API/macosx/dsym-script-settings-corefile/Makefile
===================================================================
--- /dev/null
+++ lldb/test/API/macosx/dsym-script-settings-corefile/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+include Makefile.rules
Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -7013,7 +7013,7 @@
bool ObjectFileMachO::LoadCoreFileImages(lldb_private::Process &process) {
MachOCorefileAllImageInfos image_infos = GetCorefileAllImageInfos();
- bool added_images = false;
+ ModuleList added_modules;
if (image_infos.IsValid()) {
for (const MachOCorefileImageEntry &image : image_infos.all_image_infos) {
ModuleSpec module_spec;
@@ -7035,7 +7035,7 @@
}
}
if (module_sp.get()) {
- added_images = true;
+ added_modules.Append(module_sp);
for (auto name_vmaddr_tuple : image.segment_load_addresses) {
SectionList *sectlist = module_sp->GetObjectFile()->GetSectionList();
if (sectlist) {
@@ -7050,5 +7050,6 @@
}
}
}
- return added_images;
+ process.GetTarget().ModulesDidLoad(added_modules);
+ return added_modules.GetSize() > 0;
}
Index: lldb/source/Interpreter/CommandInterpreter.cpp
===================================================================
--- lldb/source/Interpreter/CommandInterpreter.cpp
+++ lldb/source/Interpreter/CommandInterpreter.cpp
@@ -2839,9 +2839,15 @@
StartHandlingCommand();
- OverrideExecutionContext(m_debugger.GetSelectedExecutionContext());
- auto finalize = llvm::make_scope_exit([this]() {
- RestoreExecutionContext();
+ ExecutionContext exe_ctx = m_debugger.GetSelectedExecutionContext();
+ bool pushed_exe_ctx = false;
+ if (exe_ctx.HasTargetScope()) {
+ OverrideExecutionContext(exe_ctx);
+ pushed_exe_ctx = true;
+ }
+ auto finalize = llvm::make_scope_exit([this, pushed_exe_ctx]() {
+ if (pushed_exe_ctx)
+ RestoreExecutionContext();
});
lldb_private::CommandReturnObject result(m_debugger.GetUseColor());
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
