Skip to content

Conversation

@Michael137
Copy link

@Michael137 Michael137 commented Sep 20, 2024

This patch ensures that if libc++ is called from Swift (via Swift interop for example), and triggers a __builtin_verbose_trap, we don't stop in the Swift-C++ compiler-generated shims.

E.g., in the example test-case, the stacktrace looks like:

frame #0: 0x0000000102998c00 a.out`std::function_that_aborts() [inlined] __clang_trap_msg$Error$from C++ at aborts.h:0
frame #1: 0x0000000102998c00 a.out`std::function_that_aborts() at aborts.h:4:31
frame #2: 0x0000000102998c1c a.out`std::ConstIterator::operator*(this=0x0000600003954420) const at aborts.h:21:28
frame #3: 0x0000000102998ab0 a.out`std.ConstIterator.pointee.read() at <compiler-generated>:0
frame #4: 0x0000000102998a14 a.out`protocol witness for UnsafeCxxInputIterator.pointee.read in conformance std.ConstIterator at <compiler-generated>:0
frame #5: 0x0000000102998760 a.out`takes<ConstIterator>(t=Aborts.ConstIterator @ 0x000000016d46aeb8) at main.swift:4:7
frame #6: 0x00000001029985e0 a.out`main() at main.swift:9:3
frame #7: 0x0000000102998584 a.out`main at main.swift:13:1
frame #8: 0x000000019053df20 dyld`start + 1988

We want to stop in frame 5, which is where the call into std started.

rdar://136357737

static bool ShouldHideSwiftFrame(llvm::StringRef frame_name) {
static llvm::SmallVector<RegularExpression, 2> s_patterns{
RegularExpression{R"(^(__C\.)?std\.)"},
RegularExpression{R"(protocol witness for Cxx\.)"}};

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be theoretically possible to have the Swift hidden frame recognizer take over in this case? It seems wrong to maintain this list in two places.

if (auto module_node = ChildAtPath(witness_node, {Node::Kind::Module});
module_node && module_node->getText() == "__C_Synthesized")
return true;
;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray ;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, maybe it'd be okay to remove all protocol witnesses.

// 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.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this check live inside SwiftHiddenFrameRecognizer(), assuming that we never want to stop there?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depends, do we want to hide all frames from the std module? With C++ interop that could make sense. But wasn't sure.


@swiftTest
def test(self):
self.build()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just do:

lldbutil.run_to_source_breakpoint(self,  "Break here", lldb.SBFileSpec("main.swift")

?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it because we don't expect tp hit the breakpoint?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, lldbutil.run_to_source_breakpoint asserts that we stopped at some breakpoint. Which won't happen for this test.

Might be worth creating a wrapper for the "just launch the executable" case. That way we could make use of __buitin_debugtrap in more tests too.

// check the namespace prefix too.
if (!frame_name.GetStringRef().starts_with("std::") &&
!most_relevant_frame_sp->IsHidden()
#ifdef LLDB_ENABLE_SWIFT

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this live in the language plugin? The goal is to reduce the amount of changes in generic (non-Swift specific) code and we definitely don't want to add more.

@Michael137 Michael137 merged commit 14abc9e into stable/20240723 Oct 1, 2024
@Michael137 Michael137 deleted the lldb/verbose-trap-interop-to-20240723 branch October 1, 2024 12:05
@Michael137
Copy link
Author

Argh forgot to squash the commits...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants