Skip to content

Commit

Permalink
[lldb][Target] Remove BoundsSafetyTrapFrameRecognizer
Browse files Browse the repository at this point in the history
`VerboseTrapFrameRecognizer` recognizes `__builtin_verbose_trap`
frames, which is used in newer versions of `-fbounds-safety`.

This makes `BoundsSafetyTrapFrameRecognizer` redundant. This patch
removes it (but makes sure `VerboseTrapFrameRecognizer` still recognizes
the old `-fbounds-safety` frame-names).

This also fixes the test failure on Swift CI currently we currently
incorrectly pick the `BoundsSafetyTrapFrameRecognizer` because
the `__builtin_verbose_trap` message contains the string `Bounds check
failed`:
```
/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/test/Shell/BoundsSafety/boundssafetytrap.test:5:10: error: CHECK: expected string not found in input
         ^
<stdin>:1:1: note: scanning from here
(lldb) command source -s 0 '/Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/build/Ninja-ReleaseAssert+stdlib-Release/lldb-macosx-x86_64/test/Shell/lit-lldb-init-quiet'
^
<stdin>:11:69: note: possible intended match here
* thread #1, queue = 'com.apple.main-thread', stop reason = __clang_trap_msg$Bounds check failed$Dereferencing above bounds
                                                                    ^
```
  • Loading branch information
Michael137 committed Dec 12, 2024
1 parent eacd872 commit c4264d6
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 151 deletions.
47 changes: 0 additions & 47 deletions lldb/include/lldb/Target/BoundsSafetyTrapFrameRecognizer.h

This file was deleted.

91 changes: 0 additions & 91 deletions lldb/source/Target/BoundsSafetyTrapFrameRecognizer.cpp

This file was deleted.

1 change: 0 additions & 1 deletion lldb/source/Target/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ add_lldb_library(lldbTarget
AssertFrameRecognizer.cpp
DynamicRegisterInfo.cpp
ExecutionContext.cpp
BoundsSafetyTrapFrameRecognizer.cpp
InstrumentationRuntime.cpp
InstrumentationRuntimeStopInfo.cpp
JITLoader.cpp
Expand Down
6 changes: 0 additions & 6 deletions lldb/source/Target/Process.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,6 @@
#include "lldb/Target/ABI.h"
#include "lldb/Target/AssertFrameRecognizer.h"
#include "lldb/Target/DynamicLoader.h"
/* TO_UPSTREAM(BoundsSafety) ON */
#include "lldb/Target/BoundsSafetyTrapFrameRecognizer.h"
/* TO_UPSTREAM(BoundsSafety) OFF */
#include "lldb/Target/InstrumentationRuntime.h"
#include "lldb/Target/JITLoader.h"
#include "lldb/Target/JITLoaderList.h"
Expand Down Expand Up @@ -546,9 +543,6 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP listener_sp,
// common C LanguageRuntime plugin.
RegisterAssertFrameRecognizer(this);
RegisterVerboseTrapFrameRecognizer(*this);
/* TO_UPSTREAM(BoundsSafety) ON */
RegisterBoundsSafetyTrapFrameRecognizer(*this);
/* TO_UPSTREAM(BoundsSafety) OFF */
}

Process::~Process() {
Expand Down
5 changes: 4 additions & 1 deletion lldb/source/Target/VerboseTrapFrameRecognizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,11 @@ namespace lldb_private {

void RegisterVerboseTrapFrameRecognizer(Process &process) {
RegularExpressionSP module_regex_sp = nullptr;
// Older -fbounds-safety versions didn't have a ClangTrapPrefix, so the name
// of the frame was just the bounds-check failure message. This regex supports
// the old and new style of frames.
auto symbol_regex_sp = std::make_shared<RegularExpression>(
llvm::formatv("^{0}", ClangTrapPrefix).str());
llvm::formatv("^({0}|Bounds check failed|Pointer Check runtime failure)", ClangTrapPrefix).str());

StackFrameRecognizerSP srf_recognizer_sp =
std::make_shared<VerboseTrapFrameRecognizer>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,9 @@ def partial_oob(self, type_name):
def overflow_oob(self, type_name):
return self.get_idx_var_regex(oob_kind=OOBKind.Overflow, type_name=type_name)

def setUp(self):
TestBase.setUp(self)
if _get_bool_config("ios_disclosed", fail_value=False):
self.build()

def test_bidi_known_type_size(self):
self.build()

(_, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
self, r"// break here:.+", lldb.SBFileSpec("bidi_check_known_type_size.c")
)
Expand Down Expand Up @@ -154,6 +151,8 @@ def test_bidi_known_type_size(self):
self.expect("frame variable fams2", patterns=[self.bidi_full_oob("FAMS_t *")])

def test_bidi_unknown_type_size(self):
self.build()

(_, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
self, r"// break here:.+", lldb.SBFileSpec("bidi_check_unknown_type_size.c")
)
Expand Down Expand Up @@ -203,6 +202,8 @@ def test_bidi_unknown_type_size(self):
self.expect("frame variable oob_null", patterns=[self.bidi_full_oob("void *")])

def test_idx_known_type_size(self):
self.build()

(_, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
self, r"// break here:.+", lldb.SBFileSpec("idx_check_known_type_size.c")
)
Expand Down Expand Up @@ -252,6 +253,8 @@ def test_idx_known_type_size(self):
self.expect("frame variable fams2", patterns=[self.full_oob("FAMS_t *")])

def test_idx_unknown_type_size(self):
self.build()

(_, self.process, _, bkpt) = lldbutil.run_to_source_breakpoint(
self, r"// break here:.+", lldb.SBFileSpec("idx_check_unknown_type_size.c")
)
Expand Down

0 comments on commit c4264d6

Please sign in to comment.