Skip to content

Commit 534818f

Browse files
3rditAdamR05
3rdit
and
AdamR05
committed
Fix breakpoint key mismatch possible bugs (forever) and added condition change events
Co-authored-by: AdamR05 <215697996+AdamR05@users.noreply.github.com>
1 parent 1ab8515 commit 534818f

8 files changed

Lines changed: 89 additions & 81 deletions

File tree

‎api/ffi.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ extern "C"
257257
RelativeBreakpointEnabledEvent,
258258
AbsoluteBreakpointDisabledEvent,
259259
RelativeBreakpointDisabledEvent,
260+
AbsoluteBreakpointConditionChangedEvent,
261+
RelativeBreakpointConditionChangedEvent,
260262

261263
ActiveThreadChangedEvent,
262264

‎core/debuggercontroller.cpp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,13 +143,29 @@ void DebuggerController::DisableBreakpoint(const ModuleNameAndOffset& address)
143143

144144
bool DebuggerController::SetBreakpointCondition(uint64_t address, const std::string& condition)
145145
{
146-
return m_state->GetBreakpoints()->SetConditionAbsolute(address, condition);
146+
bool result = m_state->GetBreakpoints()->SetConditionAbsolute(address, condition);
147+
if (result)
148+
{
149+
DebuggerEvent event;
150+
event.type = AbsoluteBreakpointConditionChangedEvent;
151+
event.data.absoluteAddress = address;
152+
PostDebuggerEvent(event);
153+
}
154+
return result;
147155
}
148156

149157

150158
bool DebuggerController::SetBreakpointCondition(const ModuleNameAndOffset& address, const std::string& condition)
151159
{
152-
return m_state->GetBreakpoints()->SetConditionOffset(address, condition);
160+
bool result = m_state->GetBreakpoints()->SetConditionOffset(address, condition);
161+
if (result)
162+
{
163+
DebuggerEvent event;
164+
event.type = RelativeBreakpointConditionChangedEvent;
165+
event.data.relativeAddress = address;
166+
PostDebuggerEvent(event);
167+
}
168+
return result;
153169
}
154170

155171

‎core/debuggerstate.cpp‎

Lines changed: 59 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -570,43 +570,40 @@ bool DebuggerBreakpoints::RemoveAbsolute(uint64_t remoteAddress)
570570
return false;
571571

572572
ModuleNameAndOffset info = m_state->GetModules()->AbsoluteAddressToRelative(remoteAddress);
573-
if (ContainsOffset(info))
574-
{
575-
auto iter = std::find(m_breakpoints.begin(), m_breakpoints.end(), info);
576-
if (iter != m_breakpoints.end())
577-
{
578-
m_breakpoints.erase(iter);
579-
}
580-
m_enabledState.erase(info); // Remove enabled state
581-
m_conditions.erase(info); // Remove condition
582-
SerializeMetadata();
583-
m_state->GetAdapter()->RemoveBreakpoint(remoteAddress);
584-
return true;
585-
}
586-
return false;
573+
auto actualKey = FindBreakpointKey(info);
574+
if (!actualKey)
575+
return false;
576+
577+
if (auto iter = std::ranges::find(m_breakpoints, *actualKey); iter != m_breakpoints.end())
578+
m_breakpoints.erase(iter);
579+
580+
m_enabledState.erase(*actualKey);
581+
m_conditions.erase(*actualKey);
582+
SerializeMetadata();
583+
m_state->GetAdapter()->RemoveBreakpoint(remoteAddress);
584+
return true;
587585
}
588586

589587

