Skip to content

Commit 0de6fe1

Browse files
3rditAdamR05
andcommitted
Simplified conditional breakpoint implementation and removed redundant code
Co-authored-by: AdamR05 <215697996+AdamR05@users.noreply.github.com>
1 parent 534818f commit 0de6fe1

6 files changed

Lines changed: 24 additions & 73 deletions

File tree

core/debuggercontroller.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ DebuggerController::DebuggerController(BinaryViewRef data): BinaryDataNotificati
3232
m_data = data;
3333
m_data->RegisterNotification(this);
3434
m_viewStart = m_data->GetStart();
35-
m_originalFileBase = m_data->GetStart();
3635

3736
m_state = new DebuggerState(data, this);
3837
m_adapter = nullptr;

core/debuggercontroller.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@ namespace BinaryNinjaDebugger {
8585
// this does not change even if we add the debugger memory region. In the future, this should be provided by
8686
// the binary view -- we will no longer need to track it ourselves
8787
uint64_t m_viewStart;
88-
// Preserves original file base before rebasing - never updated, used for file VA to offset conversion
89-
uint64_t m_originalFileBase;
9088

9189
// inline static std::vector<DbgRef<DebuggerController>> g_debuggerControllers;
9290
static DbgRef<DebuggerController>* g_debuggerControllers;
@@ -395,7 +393,6 @@ namespace BinaryNinjaDebugger {
395393
bool ReAddDebuggerMemoryRegion();
396394

397395
uint64_t GetViewFileSegmentsStart() { return m_viewStart; }
398-
uint64_t GetOriginalFileBase() { return m_originalFileBase; }
399396

400397
bool ComputeExprValueAPI(const LowLevelILInstruction& instr, intx::uint512& value);
401398
bool ComputeExprValue(const LowLevelILInstruction& instr, intx::uint512& value);

core/debuggerstate.cpp

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -753,67 +753,47 @@ std::optional<ModuleNameAndOffset> DebuggerBreakpoints::FindBreakpointKey(const
753753
}
754754

755755

756-
std::optional<ModuleNameAndOffset> DebuggerBreakpoints::FindConditionKey(const ModuleNameAndOffset& address)
756+
std::string DebuggerBreakpoints::GetConditionAbsolute(const uint64_t address)
757757
{
758-
if (m_conditions.contains(address))
759-
return address;
760-
761-
// absolute address comparison (handles module name differences)
762-
// assumes conditions are stored with valid, resolvable module names
763-
const uint64_t targetAbsolute = m_state->GetModules()->RelativeAddressToAbsolute(address);
764-
for (const auto& [key, _] : m_conditions)
765-
{
766-
if (const uint64_t conditionAbsolute = m_state->GetModules()->RelativeAddressToAbsolute(key);
767-
conditionAbsolute == targetAbsolute)
768-
return key;
769-
}
770-
771-
// if the address has empty module name, it might be a file virtual address?
772-
// try converting to an offset by subtracting the original file base.
773-
if (address.module.empty())
758+
for (const auto& [key, condition] : m_conditions)
774759
{
775-
if (const uint64_t originalBase = m_state->GetController()->GetOriginalFileBase();
776-
address.offset >= originalBase)
777-
{
778-
const uint64_t fileOffset = address.offset - originalBase;
779-
const std::string& mainFile = m_state->GetController()->GetData()->GetFile()->GetOriginalFilename();
780-
for (const auto& [key, _] : m_conditions)
781-
{
782-
if (key.offset == fileOffset && DebugModule::IsSameBaseModule(mainFile, key.module))
783-
return key;
784-
}
785-
}
760+
if (m_state->GetModules()->RelativeAddressToAbsolute(key) == address)
761+
return condition;
786762
}
787-
788-
return std::nullopt;
789-
}
790-
791-
792-
std::string DebuggerBreakpoints::GetConditionAbsolute(const uint64_t address)
793-
{
794-
const ModuleNameAndOffset info = m_state->GetModules()->AbsoluteAddressToRelative(address);
795-
return GetConditionOffset(info);
763+
return "";
796764
}
797765

798766

799767
std::string DebuggerBreakpoints::GetConditionOffset(const ModuleNameAndOffset& address)
800768
{
801-
if (const auto key = FindConditionKey(address))
802-
return m_conditions[*key];
769+
auto actualKey = FindBreakpointKey(address);
770+
if (!actualKey)
771+
return "";
772+
773+
if (const auto it = m_conditions.find(*actualKey); it != m_conditions.end())
774+
return it->second;
803775
return "";
804776
}
805777

806778

807779
bool DebuggerBreakpoints::HasConditionAbsolute(const uint64_t address)
808780
{
809-
const ModuleNameAndOffset info = m_state->GetModules()->AbsoluteAddressToRelative(address);
810-
return HasConditionOffset(info);
781+
for (const auto& [key, _] : m_conditions)
782+
{
783+
if (m_state->GetModules()->RelativeAddressToAbsolute(key) == address)
784+
return true;
785+
}
786+
return false;
811787
}
812788

813789

814790
bool DebuggerBreakpoints::HasConditionOffset(const ModuleNameAndOffset& address)
815791
{
816-
return FindConditionKey(address).has_value();
792+
auto actualKey = FindBreakpointKey(address);
793+
if (!actualKey)
794+
return false;
795+
796+
return m_conditions.contains(*actualKey);
817797
}
818798

819799

core/debuggerstate.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,6 @@ namespace BinaryNinjaDebugger {
116116
// Helper to find the actual key in m_breakpoints matching the given address,
117117
// handling module name differences via absolute address comparison
118118
std::optional<ModuleNameAndOffset> FindBreakpointKey(const ModuleNameAndOffset& address);
119-
120-
// Helper to find the actual key in m_conditions matching the given address,
121-
// handling module name differences and file address fallback
122-
std::optional<ModuleNameAndOffset> FindConditionKey(const ModuleNameAndOffset& address);
123119
};
124120

125121

core/ffi.cpp

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -945,31 +945,13 @@ bool BNDebuggerSetBreakpointConditionRelative(BNDebuggerController* controller,
945945

946946
char* BNDebuggerGetBreakpointConditionAbsolute(BNDebuggerController* controller, uint64_t address)
947947
{
948-
const DebuggerState* state = controller->object->GetState();
949-
if (!state)
950-
return BNDebuggerAllocString("");
951-
952-
DebuggerBreakpoints* breakpoints = state->GetBreakpoints();
953-
if (!breakpoints)
954-
return BNDebuggerAllocString("");
955-
956-
const std::string condition = breakpoints->GetConditionAbsolute(address);
957-
return BNDebuggerAllocString(condition.c_str());
948+
return BNDebuggerAllocString(controller->object->GetBreakpointCondition(address).c_str());
958949
}
959950

960951

961952
char* BNDebuggerGetBreakpointConditionRelative(BNDebuggerController* controller, const char* module, uint64_t offset)
962953
{
963-
const DebuggerState* state = controller->object->GetState();
964-
if (!state)
965-
return BNDebuggerAllocString("");
966-
967-
DebuggerBreakpoints* breakpoints = state->GetBreakpoints();
968-
if (!breakpoints)
969-
return BNDebuggerAllocString("");
970-
971-
const std::string condition = breakpoints->GetConditionOffset(ModuleNameAndOffset(module, offset));
972-
return BNDebuggerAllocString(condition.c_str());
954+
return BNDebuggerAllocString(controller->object->GetBreakpointCondition(ModuleNameAndOffset(module, offset)).c_str());
973955
}
974956

975957

ui/breakpointswidget.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -577,10 +577,7 @@ void DebugBreakpointsWidget::editCondition()
577577
QLineEdit::Normal, QString::fromStdString(currentCondition), &ok);
578578

579579
if (ok)
580-
{
581580
m_controller->SetBreakpointCondition(bp.location(), newCondition.trimmed().toStdString());
582-
updateContent();
583-
}
584581
}
585582

586583

0 commit comments

Comments
 (0)