-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Handle step-in over a Darwin "branch island". #138330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-lldb Author: None (jimingham) ChangesFull diff: https://github.com/llvm/llvm-project/pull/138330.diff 7 Files Affected:
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index e25c4ff55e408..0c5e69f47905f 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -26,6 +26,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadPlanCallFunction.h"
#include "lldb/Target/ThreadPlanRunToAddress.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
#include "lldb/Utility/DataBuffer.h"
#include "lldb/Utility/DataBufferHeap.h"
#include "lldb/Utility/LLDBLog.h"
@@ -923,15 +924,15 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
if (current_symbol != nullptr) {
std::vector<Address> addresses;
- if (current_symbol->IsTrampoline()) {
- ConstString trampoline_name =
+ ConstString current_name =
current_symbol->GetMangled().GetName(Mangled::ePreferMangled);
+ if (current_symbol->IsTrampoline()) {
- if (trampoline_name) {
+ if (current_name) {
const ModuleList &images = target_sp->GetImages();
SymbolContextList code_symbols;
- images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeCode,
+ images.FindSymbolsWithNameAndType(current_name, eSymbolTypeCode,
code_symbols);
for (const SymbolContext &context : code_symbols) {
Address addr = context.GetFunctionOrSymbolAddress();
@@ -946,7 +947,7 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
SymbolContextList reexported_symbols;
images.FindSymbolsWithNameAndType(
- trampoline_name, eSymbolTypeReExported, reexported_symbols);
+ current_name, eSymbolTypeReExported, reexported_symbols);
for (const SymbolContext &context : reexported_symbols) {
if (context.symbol) {
Symbol *actual_symbol =
@@ -968,7 +969,7 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
}
SymbolContextList indirect_symbols;
- images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeResolver,
+ images.FindSymbolsWithNameAndType(current_name, eSymbolTypeResolver,
indirect_symbols);
for (const SymbolContext &context : indirect_symbols) {
@@ -1002,7 +1003,7 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
}
}
}
-
+
if (addresses.size() > 0) {
// First check whether any of the addresses point to Indirect symbols,
// and if they do, resolve them:
@@ -1028,6 +1029,18 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
thread_plan_sp = std::make_shared<ThreadPlanRunToAddress>(
thread, load_addrs, stop_others);
}
+ // One more case we have to consider is "branch islands". These are regular
+ // TEXT symbols but their names end in .island. They are to allow arm64
+ // code to branch further than the size of the address slot allows. We
+ // just need to single-instruction step in that case.
+ if (!thread_plan_sp && current_name.GetStringRef().ends_with(".island")) {
+ thread_plan_sp = std::make_shared<ThreadPlanStepInstruction>(thread,
+ /* step_over= */ false, /* stop_others */ false, eVoteNoOpinion,
+ eVoteNoOpinion);
+ LLDB_LOG(log,
+ "Stepping one instruction over branch island: '{0}'.",
+ current_name);
+ }
} else {
LLDB_LOGF(log, "Could not find symbol for step through.");
}
diff --git a/lldb/test/API/macosx/branch-islands/Makefile b/lldb/test/API/macosx/branch-islands/Makefile
new file mode 100644
index 0000000000000..8675bbf6f85de
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/Makefile
@@ -0,0 +1,13 @@
+C_SOURCES := main.c foo.c
+CFLAGS_EXTRAS := -std=c99
+
+include Makefile.rules
+
+a.out: main.o padding1.o padding2.o foo.o
+ ${CC} ${LDFLAGS} foo.o padding1.o padding2.o main.o -o a.out
+
+padding1.o: padding1.s
+ ${CC} -c $(SRCDIR)/padding1.s
+
+padding2.o: padding2.s
+ ${CC} -c $(SRCDIR)/padding2.s
diff --git a/lldb/test/API/macosx/branch-islands/TestBranchIslands.py b/lldb/test/API/macosx/branch-islands/TestBranchIslands.py
new file mode 100644
index 0000000000000..b740592113619
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/TestBranchIslands.py
@@ -0,0 +1,34 @@
+"""
+Make sure that we can step in across an arm64 branch island
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+class TestBranchIslandStepping(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ @skipUnlessDarwin
+ def test_step_in_branch_island(self):
+ """Make sure we can step in across a branch island"""
+ self.build()
+ self.main_source_file = lldb.SBFileSpec("main.c")
+ self.do_test()
+
+ def do_test(self):
+ (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+ self, "Set a breakpoint here", self.main_source_file
+ )
+
+ # Make sure that we did manage to generate a branch island for foo:
+ syms = target.FindSymbols("foo.island", lldb.eSymbolTypeCode)
+ self.assertEqual(len(syms), 1, "We did generate an island for foo")
+
+ thread.StepInto()
+ stop_frame = thread.frames[0]
+ self.assertIn("foo", stop_frame.name, "Stepped into foo")
+ var = stop_frame.FindVariable("a_variable_in_foo")
+ self.assertTrue(var.IsValid(), "Found the variable in foo")
diff --git a/lldb/test/API/macosx/branch-islands/foo.c b/lldb/test/API/macosx/branch-islands/foo.c
new file mode 100644
index 0000000000000..6de2c7f36c037
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/foo.c
@@ -0,0 +1,8 @@
+#include <stdio.h>
+
+void
+foo()
+{
+ int a_variable_in_foo = 10;
+ printf("I am foo: %d.\n", a_variable_in_foo);
+}
diff --git a/lldb/test/API/macosx/branch-islands/main.c b/lldb/test/API/macosx/branch-islands/main.c
new file mode 100644
index 0000000000000..b5578bdd715df
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/main.c
@@ -0,0 +1,6 @@
+extern void foo();
+
+int main() {
+ foo(); // Set a breakpoint here
+ return 0;
+}
diff --git a/lldb/test/API/macosx/branch-islands/padding1.s b/lldb/test/API/macosx/branch-islands/padding1.s
new file mode 100644
index 0000000000000..e3bdf7007d757
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/padding1.s
@@ -0,0 +1,3 @@
+.text
+_junk1:
+.space 120*1024*1024
diff --git a/lldb/test/API/macosx/branch-islands/padding2.s b/lldb/test/API/macosx/branch-islands/padding2.s
new file mode 100644
index 0000000000000..187a2c3ebd117
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/padding2.s
@@ -0,0 +1,3 @@
+.text
+_junk2:
+.space 120*1024*1024
|
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
JDevlieghere
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This reverts commit 1ba89ad. This was failing on the Green Dragon bot, which has an older OS than have on hand, so I'll have to dig up one and see why it's failing there.
…lvm#138569) This reverts commit 1ba89ad. This was failing on the Green Dragon bot, which has an older OS than have on hand, so I'll have to dig up one and see why it's failing there.
No description provided.