Revert branch island experiments

[jimingham]

This test is failing because when we step to what is the branch island address and ask for its symbol, we can't resolve the symbol, and just call it the last padding symbol plus a bajillion.

That has nothing to do with the changes in this patch, but I'll revert this and keep trying to figure out why symbol reading on this bot is wrong.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

This test is failing because when we step to what is the branch island address and ask for its symbol, we can't resolve the symbol, and just call it the last padding symbol plus a bajillion.

That has nothing to do with the changes in this patch, but I'll revert this and keep trying to figure out why symbol reading on this bot is wrong.

---

Full diff: https://github.com/llvm/llvm-project/pull/139192.diff

9 Files Affected:

• (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp (+7-34)
• (removed) lldb/test/API/macosx/branch-islands/Makefile (-16)
• (removed) lldb/test/API/macosx/branch-islands/TestBranchIslands.py (-72)
• (removed) lldb/test/API/macosx/branch-islands/foo.c (-6)
• (removed) lldb/test/API/macosx/branch-islands/main.c (-6)
• (removed) lldb/test/API/macosx/branch-islands/padding1.s (-3)
• (removed) lldb/test/API/macosx/branch-islands/padding2.s (-3)
• (removed) lldb/test/API/macosx/branch-islands/padding3.s (-3)
• (removed) lldb/test/API/macosx/branch-islands/padding4.s (-3)

diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 6c3040ef1a1da..e25c4ff55e408 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -26,7 +26,6 @@
 #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"
@@ -924,15 +923,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 (current_name) {
+      if (trampoline_name) {
         const ModuleList &images = target_sp->GetImages();
 
         SymbolContextList code_symbols;
-        images.FindSymbolsWithNameAndType(current_name, eSymbolTypeCode,
+        images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeCode,
                                           code_symbols);
         for (const SymbolContext &context : code_symbols) {
           Address addr = context.GetFunctionOrSymbolAddress();
@@ -946,8 +945,8 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
         }
 
         SymbolContextList reexported_symbols;
-        images.FindSymbolsWithNameAndType(current_name, eSymbolTypeReExported,
-                                          reexported_symbols);
+        images.FindSymbolsWithNameAndType(
+            trampoline_name, eSymbolTypeReExported, reexported_symbols);
         for (const SymbolContext &context : reexported_symbols) {
           if (context.symbol) {
             Symbol *actual_symbol =
@@ -969,7 +968,7 @@ DynamicLoaderDarwin::GetStepThroughTrampolinePlan(Thread &thread,
         }
 
         SymbolContextList indirect_symbols;
-        images.FindSymbolsWithNameAndType(current_name, eSymbolTypeResolver,
+        images.FindSymbolsWithNameAndType(trampoline_name, eSymbolTypeResolver,
                                           indirect_symbols);
 
         for (const SymbolContext &context : indirect_symbols) {
@@ -1029,32 +1028,6 @@ 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 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);
-    // FIXME: this is extra logging so I can figure out why this test is failing
-    // on the bot but not locally with all the same tools, etc...
-    if (thread_plan_sp && is_branch_island) {
-      if (log) {
-        StreamString s;
-        thread_plan_sp->GetDescription(&s, eDescriptionLevelVerbose);
-        LLDB_LOGF(log, "Am at a branch island, but already had plan: \n\t%s", s.GetData());
-      }
-    }
-    if (!thread_plan_sp && is_branch_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
deleted file mode 100644
index ff341522e15de..0000000000000
--- a/lldb/test/API/macosx/branch-islands/Makefile
+++ /dev/null
@@ -1,16 +0,0 @@
-C_SOURCES := main.c foo.c
-CFLAGS_EXTRAS := -std=c99
-
-include Makefile.rules
-
-a.out: main.o padding1.o padding2.o padding3.o padding4.o foo.o
-	${CC} ${LDFLAGS} -fuse-ld=/usr/bin/ld foo.o padding1.o padding2.o padding3.o padding4.o main.o -o a.out
-
-%.o: $(SRCDIR)/%.s
-	${CC} -c $<
-
-#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
deleted file mode 100644
index 2d768d35aad03..0000000000000
--- a/lldb/test/API/macosx/branch-islands/TestBranchIslands.py
+++ /dev/null
@@ -1,72 +0,0 @@
-"""
-Make sure that we can step in across an arm64 branch island
-"""
-
-import os
-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
-
-    @skipUnlessAppleSilicon
-    def test_step_in_branch_island(self):
-        """Make sure we can step in across a branch island"""
-        self.build(debug_info="dwarf")
-        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")
-
-        # Gathering some info to dump in case of failure:
-        trace_before = lldbutil.print_stacktrace(thread, True)
-        func_before = thread.frames[0].function
-
-        log_file_path = os.path.join(self.getBuildDir(), "step-log.txt")
-        self.runCmd(f"log enable -f {log_file_path} lldb step")
-        
-        thread.StepInto()
-        stop_frame = thread.frames[0]
-        # This is failing on the bot, but I can't reproduce the failure
-        # locally.  Let's see if we can dump some more info here to help
-        # figure out what went wrong...
-        if stop_frame.name.find("foo") == -1:
-            stream = lldb.SBStream()
-            print("Branch island symbols: ")
-            syms[0].GetDescription(stream)
-            for i in range(0, 6):
-                for sep in ["", "."]:
-                    syms = target.FindSymbols(
-                        f"foo.island{sep}{i}", lldb.eSymbolTypeCode
-                    )
-                    if len(syms) > 0:
-                        stream.Print("\n")
-                        syms[0].GetDescription(stream)
-
-            print(stream.GetData())
-            print(f"Start backtrace:")
-            print(trace_before)
-            print(f"\n'main' disassembly:\n{lldbutil.disassemble(target, func_before)}")
-            print("\nEnd backtrace:\n")
-            lldbutil.print_stacktrace(thread)
-            print(
-                f"\nStop disassembly:\n {lldbutil.disassemble(target, stop_frame.function)}"
-            )
-            with open(log_file_path, "r") as f:
-                data = f.read()
-                print("Step Log:")
-                print(data)
-
-        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
deleted file mode 100644
index a5dd2e59e1d82..0000000000000
--- a/lldb/test/API/macosx/branch-islands/foo.c
+++ /dev/null
@@ -1,6 +0,0 @@
-#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
deleted file mode 100644
index b5578bdd715df..0000000000000
--- a/lldb/test/API/macosx/branch-islands/main.c
+++ /dev/null
@@ -1,6 +0,0 @@
-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
deleted file mode 100644
index 4911e53b0240d..0000000000000
--- a/lldb/test/API/macosx/branch-islands/padding1.s
+++ /dev/null
@@ -1,3 +0,0 @@
-.text
-_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
deleted file mode 100644
index 5ad1bad11263b..0000000000000
--- a/lldb/test/API/macosx/branch-islands/padding2.s
+++ /dev/null
@@ -1,3 +0,0 @@
-.text
-_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
deleted file mode 100644
index 9f614eecf56d9..0000000000000
--- a/lldb/test/API/macosx/branch-islands/padding3.s
+++ /dev/null
@@ -1,3 +0,0 @@
-.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
deleted file mode 100644
index 12896cf5e5b8e..0000000000000
--- a/lldb/test/API/macosx/branch-islands/padding4.s
+++ /dev/null
@@ -1,3 +0,0 @@
-.text
-_padding4:
-.space 120*1024*1024