[lldb] fix stepping through POSIX trampolines

[mdaniels]

Summary

The DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan() function was
doing the symbol lookup using the demangled name. This stopped working
with https://reviews.llvm.org/D118814. To get things working again, just
use the mangled name for the lookup instead.

[clayborg]

Is there a test that was failing with this? If not can we add one?

[mdaniels]

Added a simple test case that reproduces the issue.

[clayborg]

LGTM, just a question if we want to restrict this test to linux only.

lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py

8 ↗

(On Diff #437741)

Do we want to limit this to linux? I am not sure this will pass the windows buildbots if we don't restrict it

[mdaniels]

@JDevlieghere could you confirm my understanding of the issue here is correct?

It looks like https://github.com/llvm/llvm-project/issues/54250 is the same issue, and should be fix with this as well.

lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py

8 ↗

(On Diff #437741)

Looking at other tests that load a shared library, it seems I should have at least

@skipIfRemote
@skipIfWindows

But I can also just restrict it to the platforms I am able to test on locally, linux and darwin, if there is still concern that other platforms might fail.

[labath]

lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py

5 ↗

(On Diff #437741)

Am I correct in assuming that the "namespace" part here comes from the fact that global functions (e.g. _Z3foov aka foo()) would be found according to their base name (foo) or something?

If yes, that's fine, but it would still be nice to mention (say in a comment or something) that the shared library is an important aspect of the test ("TestStepIntoNamespace" does not convey that notion).

If this can also be reproduced with regular functions, then it'd be better to rename the test into something else (which includes the word "trampoline").

8 ↗

(On Diff #437741)

Neither of these is strictly necessary. Remote shared library tests need additional care as one has to copy the shared library to the remote system, but afaik, you're using the correct incantation which should do that automatically.

Windows shared library support is not at the level of other platforms, but a simple setup like this should work. Or rather: if it fails, it will probably be due to the trampoline aspect, and not the shared library loading. So I think you don't need to add any decorators right now. We can add xfail-windows if it turns out to be failing.

[mdaniels]

lldb/test/API/lang/cpp/step-into-namespace/TestStepIntoNamespace.py

5 ↗

(On Diff #437741)

The lookup will be done with the full demangled name, so in this case it would be "foo::foo()".

The namespace part was just because I had trouble reproducing when reducing the testcase to a plain function call. Though I tried it again and can reproduce the issue fine, so was likely user error on my part.

I will rename as suggested, thanks.

8 ↗

(On Diff #437741)

Good to know, thanks for the feedback

[labath]

Cool, thanks. So do you have commit access, or you need someone to commit this for you?

[mdaniels]

I do not have commit access, so I would need someone to commit for me.

[stella.stamenova]

Looks like this broke the windows lldb bot: https://lab.llvm.org/buildbot/#/builders/83/builds/20566

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

DynamicLoaderPOSIXDYLD::GetStepThroughTrampolinePlan(Thread &thread,
 
   StackFrame *frame = thread.GetStackFrameAtIndex(0).get();
   const SymbolContext &context = frame->GetSymbolContext(eSymbolContextSymbol);
   Symbol *sym = context.symbol;
 
   if (sym == nullptr || !sym->IsTrampoline())
     return thread_plan_sp;
 
-  ConstString sym_name = sym->GetName();
+  ConstString sym_name = sym->GetMangled().GetName(Mangled::ePreferMangled);
   if (!sym_name)
     return thread_plan_sp;
 
   SymbolContextList target_symbols;
   Target &target = thread.GetProcess()->GetTarget();
   const ModuleList &images = target.GetImages();
 
   images.FindSymbolsWithNameAndType(sym_name, eSymbolTypeCode, target_symbols);

lldb/test/API/lang/cpp/step-through-trampoline/Makefile

+DYLIB_NAME := foo
+DYLIB_CXX_SOURCES := foo.cpp
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
+

lldb/test/API/lang/cpp/step-through-trampoline/TestStepThroughTrampoline.py

+import lldb
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+class StepThroughTrampoline(TestBase):
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test(self):
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                   "// Set a breakpoint here",
+                                   lldb.SBFileSpec("main.cpp"),
+                                   extra_images=["foo"])
+        thread.StepInto()
+
+        foo_line = line_number("foo.cpp", '// End up here')
+        self.expect("frame info", substrs=["foo.cpp:{}:".format(foo_line)])

lldb/test/API/lang/cpp/step-through-trampoline/foo.h

+extern void foo();
+

lldb/test/API/lang/cpp/step-through-trampoline/foo.cpp

+#include "foo.h"
+
+void foo() { } // End up here
+

lldb/test/API/lang/cpp/step-through-trampoline/main.cpp

+#include "foo.h"
+
+int main(void) {
+    foo(); // Set a breakpoint here
+}
+