https://github.com/jimingham updated
https://github.com/llvm/llvm-project/pull/138781
>From 9bcf63344ba475c73029c5c44975bfab57117f80 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Mon, 5 May 2025 15:02:22 -0700
Subject: [PATCH 1/3] Revert "Revert "Handle step-in over a Darwin "branch
island". (#138330)""
This reverts commit b1b7e6df31a54c4da5b30454ea910d89b0ce924b.
---
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 27 ++++++++++----
lldb/test/API/macosx/branch-islands/Makefile | 13 +++++++
.../branch-islands/TestBranchIslands.py | 35 +++++++++++++++++++
lldb/test/API/macosx/branch-islands/foo.c | 6 ++++
lldb/test/API/macosx/branch-islands/main.c | 6 ++++
.../test/API/macosx/branch-islands/padding1.s | 3 ++
.../test/API/macosx/branch-islands/padding2.s | 3 ++
7 files changed, 86 insertions(+), 7 deletions(-)
create mode 100644 lldb/test/API/macosx/branch-islands/Makefile
create mode 100644 lldb/test/API/macosx/branch-islands/TestBranchIslands.py
create mode 100644 lldb/test/API/macosx/branch-islands/foo.c
create mode 100644 lldb/test/API/macosx/branch-islands/main.c
create mode 100644 lldb/test/API/macosx/branch-islands/padding1.s
create mode 100644 lldb/test/API/macosx/branch-islands/padding2.s
diff --git
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index e25c4ff55e408..52c50cd88902a 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;
+ ConstString current_name =
+ current_symbol->GetMangled().GetName(Mangled::ePreferMangled);
if (current_symbol->IsTrampoline()) {
- ConstString trampoline_name =
- current_symbol->GetMangled().GetName(Mangled::ePreferMangled);
- 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();
@@ -945,8 +946,8 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread
&thread,
}
SymbolContextList reexported_symbols;
- images.FindSymbolsWithNameAndType(
- trampoline_name, eSymbolTypeReExported, reexported_symbols);
+ images.FindSymbolsWithNameAndType(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) {
@@ -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..b397e0c229b08
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/TestBranchIslands.py
@@ -0,0 +1,35 @@
+"""
+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..a5dd2e59e1d82
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/foo.c
@@ -0,0 +1,6 @@
+#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
>From 2d614757a2f1d2f78b1a844cff66451b66f58b75 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Tue, 6 May 2025 16:31:21 -0700
Subject: [PATCH 2/3] Fix the branch island handling to account for the other
possible forms: '.island2' and '.island.2'. Also add more padding so that we
produce an island that takes several jumps.
---
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 13 +++++++++----
lldb/test/API/macosx/branch-islands/Makefile | 15 +++++++++------
lldb/test/API/macosx/branch-islands/padding1.s | 2 +-
lldb/test/API/macosx/branch-islands/padding2.s | 2 +-
lldb/test/API/macosx/branch-islands/padding3.s | 3 +++
lldb/test/API/macosx/branch-islands/padding4.s | 3 +++
6 files changed, 26 insertions(+), 12 deletions(-)
create mode 100644 lldb/test/API/macosx/branch-islands/padding3.s
create mode 100644 lldb/test/API/macosx/branch-islands/padding4.s
diff --git
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 52c50cd88902a..ad3b63cf59135 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -1030,10 +1030,15 @@
DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
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")) {
+ // TEXT symbols but their names end in .island plus maybe a .digit suffix.
+ // 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.
+ static const char *g_branch_island_pattern = "\\.island\\.?[0-9]*$";
+ static RegularExpression g_branch_island_regex(g_branch_island_pattern);
+
+ bool is_branch_island = g_branch_island_regex.Execute(current_name);
+ if (!thread_plan_sp && is_branch_island) {
thread_plan_sp = std::make_shared<ThreadPlanStepInstruction>(
thread,
/* step_over= */ false, /* stop_others */ false, eVoteNoOpinion,
diff --git a/lldb/test/API/macosx/branch-islands/Makefile
b/lldb/test/API/macosx/branch-islands/Makefile
index 8675bbf6f85de..062e947f6d6ee 100644
--- a/lldb/test/API/macosx/branch-islands/Makefile
+++ b/lldb/test/API/macosx/branch-islands/Makefile
@@ -3,11 +3,14 @@ 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
+a.out: main.o padding1.o padding2.o padding3.o padding4.o foo.o
+ ${CC} ${LDFLAGS} foo.o padding1.o padding2.o padding3.o padding4.o
main.o -o a.out
-padding1.o: padding1.s
- ${CC} -c $(SRCDIR)/padding1.s
+%.o: $(SRCDIR)/%.s
+ ${CC} -c $<
-padding2.o: padding2.s
- ${CC} -c $(SRCDIR)/padding2.s
+#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/padding1.s
b/lldb/test/API/macosx/branch-islands/padding1.s
index e3bdf7007d757..4911e53b0240d 100644
--- a/lldb/test/API/macosx/branch-islands/padding1.s
+++ b/lldb/test/API/macosx/branch-islands/padding1.s
@@ -1,3 +1,3 @@
.text
-_junk1:
+_padding1:
.space 120*1024*1024
diff --git a/lldb/test/API/macosx/branch-islands/padding2.s
b/lldb/test/API/macosx/branch-islands/padding2.s
index 187a2c3ebd117..5ad1bad11263b 100644
--- a/lldb/test/API/macosx/branch-islands/padding2.s
+++ b/lldb/test/API/macosx/branch-islands/padding2.s
@@ -1,3 +1,3 @@
.text
-_junk2:
+_padding2:
.space 120*1024*1024
diff --git a/lldb/test/API/macosx/branch-islands/padding3.s
b/lldb/test/API/macosx/branch-islands/padding3.s
new file mode 100644
index 0000000000000..9f614eecf56d9
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/padding3.s
@@ -0,0 +1,3 @@
+.text
+_padding3:
+.space 120*1024*1024
diff --git a/lldb/test/API/macosx/branch-islands/padding4.s
b/lldb/test/API/macosx/branch-islands/padding4.s
new file mode 100644
index 0000000000000..12896cf5e5b8e
--- /dev/null
+++ b/lldb/test/API/macosx/branch-islands/padding4.s
@@ -0,0 +1,3 @@
+.text
+_padding4:
+.space 120*1024*1024
>From a5944b752915ee2f3ccfda63332d33111d59ff15 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jing...@apple.com>
Date: Tue, 6 May 2025 16:53:34 -0700
Subject: [PATCH 3/3] formatting
---
.../DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git
a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index ad3b63cf59135..578ab12268ea3 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -1030,9 +1030,9 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread
&thread,
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 plus maybe a .digit suffix.
- // 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
+ // TEXT symbols but their names end in .island plus maybe a .digit suffix.
+ // 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.
static const char *g_branch_island_pattern = "\\.island\\.?[0-9]*$";
static RegularExpression g_branch_island_regex(g_branch_island_pattern);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
