[Lldb-commits] [lldb] 852a4bd - Change the Sanitizer report breakpoint callbacks to asynchronous.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Mon Oct 3 18:10:50 PDT 2022
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;
}
More information about the lldb-commits
mailing list