From 7e8aa85bb6e7f90e5df00d69c17663c43054a206 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Fri, 20 Sep 2024 12:10:58 +0100 Subject: [PATCH 1/4] [lldb][FrameRecognizer] Make VerboseTrapFrameRecognizer aware of Swift-C++ interop frames --- .../Target/VerboseTrapFrameRecognizer.cpp | 27 ++++++++++++++++- .../cxx_interop/forward/verbose_trap/Makefile | 4 +++ .../TestSwiftForwardInteropVerboseTrap.py | 22 ++++++++++++++ .../cxx_interop/forward/verbose_trap/aborts.h | 30 +++++++++++++++++++ .../forward/verbose_trap/main.swift | 14 +++++++++ .../forward/verbose_trap/module.modulemap | 5 ++++ 6 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/Makefile create mode 100644 lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py create mode 100644 lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/aborts.h create mode 100644 lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/main.swift create mode 100644 lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/module.modulemap diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp index e9fa1b4d38d17..1e024cf0cee94 100644 --- a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp +++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp @@ -9,6 +9,7 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/RegularExpression.h" #include "clang/CodeGen/ModuleBuilder.h" @@ -16,6 +17,20 @@ using namespace llvm; using namespace lldb; using namespace lldb_private; +#ifdef LLDB_ENABLE_SWIFT +/// Returns true if \ref frame_name is the name of +/// a Swift frame we wouldn't want to stop in. +static bool ShouldHideSwiftFrame(llvm::StringRef frame_name) { + static llvm::SmallVector s_patterns{ + RegularExpression{R"(^(__C\.)?std\.)"}, + RegularExpression{R"(protocol witness for Cxx\.)"}}; + + return llvm::any_of(s_patterns, [&](RegularExpression const &pattern) { + return pattern.Execute(frame_name); + }); +} +#endif // LLDB_ENABLE_SWIFT + /// The 0th frame is the artificial inline frame generated to store /// the verbose_trap message. So, starting with the current parent frame, /// find the first frame that's not inside of the STL. @@ -39,7 +54,17 @@ static StackFrameSP FindMostRelevantFrame(Thread &selected_thread) { // Found a frame outside of the `std` namespace. That's the // first frame in user-code that ended up triggering the // verbose_trap. Hence that's the one we want to display. - if (!frame_name.GetStringRef().starts_with("std::")) + if (!frame_name.GetStringRef().starts_with("std::") +#ifdef LLDB_ENABLE_SWIFT + // In Swift-C++ interop, we generate frames with a "std." + // prefix for functions from libc++. We don't want to + // stop in those frames either. + // + // TODO: could eventually use StackFrame::IsHidden + // and/or StackFrame::IsArtificial as a heuristic too. + && !ShouldHideSwiftFrame(frame_name) +#endif + ) return most_relevant_frame_sp; ++stack_idx; diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/Makefile b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/Makefile new file mode 100644 index 0000000000000..163aa5326750a --- /dev/null +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/Makefile @@ -0,0 +1,4 @@ +SWIFT_SOURCES := main.swift +SWIFT_CXX_INTEROP := 1 +SWIFTFLAGS_EXTRAS = -Xcc -I$(SRCDIR) +include Makefile.rules diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py new file mode 100644 index 0000000000000..903d18633ef89 --- /dev/null +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py @@ -0,0 +1,22 @@ + +""" +Test that verbose_trap works on forward interop mode. +""" +from lldbsuite.test.lldbtest import * +from lldbsuite.test.decorators import * + + +class TestSwiftForwardInteropVerboseTrap(TestBase): + + @swiftTest + def test(self): + self.build() + target = self.dbg.CreateTarget(self.getBuildArtifact("a.out")) + self.assertTrue(target, VALID_TARGET) + + target.BreakpointCreateByName("Break here", "a.out") + process = target.LaunchSimple(None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + # Make sure we stopped in the first user-level frame. + self.assertTrue(self.frame().name.startswith("a.takes<") diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/aborts.h b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/aborts.h new file mode 100644 index 0000000000000..6f185a38d1488 --- /dev/null +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/aborts.h @@ -0,0 +1,30 @@ +#include + +namespace std { +void function_that_aborts() { __builtin_verbose_trap("Error", "from C++"); } + +struct ConstIterator { +private: + int value; + +public: + // Make sure this auto-conforms to UnsafeCxxInputIterator + + using iterator_category = std::input_iterator_tag; + using value_type = int; + using pointer = int *; + using reference = const int &; + using difference_type = int; + + ConstIterator(int value) : value(value) {} + + void operator*() const { std::function_that_aborts(); } + + ConstIterator &operator++() { return *this; } + ConstIterator operator++(int) { return ConstIterator(value); } + + bool operator==(const ConstIterator &other) const { return false; } + bool operator!=(const ConstIterator &other) const { return true; } +}; + +} // namespace std diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/main.swift b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/main.swift new file mode 100644 index 0000000000000..5abfac256863c --- /dev/null +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/main.swift @@ -0,0 +1,14 @@ +import Aborts + +func takes(_ t: some UnsafeCxxInputIterator) { + t.pointee +} + +func main() { + var x = std.ConstIterator(137); + takes(x); + print("Break here"); +} + +main() + diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/module.modulemap b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/module.modulemap new file mode 100644 index 0000000000000..aaca691f038c3 --- /dev/null +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/module.modulemap @@ -0,0 +1,5 @@ +module Aborts { + header "aborts.h" + requires cplusplus +} + From d9596f14bc9feee6e8f3887d2dead4f8ce6e0bde Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Mon, 23 Sep 2024 21:18:30 +0100 Subject: [PATCH 2/4] fixup! make use of IsHidden to check for compiler-generated frames --- .../Language/Swift/SwiftFrameRecognizers.cpp | 41 +++++++++++++++---- .../Target/VerboseTrapFrameRecognizer.cpp | 29 ++++--------- .../TestSwiftForwardInteropVerboseTrap.py | 2 +- 3 files changed, 44 insertions(+), 28 deletions(-) diff --git a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp index 4a05dea27a592..7c816bcd74384 100644 --- a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp +++ b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp @@ -4,6 +4,7 @@ #include "lldb/Symbol/Function.h" #include "lldb/Symbol/SymbolContext.h" #include "lldb/Target/Process.h" +#include "lldb/Target/StackFrameRecognizer.h" #include "lldb/Target/Target.h" #include "lldb/Target/Thread.h" @@ -17,6 +18,7 @@ using namespace lldb; using namespace lldb_private; + namespace lldb_private { /// Holds the stack frame that caused the runtime failure and the inlined stop @@ -150,6 +152,33 @@ class SwiftHiddenFrameRecognizer : public StackFrameRecognizer { bool ShouldHide() override { return true; } }; + /// Returns true if \ref root represents a Swift name + /// that we want to mark hidden by this recognizer. + /// + /// Currently these are: + /// * Async thunks + /// * Auto-conformed protocols in the `std` module + /// + bool ShouldHideSwiftName(NodePointer root) { + using namespace swift_demangle; + + if (NodeAtPath(root, {Node::Kind::Global, + Node::Kind::AsyncAwaitResumePartialFunction}) && + (ChildAtPath(root, {Node::Kind::BackDeploymentFallback}) || + ChildAtPath(root, {Node::Kind::PartialApplyForwarder}))) + return true; + + if (auto witness_node = + NodeAtPath(root, {Node::Kind::Global, Node::Kind::ProtocolWitness, + Node::Kind::ProtocolConformance})) + if (auto module_node = ChildAtPath(witness_node, {Node::Kind::Module}); + module_node && module_node->getText() == "__C_Synthesized") + return true; + ; + + return false; + } + public: SwiftHiddenFrameRecognizer() : m_hidden_frame(new SwiftHiddenFrame()) {} @@ -168,13 +197,11 @@ class SwiftHiddenFrameRecognizer : public StackFrameRecognizer { using namespace swift::Demangle; using namespace swift_demangle; Context demangle_ctx; - NodePointer nodes = - SwiftLanguageRuntime::DemangleSymbolAsNode(symbol_name, demangle_ctx); - if (NodeAtPath(nodes, {Node::Kind::Global, - Node::Kind::AsyncAwaitResumePartialFunction}) && - (ChildAtPath(nodes, {Node::Kind::BackDeploymentFallback}) || - ChildAtPath(nodes, {Node::Kind::PartialApplyForwarder}))) - return m_hidden_frame; + if (NodePointer nodes = SwiftLanguageRuntime::DemangleSymbolAsNode( + symbol_name, demangle_ctx)) + if (ShouldHideSwiftName(nodes)) + return m_hidden_frame; + return {}; } }; diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp index 1e024cf0cee94..b5f5dd340e1be 100644 --- a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp +++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp @@ -17,20 +17,6 @@ using namespace llvm; using namespace lldb; using namespace lldb_private; -#ifdef LLDB_ENABLE_SWIFT -/// Returns true if \ref frame_name is the name of -/// a Swift frame we wouldn't want to stop in. -static bool ShouldHideSwiftFrame(llvm::StringRef frame_name) { - static llvm::SmallVector s_patterns{ - RegularExpression{R"(^(__C\.)?std\.)"}, - RegularExpression{R"(protocol witness for Cxx\.)"}}; - - return llvm::any_of(s_patterns, [&](RegularExpression const &pattern) { - return pattern.Execute(frame_name); - }); -} -#endif // LLDB_ENABLE_SWIFT - /// The 0th frame is the artificial inline frame generated to store /// the verbose_trap message. So, starting with the current parent frame, /// find the first frame that's not inside of the STL. @@ -51,18 +37,21 @@ static StackFrameSP FindMostRelevantFrame(Thread &selected_thread) { if (!frame_name) return nullptr; - // Found a frame outside of the `std` namespace. That's the + // Find a frame outside of the `std` namespace. That's the // first frame in user-code that ended up triggering the // verbose_trap. Hence that's the one we want to display. - if (!frame_name.GetStringRef().starts_with("std::") + // + // IsHidden will get us to the first non-implementation detail + // frame. But that could still be in the `std` namespace, so + // check the namespace prefix too. + if (!frame_name.GetStringRef().starts_with("std::") && + !most_relevant_frame_sp->IsHidden() #ifdef LLDB_ENABLE_SWIFT // In Swift-C++ interop, we generate frames with a "std." // prefix for functions from libc++. We don't want to // stop in those frames either. - // - // TODO: could eventually use StackFrame::IsHidden - // and/or StackFrame::IsArtificial as a heuristic too. - && !ShouldHideSwiftFrame(frame_name) + && !frame_name.GetStringRef().starts_with("__C.std.") && + !frame_name.GetStringRef().starts_with("std.") #endif ) return most_relevant_frame_sp; diff --git a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py index 903d18633ef89..601c01c76883d 100644 --- a/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py +++ b/lldb/test/API/lang/swift/cxx_interop/forward/verbose_trap/TestSwiftForwardInteropVerboseTrap.py @@ -19,4 +19,4 @@ def test(self): self.assertTrue(process, PROCESS_IS_VALID) # Make sure we stopped in the first user-level frame. - self.assertTrue(self.frame().name.startswith("a.takes<") + self.assertTrue(self.frame().name.startswith("a.takes<")) From 0b21e7075f8d8924ead5e72f956be59cf8139e7d Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Wed, 25 Sep 2024 14:35:28 +0100 Subject: [PATCH 3/4] fixup! check source file/line to determine if we want to hide a frame --- .../Language/Swift/SwiftFrameRecognizers.cpp | 15 +++++++++++++++ lldb/source/Target/VerboseTrapFrameRecognizer.cpp | 10 +--------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp index 7c816bcd74384..e1cf0528ab262 100644 --- a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp +++ b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp @@ -188,12 +188,27 @@ class SwiftHiddenFrameRecognizer : public StackFrameRecognizer { RecognizeFrame(lldb::StackFrameSP frame_sp) override { if (!frame_sp) return {}; + + // Hide compiler-generated frames. + if (frame_sp->IsArtificial()) + return m_hidden_frame; + const auto &sc = frame_sp->GetSymbolContext(lldb::eSymbolContextFunction); if (!sc.function) return {}; + FileSpec source_file; + uint32_t line_no; + sc.function->GetStartLineSourceInfo(source_file, line_no); + // FIXME: these frames should be marked artificial + // by the Swift compiler. + if (source_file.GetFilename() == "" + && line_no == 0) + return m_hidden_frame; + auto symbol_name = sc.function->GetMangled().GetMangledName().GetStringRef(); + using namespace swift::Demangle; using namespace swift_demangle; Context demangle_ctx; diff --git a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp index b5f5dd340e1be..66c0d77526172 100644 --- a/lldb/source/Target/VerboseTrapFrameRecognizer.cpp +++ b/lldb/source/Target/VerboseTrapFrameRecognizer.cpp @@ -45,15 +45,7 @@ static StackFrameSP FindMostRelevantFrame(Thread &selected_thread) { // frame. But that could still be in the `std` namespace, so // check the namespace prefix too. if (!frame_name.GetStringRef().starts_with("std::") && - !most_relevant_frame_sp->IsHidden() -#ifdef LLDB_ENABLE_SWIFT - // In Swift-C++ interop, we generate frames with a "std." - // prefix for functions from libc++. We don't want to - // stop in those frames either. - && !frame_name.GetStringRef().starts_with("__C.std.") && - !frame_name.GetStringRef().starts_with("std.") -#endif - ) + !most_relevant_frame_sp->IsHidden()) return most_relevant_frame_sp; ++stack_idx; From 567a5d641b636a073be77a41f568ecee61619d57 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Wed, 25 Sep 2024 14:36:28 +0100 Subject: [PATCH 4/4] fixup! clang-format --- lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp index e1cf0528ab262..aa2c679779e82 100644 --- a/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp +++ b/lldb/source/Plugins/Language/Swift/SwiftFrameRecognizers.cpp @@ -202,8 +202,7 @@ class SwiftHiddenFrameRecognizer : public StackFrameRecognizer { sc.function->GetStartLineSourceInfo(source_file, line_no); // FIXME: these frames should be marked artificial // by the Swift compiler. - if (source_file.GetFilename() == "" - && line_no == 0) + if (source_file.GetFilename() == "" && line_no == 0) return m_hidden_frame; auto symbol_name =