Author: Felipe de Azevedo Piovezan Date: 2022-09-19T12:56:12-04:00 New Revision: 974958749827bc4fb88b67feab4bcd7536f70456
URL: https://github.com/llvm/llvm-project/commit/974958749827bc4fb88b67feab4bcd7536f70456 DIFF: https://github.com/llvm/llvm-project/commit/974958749827bc4fb88b67feab4bcd7536f70456.diff LOG: [lldb] Reset breakpoint hit count before new runs A common debugging pattern is to set a breakpoint that only stops after a number of hits is recorded. The current implementation never resets the hit count of breakpoints; as such, if a user re-`run`s their program, the debugger will never stop on such a breakpoint again. This behavior is arguably undesirable, as it renders such breakpoints ineffective on all but the first run. This commit changes the implementation of the `Will{Launch, Attach}` methods so that they reset the _target's_ breakpoint hitcounts. Differential Revision: https://reviews.llvm.org/D133858 Added: lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp Modified: lldb/include/lldb/Breakpoint/Breakpoint.h lldb/include/lldb/Breakpoint/BreakpointList.h lldb/include/lldb/Breakpoint/BreakpointLocation.h lldb/include/lldb/Breakpoint/BreakpointLocationList.h lldb/include/lldb/Target/Process.h lldb/include/lldb/Target/Target.h lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Breakpoint/BreakpointList.cpp lldb/source/Breakpoint/BreakpointLocationList.cpp lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/source/Target/Process.cpp lldb/source/Target/Target.cpp lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py Removed: ################################################################################ diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index 39c6d0b7f3a77..701d0823bd282 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -330,6 +330,9 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, /// The current hit count for all locations. uint32_t GetHitCount() const; + /// Resets the current hit count for all locations. + void ResetHitCount(); + /// If \a one_shot is \b true, breakpoint will be deleted on first hit. void SetOneShot(bool one_shot); diff --git a/lldb/include/lldb/Breakpoint/BreakpointList.h b/lldb/include/lldb/Breakpoint/BreakpointList.h index 346972ec3a1fc..a7399d385f6f0 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointList.h +++ b/lldb/include/lldb/Breakpoint/BreakpointList.h @@ -138,6 +138,9 @@ class BreakpointList { void ClearAllBreakpointSites(); + /// Resets the hit count of all breakpoints. + void ResetHitCounts(); + /// Sets the passed in Locker to hold the Breakpoint List mutex. /// /// \param[in] lock diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h index a6d1232162fce..2a4f9fc01bf32 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h @@ -87,6 +87,9 @@ class BreakpointLocation /// Return the current Hit Count. uint32_t GetHitCount() const { return m_hit_counter.GetValue(); } + /// Resets the current Hit Count. + void ResetHitCount() { m_hit_counter.Reset(); } + /// Return the current Ignore Count. /// /// \return diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h index 4b36c919ee3c5..f76a8fcfdd7e7 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h @@ -126,6 +126,9 @@ class BreakpointLocationList { /// Hit count of all locations in this list. uint32_t GetHitCount() const; + /// Resets the hit count of all locations in this list. + void ResetHitCount(); + /// Enquires of the breakpoint location in this list with ID \a breakID /// whether we should stop. /// diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h index 4f5ae45a431ec..6975eb8029de0 100644 --- a/lldb/include/lldb/Target/Process.h +++ b/lldb/include/lldb/Target/Process.h @@ -882,13 +882,28 @@ class Process : public std::enable_shared_from_this<Process>, // Plug-in Process Control Overrides //================================================================== + /// Called before attaching to a process. + /// + /// \return + /// Returns an error object. + Status WillAttachToProcessWithID(lldb::pid_t pid); + /// Called before attaching to a process. /// /// Allow Process plug-ins to execute some code before attaching a process. /// /// \return /// Returns an error object. - virtual Status WillAttachToProcessWithID(lldb::pid_t pid) { return Status(); } + virtual Status DoWillAttachToProcessWithID(lldb::pid_t pid) { + return Status(); + } + + /// Called before attaching to a process. + /// + /// \return + /// Returns an error object. + Status WillAttachToProcessWithName(const char *process_name, + bool wait_for_launch); /// Called before attaching to a process. /// @@ -896,8 +911,8 @@ class Process : public std::enable_shared_from_this<Process>, /// /// \return /// Returns an error object. - virtual Status WillAttachToProcessWithName(const char *process_name, - bool wait_for_launch) { + virtual Status DoWillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) { return Status(); } @@ -989,13 +1004,18 @@ class Process : public std::enable_shared_from_this<Process>, /// Called after reported vfork completion. virtual void DidVForkDone() {} + /// Called before launching to a process. + /// \return + /// Returns an error object. + Status WillLaunch(Module *module); + /// Called before launching to a process. /// /// Allow Process plug-ins to execute some code before launching a process. /// /// \return /// Returns an error object. - virtual Status WillLaunch(Module *module) { return Status(); } + virtual Status DoWillLaunch(Module *module) { return Status(); } /// Launch a new process. /// diff --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h index 89688ad52a7df..4ba936a81b646 100644 --- a/lldb/include/lldb/Target/Target.h +++ b/lldb/include/lldb/Target/Target.h @@ -780,6 +780,9 @@ class Target : public std::enable_shared_from_this<Target>, bool RemoveBreakpointByID(lldb::break_id_t break_id); + /// Resets the hit count of all breakpoints. + void ResetBreakpointHitCounts(); + // The flag 'end_to_end', default to true, signifies that the operation is // performed end to end, for both the debugger and the debuggee. diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index 578cf14283d92..aad01a2e6fb67 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -328,6 +328,11 @@ uint32_t Breakpoint::GetIgnoreCount() const { uint32_t Breakpoint::GetHitCount() const { return m_hit_counter.GetValue(); } +void Breakpoint::ResetHitCount() { + m_hit_counter.Reset(); + m_locations.ResetHitCount(); +} + bool Breakpoint::IsOneShot() const { return m_options.IsOneShot(); } void Breakpoint::SetOneShot(bool one_shot) { m_options.SetOneShot(one_shot); } diff --git a/lldb/source/Breakpoint/BreakpointList.cpp b/lldb/source/Breakpoint/BreakpointList.cpp index f4d4b2edbf08b..f7c2cdb938e62 100644 --- a/lldb/source/Breakpoint/BreakpointList.cpp +++ b/lldb/source/Breakpoint/BreakpointList.cpp @@ -186,6 +186,12 @@ void BreakpointList::ClearAllBreakpointSites() { bp_sp->ClearAllBreakpointSites(); } +void BreakpointList::ResetHitCounts() { + std::lock_guard<std::recursive_mutex> guard(m_mutex); + for (const auto &bp_sp : m_breakpoints) + bp_sp->ResetHitCount(); +} + void BreakpointList::GetListMutex( std::unique_lock<std::recursive_mutex> &lock) { lock = std::unique_lock<std::recursive_mutex>(m_mutex); diff --git a/lldb/source/Breakpoint/BreakpointLocationList.cpp b/lldb/source/Breakpoint/BreakpointLocationList.cpp index 8688f886d25f6..e0f1b9b2c8088 100644 --- a/lldb/source/Breakpoint/BreakpointLocationList.cpp +++ b/lldb/source/Breakpoint/BreakpointLocationList.cpp @@ -176,6 +176,12 @@ uint32_t BreakpointLocationList::GetHitCount() const { return hit_count; } +void BreakpointLocationList::ResetHitCount() { + std::lock_guard<std::recursive_mutex> guard(m_mutex); + for (auto &loc : m_locations) + loc->ResetHitCount(); +} + size_t BreakpointLocationList::GetNumResolvedLocations() const { std::lock_guard<std::recursive_mutex> guard(m_mutex); size_t resolve_count = 0; diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp index feb878f039ab6..6b9be1e55d4f2 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp @@ -168,21 +168,21 @@ ProcessKDP::~ProcessKDP() { Finalize(); } -Status ProcessKDP::WillLaunch(Module *module) { +Status ProcessKDP::DoWillLaunch(Module *module) { Status error; error.SetErrorString("launching not supported in kdp-remote plug-in"); return error; } -Status ProcessKDP::WillAttachToProcessWithID(lldb::pid_t pid) { +Status ProcessKDP::DoWillAttachToProcessWithID(lldb::pid_t pid) { Status error; error.SetErrorString( "attaching to a by process ID not supported in kdp-remote plug-in"); return error; } -Status ProcessKDP::WillAttachToProcessWithName(const char *process_name, - bool wait_for_launch) { +Status ProcessKDP::DoWillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) { Status error; error.SetErrorString( "attaching to a by process name not supported in kdp-remote plug-in"); diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h index a8cfb0ec9f870..3c12fd4074499 100644 --- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h +++ b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h @@ -56,17 +56,17 @@ class ProcessKDP : public lldb_private::Process { lldb_private::CommandObject *GetPluginCommandObject() override; // Creating a new process, or attaching to an existing one - lldb_private::Status WillLaunch(lldb_private::Module *module) override; + lldb_private::Status DoWillLaunch(lldb_private::Module *module) override; lldb_private::Status DoLaunch(lldb_private::Module *exe_module, lldb_private::ProcessLaunchInfo &launch_info) override; - lldb_private::Status WillAttachToProcessWithID(lldb::pid_t pid) override; + lldb_private::Status DoWillAttachToProcessWithID(lldb::pid_t pid) override; lldb_private::Status - WillAttachToProcessWithName(const char *process_name, - bool wait_for_launch) override; + DoWillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) override; lldb_private::Status DoConnectRemote(llvm::StringRef remote_url) override; diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 0c83f71ad09ab..5484ce5d30ab0 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -510,16 +510,16 @@ void ProcessGDBRemote::BuildDynamicRegisterInfo(bool force) { AddRemoteRegisters(registers, arch_to_use); } -Status ProcessGDBRemote::WillLaunch(lldb_private::Module *module) { +Status ProcessGDBRemote::DoWillLaunch(lldb_private::Module *module) { return WillLaunchOrAttach(); } -Status ProcessGDBRemote::WillAttachToProcessWithID(lldb::pid_t pid) { +Status ProcessGDBRemote::DoWillAttachToProcessWithID(lldb::pid_t pid) { return WillLaunchOrAttach(); } -Status ProcessGDBRemote::WillAttachToProcessWithName(const char *process_name, - bool wait_for_launch) { +Status ProcessGDBRemote::DoWillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) { return WillLaunchOrAttach(); } diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h index 8e2a462f61212..0f826ccab56f7 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -77,16 +77,16 @@ class ProcessGDBRemote : public Process, CommandObject *GetPluginCommandObject() override; // Creating a new process, or attaching to an existing one - Status WillLaunch(Module *module) override; + Status DoWillLaunch(Module *module) override; Status DoLaunch(Module *exe_module, ProcessLaunchInfo &launch_info) override; void DidLaunch() override; - Status WillAttachToProcessWithID(lldb::pid_t pid) override; + Status DoWillAttachToProcessWithID(lldb::pid_t pid) override; - Status WillAttachToProcessWithName(const char *process_name, - bool wait_for_launch) override; + Status DoWillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) override; Status DoConnectRemote(llvm::StringRef remote_url) override; diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 31e15d42a2e3f..1a9befdfcb0ce 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -2760,6 +2760,22 @@ ListenerSP ProcessAttachInfo::GetListenerForProcess(Debugger &debugger) { return debugger.GetListener(); } +Status Process::WillLaunch(Module *module) { + GetTarget().ResetBreakpointHitCounts(); + return DoWillLaunch(module); +} + +Status Process::WillAttachToProcessWithID(lldb::pid_t pid) { + GetTarget().ResetBreakpointHitCounts(); + return DoWillAttachToProcessWithID(pid); +} + +Status Process::WillAttachToProcessWithName(const char *process_name, + bool wait_for_launch) { + GetTarget().ResetBreakpointHitCounts(); + return DoWillAttachToProcessWithName(process_name, wait_for_launch); +} + Status Process::Attach(ProcessAttachInfo &attach_info) { m_abi_sp.reset(); m_process_input_reader.reset(); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index f28696ffd95ba..7ff2789880611 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -1003,6 +1003,10 @@ bool Target::EnableBreakpointByID(break_id_t break_id) { return false; } +void Target::ResetBreakpointHitCounts() { + GetBreakpointList().ResetHitCounts(); +} + Status Target::SerializeBreakpointsToFile(const FileSpec &file, const BreakpointIDList &bp_ids, bool append) { diff --git a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py index e2369be9ff94b..20934f680d5d8 100644 --- a/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py +++ b/lldb/test/API/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py @@ -82,5 +82,6 @@ def address_breakpoints(self): len(threads), 1, "There should be a thread stopped at our breakpoint") - # The hit count for the breakpoint should now be 2. - self.assertEquals(breakpoint.GetHitCount(), 2) + # The hit count for the breakpoint should be 1, since we reset counts + # for each run. + self.assertEquals(breakpoint.GetHitCount(), 1) diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py index 6212ba6c98646..af5441c10f79a 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py @@ -313,8 +313,9 @@ def breakpoint_command_sequence(self): substrs=['stopped', 'stop reason = breakpoint']) - # The breakpoint should have a hit count of 2. - lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 2) + # The breakpoint should have a hit count of 1, since we reset counts + # for each run. + lldbutil.check_breakpoint(self, bpno = 1, expected_hit_count = 1) def breakpoint_command_script_parameters(self): """Test that the frame and breakpoint location are being properly passed to the script breakpoint command function.""" diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile new file mode 100644 index 0000000000000..99998b20bcb05 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py new file mode 100644 index 0000000000000..0811c0773aeab --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/TestBreakpointResetUponRun.py @@ -0,0 +1,58 @@ +""" +Test breakpoint hit count is reset when target runs. +""" + +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * + + +class HitcountResetUponRun(TestBase): + BREAKPOINT_TEXT = 'Set a breakpoint here' + + def check_stopped_at_breakpoint_and_hit_once(self, thread, breakpoint): + frame0 = thread.GetFrameAtIndex(0) + location1 = breakpoint.FindLocationByAddress(frame0.GetPC()) + self.assertTrue(location1) + self.assertEqual(location1.GetHitCount(), 1) + self.assertEqual(breakpoint.GetHitCount(), 1) + + def test_hitcount_reset_upon_run(self): + self.build() + + exe = self.getBuildArtifact("a.out") + + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + breakpoint = target.BreakpointCreateBySourceRegex( + self.BREAKPOINT_TEXT, lldb.SBFileSpec('main.cpp')) + self.assertTrue( + breakpoint.IsValid() and breakpoint.GetNumLocations() == 1, + VALID_BREAKPOINT) + + process = target.LaunchSimple( + None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + from lldbsuite.test.lldbutil import get_stopped_thread + + # Verify 1st breakpoint location is hit. + thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue( + thread.IsValid(), + "There should be a thread stopped due to breakpoint") + self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint) + + # Relaunch + process.Kill() + process = target.LaunchSimple( + None, None, self.get_process_working_directory()) + self.assertTrue(process, PROCESS_IS_VALID) + + # Verify the hit counts are still one. + thread = get_stopped_thread(process, lldb.eStopReasonBreakpoint) + self.assertTrue( + thread.IsValid(), + "There should be a thread stopped due to breakpoint") + self.check_stopped_at_breakpoint_and_hit_once(thread, breakpoint) diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp new file mode 100644 index 0000000000000..05a741cb05ee5 --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_reset_upon_run/main.cpp @@ -0,0 +1,6 @@ +#include <stdio.h> + +int main(int argc, char const *argv[]) { + printf("Set a breakpoint here.\n"); + return 0; +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
