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 <jo...@devlieghere.com>
Date: Sun, 16 Feb 2025 13:27:07 -0800
Subject: [PATCH 1/4] [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 <jo...@devlieghere.com>
Date: Mon, 17 Feb 2025 08:12:32 -0800
Subject: [PATCH 2/4] 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 <jo...@devlieghere.com>
Date: Mon, 17 Feb 2025 08:46:56 -0800
Subject: [PATCH 3/4] 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 <jo...@devlieghere.com>
Date: Mon, 17 Feb 2025 08:47:01 -0800
Subject: [PATCH 4/4] 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:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
