[Lldb-commits] [lldb] 9d081a7 - Revert "Make the stop-on-sharedlibrary-events setting work."
Jim Ingham via lldb-commits
lldb-commits at lists.llvm.org
Fri Mar 19 12:39:59 PDT 2021
Author: Jim Ingham
Date: 2021-03-19T12:38:41-07:00
New Revision: 9d081a7ffe5c2f9575f77bedd6cbf4385287aeec
URL: https://github.com/llvm/llvm-project/commit/9d081a7ffe5c2f9575f77bedd6cbf4385287aeec
DIFF: https://github.com/llvm/llvm-project/commit/9d081a7ffe5c2f9575f77bedd6cbf4385287aeec.diff
LOG: Revert "Make the stop-on-sharedlibrary-events setting work."
This reverts commit 9406d43138811ac4dfd0ab31434f65a649bc882e.
I messed up a test, and when I got it right it was failing. The changed logic
doesn't work quite right (now the async callback called at sync time is
forcing us to stop. I need to be a little more careful about that.
Added:
Modified:
lldb/source/Breakpoint/BreakpointOptions.cpp
lldb/source/Target/StopInfo.cpp
Removed:
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
################################################################################
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 24427835980e..2fdb53e52723 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -453,12 +453,9 @@ bool BreakpointOptions::InvokeCallback(StoppointCallbackContext *context,
: nullptr,
context, break_id, break_loc_id);
} else if (IsCallbackSynchronous()) {
- // If a synchronous callback is called at async time, we will say we
- // should stop, we're really expression no opinion about stopping, and
- // the StopInfoBreakpoint::PerformAction will note whether an async
- // callback had already made a claim to stop or not based on the incoming
- // values of m_should_stop & m_should_stop_is_valid.
- return true;
+ // If a synchronous callback is called at async time, it should not say
+ // to stop.
+ return false;
}
}
return true;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 1cb582e83cc1..7e830c6e2bed 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -305,20 +305,6 @@ 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
@@ -436,10 +422,9 @@ class StopInfoBreakpoint : public StopInfo {
bool precondition_result =
bp_loc_sp->GetBreakpoint().EvaluatePrecondition(context);
- if (!precondition_result) {
- actually_said_continue = true;
+ if (!precondition_result)
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.
@@ -477,7 +462,6 @@ 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;
}
}
@@ -520,9 +504,6 @@ class StopInfoBreakpoint : public StopInfo {
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.
@@ -536,15 +517,9 @@ 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
deleted file mode 100644
index e87808bd222d..000000000000
--- a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/Makefile
+++ /dev/null
@@ -1,16 +0,0 @@
-CXX_SOURCES := main.cpp
-USE_LIBDL := 1
-
-a.out: lib_a
-
-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
deleted file mode 100644
index 98c4eb89ff54..000000000000
--- a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/TestStopOnSharedlibraryEvents.py
+++ /dev/null
@@ -1,96 +0,0 @@
-""" 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()
-
- def test_auto_continue(self):
- def auto_continue(bkpt):
- bkpt.SetAutoContinue(True)
- self.do_test(auto_continue)
-
- def test_failing_condition(self):
- def condition(bkpt):
- bkpt.SetCondition("1 == 2")
- self.do_test(condition)
-
- 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, _, _ = 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")
-
- thread = process.GetSelectedThread()
- self.assertEqual(thread.stop_reason, lldb.eStopReasonBreakpoint, "We attributed the stop to the breakpoint")
- self.assertEqual(thread.GetStopReasonDataCount(), 2, "Only hit one breakpoint")
- bkpt_no = thread.GetStopReasonDataAtIndex(0)
- self.assertEqual(bkpt_no, load_bkpt.id, "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")
- thread = process.GetSelectedThread()
- self.assertEqual(thread.stop_reason, lldb.eStopReasonBreakpoint, "We didn't hit some breakpoint")
- self.assertEqual(thread.GetStopReasonDataCount(), 2, "Only hit one breakpoint")
- bkpt_no = thread.GetStopReasonDataAtIndex(0)
- self.assertEqual(bkpt_no, backstop_bkpt_2.id, "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
deleted file mode 100644
index b7b702c5d62d..000000000000
--- a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/a.cpp
+++ /dev/null
@@ -1,6 +0,0 @@
-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
deleted file mode 100644
index 5a347e60db3a..000000000000
--- a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/b.cpp
+++ /dev/null
@@ -1,6 +0,0 @@
-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
deleted file mode 100644
index 96b1e1df445b..000000000000
--- a/lldb/test/API/functionalities/stop-on-sharedlibrary-load/main.cpp
+++ /dev/null
@@ -1,27 +0,0 @@
-#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