jingham created this revision.
jingham added reviewers: JDevlieghere, kubamracek.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
lldb handles breakpoints in two phases, the "sync" and "async" phase. The
synchronous phase happens early in the event handling, and is for when you need
to do some work before anyone else gets a chance to look at the breakpoint.
It's used by the dynamic loader plugins, for instance.
As such, we're in a place where we haven't quite figured out about the current
event, and are not in a state to handle another one. That makes running the
expression evaluator in synchronous callbacks flakey. The Sanitizer callbacks
were all registered as synchronous, even though their primary job is to call
report generation functions in the target. TTTT, calling functions works
better than I would have expected, but if you rapidly continue enough times
over a sequence of sanitizer reports, you eventually confuse the event state
machine.
I bet with enough thinking we could make expression evaluation in sync
callbacks work, but it would further complicate an already complex system. And
there's no reason for the sanitizer callbacks to be synchronous, they are very
like user report functions in a breakpoint, and should run in the same way. So
the simpler path of converting these to async seems the better one.
This patch switches the report functions from sync to async, adds some testing
for continuing past sequential UBSan reports, and adds a comment telling people
not to use expressions in sync callbacks. The test is not great, in that I
can get this same test code to misbehave quite regularly in Xcode, but I
couldn't reproduce enough of the Xcode environment in a test to produce a
reliably failing test, so this test for the most part passes even without the
fix.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D134927
Files:
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/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
lldb/test/API/functionalities/ubsan/basic/main.c
Index: lldb/test/API/functionalities/ubsan/basic/main.c
===================================================================
--- lldb/test/API/functionalities/ubsan/basic/main.c
+++ 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;
}
Index: lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
===================================================================
--- lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
+++ lldb/test/API/functionalities/ubsan/basic/TestUbsanBasic.py
@@ -86,4 +86,9 @@
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))
+
Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -276,7 +276,7 @@
/*hardware=*/false)
.get();
breakpoint->SetCallback(InstrumentationRuntimeUBSan::NotifyBreakpointHit,
- this, true);
+ this, false);
breakpoint->SetBreakpointKind("undefined-behavior-sanitizer-report");
SetBreakpointID(breakpoint->GetID());
Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -922,7 +922,7 @@
.CreateBreakpoint(symbol_address, internal, hardware)
.get();
breakpoint->SetCallback(InstrumentationRuntimeTSan::NotifyBreakpointHit, this,
- true);
+ false);
breakpoint->SetBreakpointKind("thread-sanitizer-report");
SetBreakpointID(breakpoint->GetID());
Index: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -215,7 +215,7 @@
/*hardware=*/false)
.get();
breakpoint->SetCallback(
- InstrumentationRuntimeMainThreadChecker::NotifyBreakpointHit, this, true);
+ InstrumentationRuntimeMainThreadChecker::NotifyBreakpointHit, this, false);
breakpoint->SetBreakpointKind("main-thread-checker-report");
SetBreakpointID(breakpoint->GetID());
Index: lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
@@ -306,7 +306,7 @@
.CreateBreakpoint(symbol_address, internal, hardware)
.get();
breakpoint->SetCallback(InstrumentationRuntimeASan::NotifyBreakpointHit, this,
- true);
+ false);
breakpoint->SetBreakpointKind("address-sanitizer-report");
SetBreakpointID(breakpoint->GetID());
Index: lldb/include/lldb/Breakpoint/Breakpoint.h
===================================================================
--- lldb/include/lldb/Breakpoint/Breakpoint.h
+++ lldb/include/lldb/Breakpoint/Breakpoint.h
@@ -382,7 +382,10 @@
/// \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);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
