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; -} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
