Author: Jim Ingham Date: 2022-10-03T18:10:28-07:00 New Revision: 852a4bdb25d145884f61cd812e66611e65fd2dd9
URL: https://github.com/llvm/llvm-project/commit/852a4bdb25d145884f61cd812e66611e65fd2dd9 DIFF: https://github.com/llvm/llvm-project/commit/852a4bdb25d145884f61cd812e66611e65fd2dd9.diff LOG: Change the Sanitizer report breakpoint callbacks to asynchronous. The synchronous callbacks are not intended to start the target running during the callback, and doing so is flakey. This patch converts them to being regular async callbacks, and adds some testing for sequential reports that have caused problems in the field. Differential Revision: https://reviews.llvm.org/D134927 Added: Modified: lldb/include/lldb/Breakpoint/Breakpoint.h lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp lldb/source/Target/StopInfo.cpp lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py lldb/test/API/functionalities/ubsan/basic/main.c Removed: ################################################################################ diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index 701d0823bd282..7490982cb05ba 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -382,7 +382,10 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// \param[in] is_synchronous /// If \b true the callback will be run on the private event thread /// before the stop event gets reported. If false, the callback will get - /// handled on the public event thread after the stop has been posted. + /// handled on the public event thread while the stop event is being + /// pulled off the event queue. + /// Note: synchronous callbacks cannot cause the target to run, in + /// particular, they should not try to run the expression evaluator. void SetCallback(BreakpointHitCallback callback, void *baton, bool is_synchronous = false); diff --git a/lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp b/lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp index 4746112873112..72dfbd5866224 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp @@ -299,14 +299,15 @@ void InstrumentationRuntimeASan::Activate() { if (symbol_address == LLDB_INVALID_ADDRESS) return; - bool internal = true; - bool hardware = false; + const bool internal = true; + const bool hardware = false; + const bool sync = false; Breakpoint *breakpoint = process_sp->GetTarget() .CreateBreakpoint(symbol_address, internal, hardware) .get(); breakpoint->SetCallback(InstrumentationRuntimeASan::NotifyBreakpointHit, this, - true); + sync); breakpoint->SetBreakpointKind("address-sanitizer-report"); SetBreakpointID(breakpoint->GetID()); diff --git a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp index a5c23615309d5..e22cc86116ce5 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp @@ -214,8 +214,9 @@ void InstrumentationRuntimeMainThreadChecker::Activate() { .CreateBreakpoint(symbol_address, /*internal=*/true, /*hardware=*/false) .get(); + const bool sync = false; breakpoint->SetCallback( - InstrumentationRuntimeMainThreadChecker::NotifyBreakpointHit, this, true); + InstrumentationRuntimeMainThreadChecker::NotifyBreakpointHit, this, sync); breakpoint->SetBreakpointKind("main-thread-checker-report"); SetBreakpointID(breakpoint->GetID()); diff --git a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp index 910992c48a7dc..425b0baa453f8 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp @@ -915,14 +915,15 @@ void InstrumentationRuntimeTSan::Activate() { if (symbol_address == LLDB_INVALID_ADDRESS) return; - bool internal = true; - bool hardware = false; + const bool internal = true; + const bool hardware = false; + const bool sync = false; Breakpoint *breakpoint = process_sp->GetTarget() .CreateBreakpoint(symbol_address, internal, hardware) .get(); breakpoint->SetCallback(InstrumentationRuntimeTSan::NotifyBreakpointHit, this, - true); + sync); breakpoint->SetBreakpointKind("thread-sanitizer-report"); SetBreakpointID(breakpoint->GetID()); diff --git a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp index 5544c5f08f3be..7ea8b4697d204 100644 --- a/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp +++ b/lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp @@ -275,8 +275,9 @@ void InstrumentationRuntimeUBSan::Activate() { .CreateBreakpoint(symbol_address, /*internal=*/true, /*hardware=*/false) .get(); + const bool sync = false; breakpoint->SetCallback(InstrumentationRuntimeUBSan::NotifyBreakpointHit, - this, true); + this, sync); breakpoint->SetBreakpointKind("undefined-behavior-sanitizer-report"); SetBreakpointID(breakpoint->GetID()); diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp index d8fb10441d892..225234c0ffbad 100644 --- a/lldb/source/Target/StopInfo.cpp +++ b/lldb/source/Target/StopInfo.cpp @@ -362,29 +362,21 @@ class StopInfoBreakpoint : public StopInfo { " not running commands to avoid recursion."); bool ignoring_breakpoints = process->GetIgnoreBreakpointsInExpressions(); - if (ignoring_breakpoints) { - m_should_stop = false; - // Internal breakpoints will always stop. - for (size_t j = 0; j < num_owners; j++) { - lldb::BreakpointLocationSP bp_loc_sp = - bp_site_sp->GetOwnerAtIndex(j); - if (bp_loc_sp->GetBreakpoint().IsInternal()) { - m_should_stop = true; - break; - } - } - } else { - m_should_stop = true; + // Internal breakpoints should be allowed to do their job, we + // can make sure they don't do anything that would cause recursive + // command execution: + if (!m_was_all_internal) { + m_should_stop = !ignoring_breakpoints; + LLDB_LOGF(log, + "StopInfoBreakpoint::PerformAction - in expression, " + "continuing: %s.", + m_should_stop ? "true" : "false"); + Debugger::ReportWarning( + "hit breakpoint while running function, skipping commands " + "and conditions to prevent recursion", + process->GetTarget().GetDebugger().GetID()); + return; } - LLDB_LOGF(log, - "StopInfoBreakpoint::PerformAction - in expression, " - "continuing: %s.", - m_should_stop ? "true" : "false"); - Debugger::ReportWarning( - "hit breakpoint while running function, skipping commands and " - "conditions to prevent recursion", - process->GetTarget().GetDebugger().GetID()); - return; } StoppointCallbackContext context(event_ptr, exe_ctx, false); diff --git a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py index 2af97098a431c..8ef7a0210fe2e 100644 --- a/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py +++ b/lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py @@ -86,4 +86,9 @@ def ubsan_tests(self): self.assertEqual(os.path.basename(data["filename"]), "main.c") self.assertEqual(data["line"], self.line_align) - self.runCmd("continue") + for count in range(0,8): + process.Continue() + stop_reason = thread.GetStopReason() + self.assertEqual(stop_reason, lldb.eStopReasonInstrumentation, + "Round {0} wasn't instrumentation".format(count)) + diff --git a/lldb/test/API/functionalities/ubsan/basic/main.c b/lldb/test/API/functionalities/ubsan/basic/main.c index 4991fc074d09d..2fc9663d57571 100644 --- a/lldb/test/API/functionalities/ubsan/basic/main.c +++ b/lldb/test/API/functionalities/ubsan/basic/main.c @@ -1,4 +1,16 @@ int main() { int data[4]; - return *(int *)(((char *)&data[0]) + 2); // align line + int result = *(int *)(((char *)&data[0]) + 2); // align line + + int *p = data + 5; // Index 5 out of bounds for type 'int [4]' + *p = data + 5; + *p = data + 5; + *p = data + 5; + *p = data + 5; + *p = data + 5; + *p = data + 5; + *p = data + 5; + *p = data + 5; + + return 0; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits