[Lldb-commits] [lldb] [lldb] Support stepping through C++ thunks (PR #127419)
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 17 14:14:37 PST 2025
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/127419
>From c533d71a56fa7d6347b1698c63c8626513e70ab0 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Sun, 16 Feb 2025 13:27:07 -0800
Subject: [PATCH 1/8] [lldb] Support stepping over C++ thunks
This PR fixes LLDB stepping out, rather than stepping through a C++
thunk. The implementation is based on, and upstreams, the support for
runtime thunks in the Swift fork.
Fixes #43413
---
lldb/include/lldb/Target/LanguageRuntime.h | 2 +
.../CPlusPlus/CPPLanguageRuntime.cpp | 6 +++
.../CPlusPlus/CPPLanguageRuntime.h | 3 ++
.../Target/ThreadPlanShouldStopHere.cpp | 51 +++++++++++++++----
lldb/test/API/lang/cpp/thunk/Makefile | 3 ++
lldb/test/API/lang/cpp/thunk/TestThunk.py | 24 +++++++++
lldb/test/API/lang/cpp/thunk/main.cpp | 36 +++++++++++++
7 files changed, 114 insertions(+), 11 deletions(-)
create mode 100644 lldb/test/API/lang/cpp/thunk/Makefile
create mode 100644 lldb/test/API/lang/cpp/thunk/TestThunk.py
create mode 100644 lldb/test/API/lang/cpp/thunk/main.cpp
diff --git a/lldb/include/lldb/Target/LanguageRuntime.h b/lldb/include/lldb/Target/LanguageRuntime.h
index f9ae2dc589632..7e4c11df0da7f 100644
--- a/lldb/include/lldb/Target/LanguageRuntime.h
+++ b/lldb/include/lldb/Target/LanguageRuntime.h
@@ -201,6 +201,8 @@ class LanguageRuntime : public Runtime, public PluginInterface {
return false;
}
+ virtual bool IsSymbolARuntimeThunk(const Symbol &symbol) { return false; }
+
// Given the name of a runtime symbol (e.g. in Objective-C, an ivar offset
// symbol), try to determine from the runtime what the value of that symbol
// would be. Useful when the underlying binary is stripped.
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index 42fa54634841c..e648b9665bef6 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -476,3 +476,9 @@ CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
return ret_plan_sp;
}
+
+bool CPPLanguageRuntime::IsSymbolARuntimeThunk(const Symbol &symbol) {
+ llvm::outs() << symbol.GetMangled().GetMangledName().GetStringRef() << '\n';
+ return symbol.GetMangled().GetMangledName().GetStringRef().starts_with(
+ "_ZThn");
+}
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
index 57cfe28245808..05639e9798917 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.h
@@ -78,6 +78,9 @@ class CPPLanguageRuntime : public LanguageRuntime {
bool stop_others) override;
bool IsAllowedRuntimeValue(ConstString name) override;
+
+ bool IsSymbolARuntimeThunk(const Symbol &symbol) override;
+
protected:
// Classes that inherit from CPPLanguageRuntime can see and modify these
CPPLanguageRuntime(Process *process);
diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp
index e72f8d8f51a20..723d7965c3467 100644
--- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp
+++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp
@@ -8,6 +8,7 @@
#include "lldb/Target/ThreadPlanShouldStopHere.h"
#include "lldb/Symbol/Symbol.h"
+#include "lldb/Target/LanguageRuntime.h"
#include "lldb/Target/RegisterContext.h"
#include "lldb/Target/Thread.h"
#include "lldb/Utility/LLDBLog.h"
@@ -76,6 +77,18 @@ bool ThreadPlanShouldStopHere::DefaultShouldStopHereCallback(
}
}
+ // Check whether the frame we are in is a language runtime thunk, only for
+ // step out:
+ if (operation == eFrameCompareOlder) {
+ Symbol *symbol = frame->GetSymbolContext(eSymbolContextSymbol).symbol;
+ if (symbol) {
+ ProcessSP process_sp(current_plan->GetThread().GetProcess());
+ for (auto *runtime : process_sp->GetLanguageRuntimes()) {
+ if (runtime->IsSymbolARuntimeThunk(*symbol))
+ should_stop_here = false;
+ }
+ }
+ }
// Always avoid code with line number 0.
// FIXME: At present the ShouldStop and the StepFromHere calculate this
// independently. If this ever
@@ -109,18 +122,34 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback(
if (sc.line_entry.line == 0) {
AddressRange range = sc.line_entry.range;
-
- // If the whole function is marked line 0 just step out, that's easier &
- // faster than continuing to step through it.
bool just_step_out = false;
- if (sc.symbol && sc.symbol->ValueIsAddress()) {
- Address symbol_end = sc.symbol->GetAddress();
- symbol_end.Slide(sc.symbol->GetByteSize() - 1);
- if (range.ContainsFileAddress(sc.symbol->GetAddress()) &&
- range.ContainsFileAddress(symbol_end)) {
- LLDB_LOGF(log, "Stopped in a function with only line 0 lines, just "
- "stepping out.");
- just_step_out = true;
+ if (sc.symbol) {
+ ProcessSP process_sp(current_plan->GetThread().GetProcess());
+
+ // If this is a runtime thunk, step through it, rather than stepping out
+ // because it's marked line 0.
+ bool is_thunk = false;
+ for (auto *runtime : process_sp->GetLanguageRuntimes()) {
+ if (runtime->IsSymbolARuntimeThunk(*sc.symbol)) {
+ LLDB_LOGF(log, "In runtime thunk %s - stepping out.",
+ sc.symbol->GetName().GetCString());
+ is_thunk = true;
+ }
+ }
+
+ // If the whole function is marked line 0 just step out, that's easier &
+ // faster than continuing to step through it.
+ // FIXME: This assumes that the function is a single line range. It could
+ // be a series of contiguous line 0 ranges. Check for that too.
+ if (!is_thunk && sc.symbol->ValueIsAddress()) {
+ Address symbol_end = sc.symbol->GetAddress();
+ symbol_end.Slide(sc.symbol->GetByteSize() - 1);
+ if (range.ContainsFileAddress(sc.symbol->GetAddress()) &&
+ range.ContainsFileAddress(symbol_end)) {
+ LLDB_LOGF(log, "Stopped in a function with only line 0 lines, just "
+ "stepping out.");
+ just_step_out = true;
+ }
}
}
if (!just_step_out) {
diff --git a/lldb/test/API/lang/cpp/thunk/Makefile b/lldb/test/API/lang/cpp/thunk/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/thunk/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/thunk/TestThunk.py b/lldb/test/API/lang/cpp/thunk/TestThunk.py
new file mode 100644
index 0000000000000..bd17401f1d935
--- /dev/null
+++ b/lldb/test/API/lang/cpp/thunk/TestThunk.py
@@ -0,0 +1,24 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ThunkTest(TestBase):
+ def test(self):
+ self.build()
+ lldbutil.run_to_name_breakpoint(self, "testit")
+
+ self.expect(
+ "step",
+ STEP_IN_SUCCEEDED,
+ substrs=["stop reason = step in", "Derived1::doit"],
+ )
+
+ self.runCmd("continue")
+
+ self.expect(
+ "step",
+ STEP_IN_SUCCEEDED,
+ substrs=["stop reason = step in", "Derived2::doit"],
+ )
diff --git a/lldb/test/API/lang/cpp/thunk/main.cpp b/lldb/test/API/lang/cpp/thunk/main.cpp
new file mode 100644
index 0000000000000..36f2d7c2ce421
--- /dev/null
+++ b/lldb/test/API/lang/cpp/thunk/main.cpp
@@ -0,0 +1,36 @@
+#include <stdio.h>
+
+class Base1 {
+public:
+ virtual ~Base1() {}
+};
+
+class Base2 {
+public:
+ virtual void doit() = 0;
+};
+
+Base2 *b = nullptr;
+
+class Derived1 : public Base1, public Base2 {
+public:
+ virtual void doit() { printf("Derived1\n"); }
+};
+
+class Derived2 : public Base2 {
+public:
+ virtual void doit() { printf("Derived2\n"); }
+};
+
+void testit() { b->doit(); }
+
+int main() {
+
+ b = new Derived1();
+ testit();
+
+ b = new Derived2();
+ testit();
+
+ return 0;
+}
>From b8bd89ccf623847f10ed6646d5e6c86fc6e49c45 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 08:12:32 -0800
Subject: [PATCH 2/8] Remove debug statement, support virtual thunks and
document the mangling
---
.../LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index e648b9665bef6..fa5a069f86b99 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -478,7 +478,10 @@ CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
}
bool CPPLanguageRuntime::IsSymbolARuntimeThunk(const Symbol &symbol) {
- llvm::outs() << symbol.GetMangled().GetMangledName().GetStringRef() << '\n';
+ // Virtual function override thunks come in two forms. Those overriding from a
+ // non-virtual base, with fixed this adjustments, use a "Th" prefix and encode
+ // the required adjustment offset, probably negative, indicated by a 'n'
+ // prefix, and the encoding of the target function.
return symbol.GetMangled().GetMangledName().GetStringRef().starts_with(
- "_ZThn");
+ "_ZTh");
}
>From 73674673983731704705af98f5c003e6ea297896 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 08:46:56 -0800
Subject: [PATCH 3/8] Support all thunk types
---
.../LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index fa5a069f86b99..48850955285d9 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -478,10 +478,10 @@ CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
}
bool CPPLanguageRuntime::IsSymbolARuntimeThunk(const Symbol &symbol) {
- // Virtual function override thunks come in two forms. Those overriding from a
- // non-virtual base, with fixed this adjustments, use a "Th" prefix and encode
- // the required adjustment offset, probably negative, indicated by a 'n'
- // prefix, and the encoding of the target function.
- return symbol.GetMangled().GetMangledName().GetStringRef().starts_with(
- "_ZTh");
+ llvm::StringRef mangled_name = symbol.GetMangled().GetMangledName().GetStringRef();
+ // Virtual function overriding from a non-virtual base use a "Th" prefix.
+ // Virtual function overriding from a virtual base must use a "Tv" prefix.
+ // Virtual function overriding thunks with covariant returns use a "Tc" prefix.
+ return mangled_name.starts_with("_ZTh") || mangled_name.starts_with("_ZTv") ||
+ mangled_name.starts_with("_ZTc");
}
>From b5904f01c64b2c6005f986886ce00ac507172bd2 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 08:47:01 -0800
Subject: [PATCH 4/8] Remove nullptr from test
---
lldb/test/API/lang/cpp/thunk/main.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/test/API/lang/cpp/thunk/main.cpp b/lldb/test/API/lang/cpp/thunk/main.cpp
index 36f2d7c2ce421..5ba2fcf7a6a76 100644
--- a/lldb/test/API/lang/cpp/thunk/main.cpp
+++ b/lldb/test/API/lang/cpp/thunk/main.cpp
@@ -10,7 +10,7 @@ class Base2 {
virtual void doit() = 0;
};
-Base2 *b = nullptr;
+Base2 *b;
class Derived1 : public Base1, public Base2 {
public:
>From ac40a1eab9754ba7551d846097e00e49443b8324 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 08:51:44 -0800
Subject: [PATCH 5/8] Formatting
---
.../LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
index 48850955285d9..21a5ebe53073a 100644
--- a/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/CPlusPlus/CPPLanguageRuntime.cpp
@@ -478,10 +478,12 @@ CPPLanguageRuntime::GetStepThroughTrampolinePlan(Thread &thread,
}
bool CPPLanguageRuntime::IsSymbolARuntimeThunk(const Symbol &symbol) {
- llvm::StringRef mangled_name = symbol.GetMangled().GetMangledName().GetStringRef();
+ llvm::StringRef mangled_name =
+ symbol.GetMangled().GetMangledName().GetStringRef();
// Virtual function overriding from a non-virtual base use a "Th" prefix.
// Virtual function overriding from a virtual base must use a "Tv" prefix.
- // Virtual function overriding thunks with covariant returns use a "Tc" prefix.
+ // Virtual function overriding thunks with covariant returns use a "Tc"
+ // prefix.
return mangled_name.starts_with("_ZTh") || mangled_name.starts_with("_ZTv") ||
mangled_name.starts_with("_ZTc");
}
>From 074a58ec50117d46aaa8029f87db110547d85ed0 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 11:50:48 -0800
Subject: [PATCH 6/8] Add a test for stepping out thunks
---
lldb/test/API/lang/cpp/thunk/TestThunk.py | 22 +++++++++++++++++++++-
lldb/test/API/lang/cpp/thunk/main.cpp | 7 +++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/lldb/test/API/lang/cpp/thunk/TestThunk.py b/lldb/test/API/lang/cpp/thunk/TestThunk.py
index bd17401f1d935..231b78842cbef 100644
--- a/lldb/test/API/lang/cpp/thunk/TestThunk.py
+++ b/lldb/test/API/lang/cpp/thunk/TestThunk.py
@@ -5,10 +5,11 @@
class ThunkTest(TestBase):
- def test(self):
+ def test_step_through_thunk(self):
self.build()
lldbutil.run_to_name_breakpoint(self, "testit")
+ # Make sure we step through the thunk into Derived1::doit
self.expect(
"step",
STEP_IN_SUCCEEDED,
@@ -22,3 +23,22 @@ def test(self):
STEP_IN_SUCCEEDED,
substrs=["stop reason = step in", "Derived2::doit"],
)
+
+ def test_step_out_thunk(self):
+ self.build()
+ lldbutil.run_to_name_breakpoint(self, "testit_debug")
+
+ # Make sure we step out of the thunk and don't end up in Derived1::doit.
+ self.expect(
+ "step",
+ STEP_IN_SUCCEEDED,
+ substrs=["stop reason = step in", "main at main.cpp"],
+ )
+
+ self.runCmd("continue")
+
+ self.expect(
+ "step",
+ STEP_IN_SUCCEEDED,
+ substrs=["stop reason = step in", "Derived2::doit_debug"],
+ )
diff --git a/lldb/test/API/lang/cpp/thunk/main.cpp b/lldb/test/API/lang/cpp/thunk/main.cpp
index 5ba2fcf7a6a76..bbbf4ab101a0d 100644
--- a/lldb/test/API/lang/cpp/thunk/main.cpp
+++ b/lldb/test/API/lang/cpp/thunk/main.cpp
@@ -8,6 +8,7 @@ class Base1 {
class Base2 {
public:
virtual void doit() = 0;
+ virtual void doit_debug() = 0;
};
Base2 *b;
@@ -15,22 +16,28 @@ Base2 *b;
class Derived1 : public Base1, public Base2 {
public:
virtual void doit() { printf("Derived1\n"); }
+ virtual void __attribute__((nodebug)) doit_debug() { printf("Derived1 (no debug)\n"); }
};
class Derived2 : public Base2 {
public:
virtual void doit() { printf("Derived2\n"); }
+ virtual void doit_debug() { printf("Derived2 (debug)\n"); }
};
void testit() { b->doit(); }
+void testit_debug() { b->doit_debug(); }
+
int main() {
b = new Derived1();
testit();
+ testit_debug();
b = new Derived2();
testit();
+ testit_debug();
return 0;
}
>From d826c101f8dd1cdf4566aa9b25168091797870b8 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 11:51:01 -0800
Subject: [PATCH 7/8] Address Ismail's feedback
---
lldb/source/Target/ThreadPlanShouldStopHere.cpp | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lldb/source/Target/ThreadPlanShouldStopHere.cpp b/lldb/source/Target/ThreadPlanShouldStopHere.cpp
index 723d7965c3467..fa6bc08a9914d 100644
--- a/lldb/source/Target/ThreadPlanShouldStopHere.cpp
+++ b/lldb/source/Target/ThreadPlanShouldStopHere.cpp
@@ -80,12 +80,13 @@ bool ThreadPlanShouldStopHere::DefaultShouldStopHereCallback(
// Check whether the frame we are in is a language runtime thunk, only for
// step out:
if (operation == eFrameCompareOlder) {
- Symbol *symbol = frame->GetSymbolContext(eSymbolContextSymbol).symbol;
- if (symbol) {
+ if (Symbol *symbol = frame->GetSymbolContext(eSymbolContextSymbol).symbol) {
ProcessSP process_sp(current_plan->GetThread().GetProcess());
for (auto *runtime : process_sp->GetLanguageRuntimes()) {
- if (runtime->IsSymbolARuntimeThunk(*symbol))
+ if (runtime->IsSymbolARuntimeThunk(*symbol)) {
should_stop_here = false;
+ break;
+ }
}
}
}
@@ -134,6 +135,7 @@ ThreadPlanSP ThreadPlanShouldStopHere::DefaultStepFromHereCallback(
LLDB_LOGF(log, "In runtime thunk %s - stepping out.",
sc.symbol->GetName().GetCString());
is_thunk = true;
+ break;
}
}
>From 6f1493b2be229fdf03e7b6c5512ec8b2d7977196 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Mon, 17 Feb 2025 14:04:39 -0800
Subject: [PATCH 8/8] Format the test
---
lldb/test/API/lang/cpp/thunk/main.cpp | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lldb/test/API/lang/cpp/thunk/main.cpp b/lldb/test/API/lang/cpp/thunk/main.cpp
index bbbf4ab101a0d..8f876c96ae2e2 100644
--- a/lldb/test/API/lang/cpp/thunk/main.cpp
+++ b/lldb/test/API/lang/cpp/thunk/main.cpp
@@ -16,7 +16,9 @@ Base2 *b;
class Derived1 : public Base1, public Base2 {
public:
virtual void doit() { printf("Derived1\n"); }
- virtual void __attribute__((nodebug)) doit_debug() { printf("Derived1 (no debug)\n"); }
+ virtual void __attribute__((nodebug)) doit_debug() {
+ printf("Derived1 (no debug)\n");
+ }
};
class Derived2 : public Base2 {
More information about the lldb-commits
mailing list