590588
bool DebuggerBreakpoints::RemoveOffset(const ModuleNameAndOffset& address)
591589
{
592-
if (ContainsOffset(address))
593-
{
594-
if (auto iter = std::find(m_breakpoints.begin(), m_breakpoints.end(), address); iter != m_breakpoints.end())
595-
m_breakpoints.erase(iter);
590+
auto actualKey = FindBreakpointKey(address);
591+
if (!actualKey)
592+
return false;
596593

597-
m_enabledState.erase(address); // Remove enabled state
598-
m_conditions.erase(address); // Remove condition
599-
SerializeMetadata();
594+
if (const auto iter = std::ranges::find(m_breakpoints, *actualKey); iter != m_breakpoints.end())
595+
m_breakpoints.erase(iter);
600596

601-
if (m_state->GetAdapter() && m_state->IsConnected())
602-
{
603-
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(address);
604-
m_state->GetAdapter()->RemoveBreakpoint(remoteAddress);
605-
return true;
606-
}
607-
return true;
597+
m_enabledState.erase(*actualKey);
598+
m_conditions.erase(*actualKey);
599+
SerializeMetadata();
600+
601+
if (m_state->GetAdapter() && m_state->IsConnected())
602+
{
603+
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(*actualKey);
604+
m_state->GetAdapter()->RemoveBreakpoint(remoteAddress);
608605
}
609-
return false;
606+
return true;
610607
}
611608

612609

@@ -619,18 +616,17 @@ bool DebuggerBreakpoints::EnableAbsolute(uint64_t remoteAddress)
619616

620617
bool DebuggerBreakpoints::EnableOffset(const ModuleNameAndOffset& address)
621618
{
622-
if (!ContainsOffset(address))
619+
auto actualKey = FindBreakpointKey(address);
620+
if (!actualKey)
623621
return false;
624622

625-
m_enabledState[address] = true;
623+
m_enabledState[*actualKey] = true;
626624
SerializeMetadata();
627625

628-
// If connected, make sure the breakpoint is active in the target
629626
if (m_state->GetAdapter() && m_state->IsConnected())
630627
{
631-
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(address);
628+
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(*actualKey);
632629
m_state->GetAdapter()->AddBreakpoint(remoteAddress);
633-
return true;
634630
}
635631
return true;
636632
}
@@ -645,18 +641,17 @@ bool DebuggerBreakpoints::DisableAbsolute(uint64_t remoteAddress)
645641

646642
bool DebuggerBreakpoints::DisableOffset(const ModuleNameAndOffset& address)
647643
{
648-
if (!ContainsOffset(address))
644+
auto actualKey = FindBreakpointKey(address);
645+
if (!actualKey)
649646
return false;
650647

651-
m_enabledState[address] = false;
648+
m_enabledState[*actualKey] = false;
652649
SerializeMetadata();
653650

654-
// If connected, remove the breakpoint from the target but keep it in our list
655651
if (m_state->GetAdapter() && m_state->IsConnected())
656652
{
657-
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(address);
653+
uint64_t remoteAddress = m_state->GetModules()->RelativeAddressToAbsolute(*actualKey);
658654
m_state->GetAdapter()->RemoveBreakpoint(remoteAddress);
659-
return true;
660655
}
661656
return true;
662657
}
@@ -671,12 +666,15 @@ bool DebuggerBreakpoints::IsEnabledAbsolute(uint64_t address)
671666

672667
bool DebuggerBreakpoints::IsEnabledOffset(const ModuleNameAndOffset& address)
673668
{
674-
auto iter = m_enabledState.find(address);
669+
auto actualKey = FindBreakpointKey(address);
670+
if (!actualKey)
671+
return true; // Default to enabled if breakpoint not found
672+
673+
auto iter = m_enabledState.find(*actualKey);
675674
if (iter != m_enabledState.end())
676675
return iter->second;
677-
678-
// Default to enabled if not explicitly set
679-
return true;
676+
677+
return true; // Default to enabled if not explicitly set
680678
}
681679

682680

@@ -721,36 +719,37 @@ bool DebuggerBreakpoints::SetConditionAbsolute(const uint64_t remoteAddress, con
721719

722720
bool DebuggerBreakpoints::SetConditionOffset(const ModuleNameAndOffset& address, const std::string& condition)
723721
{
724-
std::optional<ModuleNameAndOffset> actualKey;
722+
auto actualKey = FindBreakpointKey(address);
723+
if (!actualKey)
724+
return false;
725+
726+
if (condition.empty())
727+
m_conditions.erase(*actualKey);
728+
else
729+
m_conditions[*actualKey] = condition;
730+
731+
SerializeMetadata();
732+
return true;
733+
}
725734

735+
736+
std::optional<ModuleNameAndOffset> DebuggerBreakpoints::FindBreakpointKey(const ModuleNameAndOffset& address)
737+
{
726738
if (m_state->GetAdapter())
727739
{
728740
const uint64_t targetAbsolute = m_state->GetModules()->RelativeAddressToAbsolute(address);
729741
for (const ModuleNameAndOffset& bp : m_breakpoints)
730742
{
731743
if (m_state->GetModules()->RelativeAddressToAbsolute(bp) == targetAbsolute)
732-
{
733-
actualKey = bp;
734-
break;
735-
}
744+
return bp;
736745
}
737746
}
738747
else
739748
{
740749
if (const auto it = std::ranges::find(m_breakpoints, address); it != m_breakpoints.end())
741-
actualKey = *it;
750+
return *it;
742751
}
743-
744-
if (!actualKey)
745-
return false;
746-
747-
if (condition.empty())
748-
m_conditions.erase(*actualKey);
749-
else
750-
m_conditions[*actualKey] = condition;
751-
752-
SerializeMetadata();
753-
return true;
752+
return std::nullopt;
754753
}
755754

756755

‎core/debuggerstate.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ namespace BinaryNinjaDebugger {
113113
bool HasConditionOffset(const ModuleNameAndOffset& address);
114114

115115
private:
116+
// Helper to find the actual key in m_breakpoints matching the given address,
117+
// handling module name differences via absolute address comparison
118+
std::optional<ModuleNameAndOffset> FindBreakpointKey(const ModuleNameAndOffset& address);
119+
116120
// Helper to find the actual key in m_conditions matching the given address,
117121
// handling module name differences and file address fallback
118122
std::optional<ModuleNameAndOffset> FindConditionKey(const ModuleNameAndOffset& address);

‎core/ffi.cpp‎

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -933,29 +933,13 @@ bool BNDebuggerContainsRelativeBreakpoint(BNDebuggerController* controller, cons
933933

934934
bool BNDebuggerSetBreakpointConditionAbsolute(BNDebuggerController* controller, uint64_t address, const char* condition)
935935
{
936-
const DebuggerState* state = controller->object->GetState();
937-
if (!state)
938-
return false;
939-
940-
DebuggerBreakpoints* breakpoints = state->GetBreakpoints();
941-
if (!breakpoints)
942-
return false;
943-
944-
return breakpoints->SetConditionAbsolute(address, condition ? condition : "");
936+
return controller->object->SetBreakpointCondition(address, condition ? condition : "");
945937
}
946938

947939

948940
bool BNDebuggerSetBreakpointConditionRelative(BNDebuggerController* controller, const char* module, uint64_t offset, const char* condition)
949941
{
950-
const DebuggerState* state = controller->object->GetState();
951-
if (!state)
952-
return false;
953-
954-
DebuggerBreakpoints* breakpoints = state->GetBreakpoints();
955-
if (!breakpoints)
956-
return false;
957-
958-
return breakpoints->SetConditionOffset(ModuleNameAndOffset(module, offset), condition ? condition : "");
942+
return controller->object->SetBreakpointCondition(ModuleNameAndOffset(module, offset), condition ? condition : "");
959943
}
960944

961945

‎ui/breakpointswidget.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ class DebugBreakpointsWidget : public QTableView
144144
DebugBreakpointsWidget(ViewFrame* view, BinaryViewRef data, Menu* menu);
145145
~DebugBreakpointsWidget();
146146

147-
void uiEventHandler(const DebuggerEvent& event);
148147
void updateFonts();
149148

150149
private slots:

‎ui/debuggerwidget.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ void DebuggerWidget::uiEventHandler(const DebuggerEvent& event)
135135
case RelativeBreakpointEnabledEvent:
136136
case AbsoluteBreakpointDisabledEvent:
137137
case RelativeBreakpointDisabledEvent:
138+
case AbsoluteBreakpointConditionChangedEvent:
139+
case RelativeBreakpointConditionChangedEvent:
138140
m_breakpointsWidget->updateContent();
139141
break;
140142
default:

‎ui/ui.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1788,6 +1788,8 @@ void DebuggerUI::updateUI(const DebuggerEvent& event)
17881788
case AbsoluteBreakpointEnabledEvent:
17891789
case RelativeBreakpointDisabledEvent:
17901790
case AbsoluteBreakpointDisabledEvent:
1791+
case AbsoluteBreakpointConditionChangedEvent:
1792+
case RelativeBreakpointConditionChangedEvent:
17911793
{
17921794
m_context->refreshCurrentViewContents();
17931795
break;

0 commit comments

Comments
 (0)