Author: Jonas Devlieghere Date: 2025-07-03T11:17:19-07:00 New Revision: 378f248934d603e3fba9d958d2db997814d057d5
URL: https://github.com/llvm/llvm-project/commit/378f248934d603e3fba9d958d2db997814d057d5 DIFF: https://github.com/llvm/llvm-project/commit/378f248934d603e3fba9d958d2db997814d057d5.diff LOG: [lldb] Add SB API to make a breakpoint a hardware breakpoint (#146602) This adds SBBreakpoint::SetIsHardware, allowing clients to mark an existing breakpoint as a hardware breakpoint purely through the API. This is safe to do after creation, as the hardware/software distinction doesn't affect how breakpoint locations are selected. In some cases (e.g. when writing a trap instruction would alter program behavior), it's important to use hardware breakpoints. Ideally, we’d extend the various `Create` methods to support this, but given their number, this patch limits the scope to the post-creation API. As a workaround, users can also rely on target.require-hardware-breakpoint or use the `breakpoint set` command. rdar://153528045 Added: lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/Makefile lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/TestSimpleHWBreakpoints.py lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/main.c Modified: lldb/include/lldb/API/SBBreakpoint.h lldb/include/lldb/Breakpoint/Breakpoint.h lldb/include/lldb/Breakpoint/BreakpointLocation.h lldb/source/API/SBBreakpoint.cpp lldb/source/Breakpoint/Breakpoint.cpp lldb/source/Breakpoint/BreakpointLocation.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/API/SBBreakpoint.h b/lldb/include/lldb/API/SBBreakpoint.h index e08df3b6d5ab0..18ed3e7226d3b 100644 --- a/lldb/include/lldb/API/SBBreakpoint.h +++ b/lldb/include/lldb/API/SBBreakpoint.h @@ -148,6 +148,11 @@ class LLDB_API SBBreakpoint { bool IsHardware() const; + /// Make this breakpoint a hardware breakpoint. This will replace all existing + /// breakpoint locations with hardware breakpoints. Returns an error if this + /// fails, e.g. when there aren't enough hardware resources available. + lldb::SBError SetIsHardware(bool is_hardware); + // Can only be called from a ScriptedBreakpointResolver... SBError AddLocation(SBAddress &address); diff --git a/lldb/include/lldb/Breakpoint/Breakpoint.h b/lldb/include/lldb/Breakpoint/Breakpoint.h index f623a2e0c295b..b200a1e4893df 100644 --- a/lldb/include/lldb/Breakpoint/Breakpoint.h +++ b/lldb/include/lldb/Breakpoint/Breakpoint.h @@ -519,6 +519,8 @@ class Breakpoint : public std::enable_shared_from_this<Breakpoint>, bool IsHardware() const { return m_hardware; } + llvm::Error SetIsHardware(bool is_hardware); + lldb::BreakpointResolverSP GetResolver() { return m_resolver_sp; } lldb::SearchFilterSP GetSearchFilter() { return m_filter_sp; } diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h index 3592291bb2d06..f4fe2e9a9abfc 100644 --- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h +++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h @@ -69,7 +69,7 @@ class BreakpointLocation // The next section deals with various breakpoint options. /// If \a enabled is \b true, enable the breakpoint, if \b false disable it. - void SetEnabled(bool enabled); + bool SetEnabled(bool enabled); /// Check the Enable/Disable state. /// diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp index 87fadbcec4f26..397afc1f10f94 100644 --- a/lldb/source/API/SBBreakpoint.cpp +++ b/lldb/source/API/SBBreakpoint.cpp @@ -781,6 +781,18 @@ bool SBBreakpoint::IsHardware() const { return false; } +lldb::SBError SBBreakpoint::SetIsHardware(bool is_hardware) { + LLDB_INSTRUMENT_VA(this, is_hardware); + + BreakpointSP bkpt_sp = GetSP(); + if (bkpt_sp) { + std::lock_guard<std::recursive_mutex> guard( + bkpt_sp->GetTarget().GetAPIMutex()); + return SBError(Status::FromError(bkpt_sp->SetIsHardware(is_hardware))); + } + return SBError(); +} + BreakpointSP SBBreakpoint::GetSP() const { return m_opaque_wp.lock(); } // This is simple collection of breakpoint id's and their target. diff --git a/lldb/source/Breakpoint/Breakpoint.cpp b/lldb/source/Breakpoint/Breakpoint.cpp index b308644825dad..8fc93cc8e0e51 100644 --- a/lldb/source/Breakpoint/Breakpoint.cpp +++ b/lldb/source/Breakpoint/Breakpoint.cpp @@ -61,7 +61,7 @@ Breakpoint::Breakpoint(Target &new_target, const Breakpoint &source_bp) Breakpoint::~Breakpoint() = default; BreakpointSP Breakpoint::CopyFromBreakpoint(TargetSP new_target, - const Breakpoint& bp_to_copy_from) { + const Breakpoint &bp_to_copy_from) { if (!new_target) return BreakpointSP(); @@ -163,7 +163,7 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData( std::make_shared<SearchFilterForUnconstrainedSearches>(target_sp); else { filter_sp = SearchFilter::CreateFromStructuredData(target_sp, *filter_dict, - create_error); + create_error); if (create_error.Fail()) { error = Status::FromErrorStringWithFormat( "Error creating breakpoint filter from data: %s.", @@ -174,7 +174,7 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData( std::unique_ptr<BreakpointOptions> options_up; StructuredData::Dictionary *options_dict; - Target& target = *target_sp; + Target &target = *target_sp; success = breakpoint_dict->GetValueForKeyAsDictionary( BreakpointOptions::GetSerializationKey(), options_dict); if (success) { @@ -192,8 +192,8 @@ lldb::BreakpointSP Breakpoint::CreateFromStructuredData( success = breakpoint_dict->GetValueForKeyAsBoolean( Breakpoint::GetKey(OptionNames::Hardware), hardware); - result_sp = target.CreateBreakpoint(filter_sp, resolver_sp, false, - hardware, true); + result_sp = + target.CreateBreakpoint(filter_sp, resolver_sp, false, hardware, true); if (result_sp && options_up) { result_sp->m_options = *options_up; @@ -251,6 +251,45 @@ const lldb::TargetSP Breakpoint::GetTargetSP() { bool Breakpoint::IsInternal() const { return LLDB_BREAK_ID_IS_INTERNAL(m_bid); } +llvm::Error Breakpoint::SetIsHardware(bool is_hardware) { + if (is_hardware == m_hardware) + return llvm::Error::success(); + + // Disable all non-hardware breakpoint locations. + std::vector<BreakpointLocationSP> locations; + for (BreakpointLocationSP location_sp : m_locations.BreakpointLocations()) { + if (!location_sp || !location_sp->IsEnabled()) + continue; + + lldb::BreakpointSiteSP breakpoint_site_sp = + location_sp->GetBreakpointSite(); + if (!breakpoint_site_sp || + breakpoint_site_sp->GetType() == BreakpointSite::eHardware) + continue; + + locations.push_back(location_sp); + location_sp->SetEnabled(false); + } + + // Toggle the hardware mode. + m_hardware = is_hardware; + + // Re-enable all breakpoint locations. + size_t num_failures = 0; + for (BreakpointLocationSP location_sp : locations) { + if (!location_sp->SetEnabled(true)) + num_failures++; + } + + if (num_failures != 0) + return llvm::createStringError( + "%ull out of %ull breakpoint locations left disabled because they " + "couldn't be converted to hardware", + num_failures, locations.size()); + + return llvm::Error::success(); +} + BreakpointLocationSP Breakpoint::AddLocation(const Address &addr, bool *new_location) { return m_locations.AddLocation(addr, m_resolve_indirect_symbols, @@ -952,8 +991,7 @@ void Breakpoint::GetResolverDescription(Stream *s) { m_resolver_sp->GetDescription(s); } -bool Breakpoint::GetMatchingFileLine(ConstString filename, - uint32_t line_number, +bool Breakpoint::GetMatchingFileLine(ConstString filename, uint32_t line_number, BreakpointLocationCollection &loc_coll) { // TODO: To be correct, this method needs to fill the breakpoint location // collection @@ -1010,19 +1048,32 @@ void Breakpoint::SendBreakpointChangedEvent( const char *Breakpoint::BreakpointEventTypeAsCString(BreakpointEventType type) { switch (type) { - case eBreakpointEventTypeInvalidType: return "invalid"; - case eBreakpointEventTypeAdded: return "breakpoint added"; - case eBreakpointEventTypeRemoved: return "breakpoint removed"; - case eBreakpointEventTypeLocationsAdded: return "locations added"; - case eBreakpointEventTypeLocationsRemoved: return "locations removed"; - case eBreakpointEventTypeLocationsResolved: return "locations resolved"; - case eBreakpointEventTypeEnabled: return "breakpoint enabled"; - case eBreakpointEventTypeDisabled: return "breakpoint disabled"; - case eBreakpointEventTypeCommandChanged: return "command changed"; - case eBreakpointEventTypeConditionChanged: return "condition changed"; - case eBreakpointEventTypeIgnoreChanged: return "ignore count changed"; - case eBreakpointEventTypeThreadChanged: return "thread changed"; - case eBreakpointEventTypeAutoContinueChanged: return "autocontinue changed"; + case eBreakpointEventTypeInvalidType: + return "invalid"; + case eBreakpointEventTypeAdded: + return "breakpoint added"; + case eBreakpointEventTypeRemoved: + return "breakpoint removed"; + case eBreakpointEventTypeLocationsAdded: + return "locations added"; + case eBreakpointEventTypeLocationsRemoved: + return "locations removed"; + case eBreakpointEventTypeLocationsResolved: + return "locations resolved"; + case eBreakpointEventTypeEnabled: + return "breakpoint enabled"; + case eBreakpointEventTypeDisabled: + return "breakpoint disabled"; + case eBreakpointEventTypeCommandChanged: + return "command changed"; + case eBreakpointEventTypeConditionChanged: + return "condition changed"; + case eBreakpointEventTypeIgnoreChanged: + return "ignore count changed"; + case eBreakpointEventTypeThreadChanged: + return "thread changed"; + case eBreakpointEventTypeAutoContinueChanged: + return "autocontinue changed"; }; llvm_unreachable("Fully covered switch above!"); } @@ -1060,7 +1111,7 @@ void Breakpoint::BreakpointEventData::Dump(Stream *s) const { BreakpointEventType event_type = GetBreakpointEventType(); break_id_t bkpt_id = GetBreakpoint()->GetID(); s->Format("bkpt: {0} type: {1}", bkpt_id, - BreakpointEventTypeAsCString(event_type)); + BreakpointEventTypeAsCString(event_type)); } const Breakpoint::BreakpointEventData * diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp index 2a00c94eac7e7..1d11f3a54b67f 100644 --- a/lldb/source/Breakpoint/BreakpointLocation.cpp +++ b/lldb/source/Breakpoint/BreakpointLocation.cpp @@ -72,15 +72,13 @@ bool BreakpointLocation::IsEnabled() const { return true; } -void BreakpointLocation::SetEnabled(bool enabled) { +bool BreakpointLocation::SetEnabled(bool enabled) { GetLocationOptions().SetEnabled(enabled); - if (enabled) { - ResolveBreakpointSite(); - } else { - ClearBreakpointSite(); - } + const bool success = + enabled ? ResolveBreakpointSite() : ClearBreakpointSite(); SendBreakpointLocationChangedEvent(enabled ? eBreakpointEventTypeEnabled : eBreakpointEventTypeDisabled); + return success; } bool BreakpointLocation::IsAutoContinue() const { @@ -436,10 +434,10 @@ bool BreakpointLocation::ResolveBreakpointSite() { process->CreateBreakpointSite(shared_from_this(), m_owner.IsHardware()); if (new_id == LLDB_INVALID_BREAK_ID) { - Log *log = GetLog(LLDBLog::Breakpoints); - if (log) - log->Warning("Failed to add breakpoint site at 0x%" PRIx64, - m_address.GetOpcodeLoadAddress(&m_owner.GetTarget())); + LLDB_LOGF(GetLog(LLDBLog::Breakpoints), + "Failed to add breakpoint site at 0x%" PRIx64 "(resolved=%s)", + m_address.GetOpcodeLoadAddress(&m_owner.GetTarget()), + IsResolved() ? "yes" : "no"); } return IsResolved(); diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/Makefile b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/Makefile new file mode 100644 index 0000000000000..304633c2dca1f --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/Makefile @@ -0,0 +1,7 @@ +C_SOURCES := main.c + +ifeq ($(CC_TYPE), icc) + CFLAGS_EXTRAS := -debug inline-debug-info +endif + +include Makefile.rules diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/TestSimpleHWBreakpoints.py b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/TestSimpleHWBreakpoints.py new file mode 100644 index 0000000000000..ccbb23507c1be --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/TestSimpleHWBreakpoints.py @@ -0,0 +1,50 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + +from functionalities.breakpoint.hardware_breakpoints.base import * + + +class SimpleHWBreakpointTest(HardwareBreakpointTestBase): + def does_not_support_hw_breakpoints(self): + # FIXME: Use HardwareBreakpointTestBase.supports_hw_breakpoints + if super().supports_hw_breakpoints() is None: + return "Hardware breakpoints are unsupported" + return None + + @skipTestIfFn(does_not_support_hw_breakpoints) + def test(self): + """Test SBBreakpoint::SetIsHardware""" + self.build() + + # Set a breakpoint on main. + target, process, _, main_bp = lldbutil.run_to_source_breakpoint( + self, "main", lldb.SBFileSpec("main.c") + ) + + break_on_me_bp = target.BreakpointCreateByLocation("main.c", 1) + + self.assertFalse(main_bp.IsHardware()) + self.assertFalse(break_on_me_bp.IsHardware()) + self.assertGreater(break_on_me_bp.GetNumResolvedLocations(), 0) + + error = break_on_me_bp.SetIsHardware(True) + + # Regardless of whether we succeeded in updating all the locations, the + # breakpoint will be marked as a hardware breakpoint. + self.assertTrue(break_on_me_bp.IsHardware()) + + if super().supports_hw_breakpoints(): + self.assertSuccess(error) + + # Continue to our Hardware breakpoint and verify that's the reason + # we're stopped. + process.Continue() + self.expect( + "thread list", + STOPPED_DUE_TO_BREAKPOINT, + substrs=["stopped", "stop reason = breakpoint"], + ) + else: + self.assertFailure(error) diff --git a/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/main.c b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/main.c new file mode 100644 index 0000000000000..4f1c9df56200a --- /dev/null +++ b/lldb/test/API/functionalities/breakpoint/hardware_breakpoints/simple_hw_breakpoints/main.c @@ -0,0 +1,7 @@ +int break_on_me() { + int i = 10; + i++; + return i; +} + +int main() { return break_on_me(); } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
