[Lldb-commits] [lldb] c8faa8c - Make the stop-on-sharedlibrary-events setting work.
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 24 11:15:20 PDT 2021
Author: Jim Ingham
Date: 2021-03-24T11:15:11-07:00
New Revision: c8faa8c2669c1867ef6ac33466f219a39d5faaa7
URL: https://github.com/llvm/llvm-project/commit/c8faa8c2669c1867ef6ac33466f219a39d5faaa7
DIFF: https://github.com/llvm/llvm-project/commit/c8faa8c2669c1867ef6ac33466f219a39d5faaa7.diff
LOG: Make the stop-on-sharedlibrary-events setting work.
The StopInfoBreakpoint::PerformAction was overriding the synchronous
breakpoint's ShouldStop report. Fix that and add a test.
This fixes two bugs in the original submission:
1) Actually generate both dylibs by including the second one in the Makefile
2) Don't ask synchronous callbacks for their opinion on whether to stop
in the async context, that info is taken care of by recording the m_should_stop
on entry to PerformAction.
Differential Revision: https://reviews.llvm.org/D98914
Added:
lldb/test/API/functionalities/stop-on-sharedlibrary-load/Makefile
lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
lldb/test/API/functionalities/stop-on-sharedlibrary-load/a.cpp
lldb/test/API/functionalities/stop-on-sharedlibrary-load/b.cpp
lldb/test/API/functionalities/stop-on-sharedlibrary-load/main.cpp
Modified:
lldb/include/lldb/Breakpoint/BreakpointLocation.h
lldb/source/Breakpoint/BreakpointLocation.cpp
lldb/source/Breakpoint/BreakpointOptions.cpp
lldb/source/Target/StopInfo.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 4e1c57a40435..a12696be91e3 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -230,6 +230,12 @@ class BreakpointLocation
/// \b true if the target should stop at this breakpoint and \b
/// false not.
bool InvokeCallback(StoppointCallbackContext *context);
+
+ /// Report whether the callback for this location is synchronous or not.
+ ///
+ /// \return
+ /// \b true if the callback is synchronous and \b false if not.
+ bool IsCallbackSynchronous();
/// Returns whether we should resolve Indirect functions in setting the
/// breakpoint site for this location.
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index d3d6ea08bdb3..617b89bf1964 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -195,6 +195,13 @@ bool BreakpointLocation::InvokeCallback(StoppointCallbackContext *context) {
return m_owner.InvokeCallback(context, GetID());
}
+bool BreakpointLocation::IsCallbackSynchronous() {
+ if (m_options_up != nullptr && m_options_up->HasCallback())
+ return m_options_up->IsCallbackSynchronous();
+ else
+ return m_owner.GetOptions()->IsCallbackSynchronous();
+}
+
void BreakpointLocation::SetCallback(BreakpointHitCallback callback,
void *baton, bool is_synchronous) {
// The default "Baton" class will keep a copy of "baton" and won't free or
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 2fdb53e52723..920f843d9764 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -453,8 +453,6 @@ bool BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
: nullptr,
context, break_id, break_loc_id);
} else if (IsCallbackSynchronous()) {
- // If a synchronous callback is called at async time, it should not say
- // to stop.
return false;
}
}
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 7e830c6e2bed..95efd0a17127 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -305,6 +305,20 @@ class StopInfoBreakpoint : public StopInfo {
// location said we should stop. But that's better than not running
// all the callbacks.
+ // There's one other complication here. We may have run an async
+ // breakpoint callback that said we should stop. We only want to
+ // override that if another breakpoint action says we shouldn't
+ // stop. If nobody else has an opinion, then we should stop if the
+ // async callback says we should. An example of this is the async
+ // shared library load notification breakpoint and the setting
+ // stop-on-sharedlibrary-events.
+ // We'll keep the async value in async_should_stop, and track whether
+ // anyone said we should NOT stop in actually_said_continue.
+ bool async_should_stop = false;
+ if (m_should_stop_is_valid)
+ async_should_stop = m_should_stop;
+ bool actually_said_continue = false;
+
m_should_stop = false;
// We don't select threads as we go through them testing breakpoint
@@ -422,9 +436,10 @@ class StopInfoBreakpoint : public StopInfo {
bool precondition_result =
bp_loc_sp->GetBreakpoint().EvaluatePrecondition(context);
- if (!precondition_result)
+ if (!precondition_result) {
+ actually_said_continue = true;
continue;
-
+ }
// Next run the condition for the breakpoint. If that says we
// should stop, then we'll run the callback for the breakpoint. If
// the callback says we shouldn't stop that will win.
@@ -462,6 +477,7 @@ class StopInfoBreakpoint : public StopInfo {
// the condition fails. We've already bumped it by the time
// we get here, so undo the bump:
bp_loc_sp->UndoBumpHitCount();
+ actually_said_continue = true;
continue;
}
}
@@ -494,16 +510,22 @@ class StopInfoBreakpoint : public StopInfo {
// When we figure out how to nest breakpoint hits then this will
// change.
- Debugger &debugger = thread_sp->CalculateTarget()->GetDebugger();
- bool old_async = debugger.GetAsyncExecution();
- debugger.SetAsyncExecution(true);
+ // Don't run async callbacks in PerformAction. They have already
+ // been taken into account with async_should_stop.
+ if (!bp_loc_sp->IsCallbackSynchronous()) {
+ Debugger &debugger = thread_sp->CalculateTarget()->GetDebugger();
+ bool old_async = debugger.GetAsyncExecution();
+ debugger.SetAsyncExecution(true);
- callback_says_stop = bp_loc_sp->InvokeCallback(&context);
+ callback_says_stop = bp_loc_sp->InvokeCallback(&context);
- debugger.SetAsyncExecution(old_async);
+ debugger.SetAsyncExecution(old_async);
- if (callback_says_stop && auto_continue_says_stop)
- m_should_stop = true;
+ if (callback_says_stop && auto_continue_says_stop)
+ m_should_stop = true;
+ else
+ actually_said_continue = true;
+ }
// If we are going to stop for this breakpoint, then remove the
// breakpoint.
@@ -517,9 +539,15 @@ class StopInfoBreakpoint : public StopInfo {
// here.
if (HasTargetRunSinceMe()) {
m_should_stop = false;
+ actually_said_continue = true;
break;
}
}
+ // At this point if nobody actually told us to continue, we should
+ // give the async breakpoint callback a chance to weigh in:
+ if (!actually_said_continue && !m_should_stop) {
+ m_should_stop = async_should_stop;
+ }
}
// We've figured out what this stop wants to do, so mark it as valid so
// we don't compute it again.
diff --git a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/Makefile b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/Makefile
new file mode 100644
index 000000000000..4abcab84eac2
--- /dev/null
+++ b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/Makefile
@@ -0,0 +1,16 @@
+CXX_SOURCES := main.cpp
+USE_LIBDL := 1
+
+a.out: lib_a lib_b
+
+include Makefile.rules
+
+lib_a:
+ $(MAKE) -f $(MAKEFILE_RULES) \
+ DYLIB_ONLY=YES DYLIB_CXX_SOURCES=a.cpp DYLIB_NAME=load_a
+
+lib_b:
+ $(MAKE) -f $(MAKEFILE_RULES) \
+ DYLIB_ONLY=YES DYLIB_CXX_SOURCES=b.cpp DYLIB_NAME=load_b
+
+
diff --git a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
new file mode 100644
index 000000000000..81f00f288f60
--- /dev/null
+++ b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
@@ -0,0 +1,99 @@
+""" Test that stop-on-sharedlibrary-events works and cooperates with breakpoints. """
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestStopOnSharedlibraryEvents(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ @skipIfRemote
+ @skipIfWindows
+ @no_debug_info_test
+ def test_stopping_breakpoints(self):
+ self.do_test()
+
+ @skipIfRemote
+ @skipIfWindows
+ @no_debug_info_test
+ def test_auto_continue(self):
+ def auto_continue(bkpt):
+ bkpt.SetAutoContinue(True)
+ self.do_test(auto_continue)
+
+ @skipIfRemote
+ @skipIfWindows
+ @no_debug_info_test
+ def test_failing_condition(self):
+ def condition(bkpt):
+ bkpt.SetCondition("1 == 2")
+ self.do_test(condition)
+
+ @skipIfRemote
+ @skipIfWindows
+ @no_debug_info_test
+ def test_continue_callback(self):
+ def bkpt_callback(bkpt):
+ bkpt.SetScriptCallbackBody("return False")
+ self.do_test(bkpt_callback)
+
+ def do_test(self, bkpt_modifier = None):
+ self.build()
+ main_spec = lldb.SBFileSpec("main.cpp")
+ # Launch and stop before the dlopen call.
+ target, process, thread, _ = lldbutil.run_to_source_breakpoint(self,
+ "// Set a breakpoint here", main_spec)
+
+ # Now turn on shared library events, continue and make sure we stop for the event.
+ self.runCmd("settings set target.process.stop-on-sharedlibrary-events 1")
+ self.addTearDownHook(lambda: self.runCmd(
+ "settings set target.process.stop-on-sharedlibrary-events 0"))
+
+ # Since I don't know how to check that we are at the "right place" to stop for
+ # shared library events, make an breakpoint after the load is done and
+ # make sure we don't stop there:
+ backstop_bkpt_1 = target.BreakpointCreateBySourceRegex("Set another here - we should not hit this one", main_spec)
+ self.assertGreater(backstop_bkpt_1.GetNumLocations(), 0, "Set our second breakpoint")
+
+ process.Continue()
+ self.assertEqual(process.GetState(), lldb.eStateStopped, "We didn't stop for the load")
+ self.assertEqual(backstop_bkpt_1.GetHitCount(), 0, "Hit our backstop breakpoint")
+
+ # We should be stopped after the library is loaded, check that:
+ found_it = False
+ for module in target.modules:
+ if module.file.basename.find("load_a") > -1:
+ found_it = True
+ break
+ self.assertTrue(found_it, "Found the loaded module.")
+
+ # Now capture the place where we stopped so we can set a breakpoint and make
+ # sure the breakpoint there works correctly:
+ load_address = process.GetSelectedThread().frames[0].addr
+ load_bkpt = target.BreakpointCreateBySBAddress(load_address)
+ self.assertGreater(load_bkpt.GetNumLocations(), 0, "Set the load breakpoint")
+
+ backstop_bkpt_1.SetEnabled(False)
+
+ backstop_bkpt_2 = target.BreakpointCreateBySourceRegex("Set a third here - we should not hit this one", main_spec)
+ self.assertGreater(backstop_bkpt_2.GetNumLocations(), 0, "Set our third breakpoint")
+
+ if bkpt_modifier == None:
+ process.Continue()
+ self.assertEqual(process.GetState(), lldb.eStateStopped, "We didn't stop for the load")
+ self.assertEqual(backstop_bkpt_2.GetHitCount(), 0, "Hit our backstop breakpoint")
+ self.assertEqual(thread.stop_reason, lldb.eStopReasonBreakpoint, "We attributed the stop to the breakpoint")
+ self.assertEqual(load_bkpt.GetHitCount(), 1, "We hit our breakpoint at the load address")
+ else:
+ bkpt_modifier(load_bkpt)
+ process.Continue()
+ self.assertEqual(process.GetState(), lldb.eStateStopped, "We didn't stop")
+ self.assertTrue(thread.IsValid(), "Our thread was no longer valid.")
+ self.assertEqual(thread.stop_reason, lldb.eStopReasonBreakpoint, "We didn't hit some breakpoint")
+ self.assertEqual(backstop_bkpt_2.GetHitCount(), 1, "We continued to the right breakpoint")
+
+
+
+
+
diff --git a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/a.cpp b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/a.cpp
new file mode 100644
index 000000000000..b7b702c5d62d
--- /dev/null
+++ b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/a.cpp
@@ -0,0 +1,6 @@
+extern int a_has_a_function();
+
+int
+a_has_a_function() {
+ return 10;
+}
diff --git a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/b.cpp b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/b.cpp
new file mode 100644
index 000000000000..5a347e60db3a
--- /dev/null
+++ b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/b.cpp
@@ -0,0 +1,6 @@
+extern int b_has_a_function();
+
+int
+b_has_a_function() {
+ return 100;
+}
diff --git a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/main.cpp b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/main.cpp
new file mode 100644
index 000000000000..96b1e1df445b
--- /dev/null
+++ b/lldb/test/API/functionalities/stop-on-sharedlibrary-load/main.cpp
@@ -0,0 +1,27 @@
+#include "dylib.h"
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int main(int argc, char const *argv[]) {
+ const char *a_name = "load_a";
+ void *a_dylib_handle = NULL;
+
+ a_dylib_handle = dylib_open(a_name); // Set a breakpoint here.
+ if (a_dylib_handle == NULL) { // Set another here - we should not hit this one
+ fprintf(stderr, "%s\n", dylib_last_error());
+ exit(1);
+ }
+
+ const char *b_name = "load_b";
+ void *b_dylib_handle = NULL;
+
+ b_dylib_handle = dylib_open(b_name);
+ if (b_dylib_handle == NULL) { // Set a third here - we should not hit this one
+ fprintf(stderr, "%s\n", dylib_last_error());
+ exit(1);
+ }
+
+ return 0;
+}
More information about the lldb-commits
mailing list