Navigate double-clicked sidebar addresses by content type

ui/breakpointswidget.cpp

@@ -29,6 +29,7 @@ limitations under the License.
 #include <QMessageBox>
 #include <QStyleOptionButton>
 #include "breakpointswidget.h"
+#include "debuggeruicommon.h"
 #include "base/assertions.h"
 #include "hardwarebreakpointdialog.h"
 #include "ui.h"
@@ -453,7 +454,16 @@ DebugBreakpointsWidget::~DebugBreakpointsWidget() {}
 
 void DebugBreakpointsWidget::onDoubleClicked()
 {
-	jump();
+	// Navigate to the breakpoint, opening it in the other pane when it is a different kind
+	// of thing (code vs data) than the current pane shows (see NavigateToAddress, issue
+	// #1134). The "Jump To Breakpoint" context menu action always uses the current pane.
+	QModelIndexList sel = selectionModel()->selectedRows();
+	if (sel.empty())
+		return;
+
+	BreakpointItem bp = m_model->getRow(sel[0].row());
+	if (m_controller->GetData())
+		NavigateToAddress(this, m_controller->GetData(), bp.address());
 }
 
 

ui/debuggerinfowidget.cpp

@@ -17,10 +17,13 @@ limitations under the License.
 #include <QPainter>
 #include <QHeaderView>
 #include <QGuiApplication>
+#include <QMenu>
+#include <QContextMenuEvent>
 #include <QMimeData>
 #include <QClipboard>
 #include "ui.h"
 #include "debuggerinfowidget.h"
+#include "debuggeruicommon.h"
 #include "lowlevelilinstruction.h"
 #include "mediumlevelilinstruction.h"
 #include "highlevelilinstruction.h"
@@ -935,25 +938,56 @@ void DebuggerInfoTable::updateFonts()
 }
 
 
-void DebuggerInfoTable::onDoubleClicked()
+bool DebuggerInfoTable::selectedValue(uint64_t& value)
 {
 	QModelIndexList sel = selectionModel()->selectedIndexes();
 	if (sel.empty())
-		return;
+		return false;
 
-	auto info = m_model->getRow(sel[0].row());
-	uint64_t value = (uint64_t)info.value;
+	value = (uint64_t)m_model->getRow(sel[0].row()).value;
+	return true;
+}
 
-	UIContext* context = UIContext::contextForWidget(this);
-	if (!context)
-		return;
 
-	ViewFrame* frame = context->getCurrentViewFrame();
-	if (!frame)
+void DebuggerInfoTable::onDoubleClicked()
+{
+	uint64_t value = 0;
+	if (!selectedValue(value))
 		return;
 
+	// Navigate to the target, opening it in the other pane when it is a different kind
+	// of thing (code vs data) than the current pane shows (see NavigateToAddress, #1134).
 	if (m_debugger->GetData())
-		frame->navigate(m_debugger->GetData(), value, true, true);
+		NavigateToAddress(this, m_debugger->GetData(), value);
+}
+
+
+void DebuggerInfoTable::navigateInCurrentPane()
+{
+	uint64_t value = 0;
+	if (selectedValue(value) && m_debugger->GetData())
+		NavigateToAddressInCurrentPane(this, m_debugger->GetData(), value);
+}
+
+
+void DebuggerInfoTable::contextMenuEvent(QContextMenuEvent* event)
+{
+	// Select the row under the cursor so the action targets the right entry even when the
+	// right-click did not already move the selection.
+	QModelIndex index = indexAt(event->pos());
+	if (index.isValid())
+		selectRow(index.row());
+
+	// Force navigation into the currently focused pane (double-click instead picks the
+	// pane by content type; see NavigateToAddress).
+	uint64_t value = 0;
+	if (!selectedValue(value))
+		return;
+
+	QMenu menu(this);
+	QAction* navigate = menu.addAction("Navigate in Current Pane");
+	connect(navigate, &QAction::triggered, this, &DebuggerInfoTable::navigateInCurrentPane);
+	menu.exec(event->globalPos());
 }
 
 

ui/debuggerinfowidget.h

@@ -127,9 +127,15 @@ Q_OBJECT;
 	std::vector<DebuggerInfoEntry> getInfoForHLILConditions(HighLevelILFunctionRef hlil, const HighLevelILInstruction& instr);
 
 	void updateColumnWidths();
+	// The value of the selected entry, interpreted as an address, or false if there is no
+	// selection.
+	bool selectedValue(uint64_t& value);
+
+	virtual void contextMenuEvent(QContextMenuEvent* event) override;
 
 private slots:
 	void onDoubleClicked();
+	void navigateInCurrentPane();
 
 public:
 	DebuggerInfoTable(BinaryViewRef data);

ui/debuggeruicommon.cpp

@@ -15,6 +15,8 @@ limitations under the License.
 */
 
 #include "debuggeruicommon.h"
+#include "uicontext.h"
+#include "viewframe.h"
 
 using namespace BinaryNinja;
 
@@ -69,3 +71,52 @@ bool ParseAddress(const QString& text, BinaryNinja::Ref<BinaryNinja::BinaryView>
 
 	return false;
 }
+
+
+static bool AddressIsInFunction(Ref<BinaryView> data, uint64_t address)
+{
+	return data && !data->GetAnalysisFunctionsContainingAddress(address).empty();
+}
+
+
+void NavigateToAddress(QWidget* widget, Ref<BinaryView> data, uint64_t target)
+{
+	UIContext* context = UIContext::contextForWidget(widget);
+	if (!context)
+		return;
+
+	ViewFrame* frame = context->getCurrentViewFrame();
+	if (!frame)
+		return;
+
+	View* view = frame->getCurrentViewInterface();
+
+	// Decide same-pane vs other-pane by whether the target and the current view are the
+	// same kind of thing (both code, or both data). Matching types navigate in place;
+	// crossing between code and data opens the target in the other pane so what the user
+	// is looking at stays visible.
+	bool targetInFunction = AddressIsInFunction(data, target);
+	bool currentInFunction = view && AddressIsInFunction(data, view->getCurrentOffset());
+
+	if (view && currentInFunction == targetInFunction)
+	{
+		frame->navigate(data, target, true, true);
+		return;
+	}
+
+	// Different types: show the target in the other pane. If there is no other pane to use
+	// (or no view), fall back to navigating the current pane.
+	if (!view || !view->navigateOnOtherPane(target))
+		frame->navigate(data, target, true, true);
+}
+
+
+void NavigateToAddressInCurrentPane(QWidget* widget, Ref<BinaryView> data, uint64_t target)
+{
+	UIContext* context = UIContext::contextForWidget(widget);
+	if (!context)
+		return;
+
+	if (ViewFrame* frame = context->getCurrentViewFrame())
+		frame->navigate(data, target, true, true);
+}

ui/debuggeruicommon.h

@@ -51,3 +51,21 @@ class NumericalTableWidgetItem : public QTableWidgetItem
 // Returns true if parsing succeeded and sets 'result' to the parsed address.
 // Returns false if parsing failed.
 bool ParseAddress(const QString& text, BinaryNinja::Ref<BinaryNinja::BinaryView> data, uint64_t& result, std::string* errorMessage = nullptr);
+
+// Navigate to `target` from a debugger sidebar widget, choosing the pane by content type.
+//
+// If the current pane and `target` are the same kind of thing -- both inside a function
+// (code), or both outside one (data) -- navigate the current pane in place, like an
+// ordinary jump. If they differ, open `target` in the other pane so what the user is
+// looking at stays visible. Because a data target then matches the (data) companion pane,
+// subsequent data double-clicks reuse that pane instead of bouncing back to the code pane;
+// that is what keeps repeated double-clicks from drifting. Falls back to the current pane
+// when there is no other pane available.
+//
+// `widget` is any widget inside the window whose current view should be navigated from.
+void NavigateToAddress(QWidget* widget, BinaryNinja::Ref<BinaryNinja::BinaryView> data, uint64_t target);
+
+// Navigate to `target` in the currently focused pane, in place, unconditionally. This is
+// the "force it into the pane I have focused" fallback exposed via the right-click menu.
+void NavigateToAddressInCurrentPane(
+    QWidget* widget, BinaryNinja::Ref<BinaryNinja::BinaryView> data, uint64_t target);

ui/memorymapwidget.cpp

@@ -25,6 +25,7 @@ limitations under the License.
 #include <algorithm>
 #include "ui.h"
 #include "memorymapwidget.h"
+#include "debuggeruicommon.h"
 #include "clickablelabel.h"
 
 using namespace BinaryNinja;
@@ -705,16 +706,10 @@ void DebugMemoryMapWidget::onDoubleClicked()
 	else
 		address = region.endAddress();
 
-	UIContext* context = UIContext::contextForWidget(this);
-	if (!context)
-		return;
-
-	ViewFrame* frame = context->getCurrentViewFrame();
-	if (!frame)
-		return;
-
+	// Navigate to the target, opening it in the other pane when it is a different kind
+	// of thing (code vs data) than the current pane shows (see NavigateToAddress, #1134).
 	if (m_controller->GetData())
-		frame->navigate(m_controller->GetData(), address, true, true);
+		NavigateToAddress(this, m_controller->GetData(), address);
 };
 
 

ui/moduleswidget.cpp

@@ -21,6 +21,7 @@ limitations under the License.
 #include <QClipboard>
 #include "ui.h"
 #include "moduleswidget.h"
+#include "debuggeruicommon.h"
 #include "clickablelabel.h"
 
 using namespace BinaryNinja;
@@ -697,16 +698,10 @@ void DebugModulesWidget::onDoubleClicked()
 	else
 		address = module.endAddress();
 
-	UIContext* context = UIContext::contextForWidget(this);
-	if (!context)
-		return;
-
-	ViewFrame* frame = context->getCurrentViewFrame();
-	if (!frame)
-		return;
-
+	// Navigate to the target, opening it in the other pane when it is a different kind
+	// of thing (code vs data) than the current pane shows (see NavigateToAddress, #1134).
 	if (m_controller->GetData())
-		frame->navigate(m_controller->GetData(), address, true, true);
+		NavigateToAddress(this, m_controller->GetData(), address);
 };
 
 

ui/registerswidget.cpp

@@ -25,6 +25,7 @@ limitations under the License.
 #include "util.h"
 #include "clickablelabel.h"
 #include "registerswidget.h"
+#include "debuggeruicommon.h"
 #include "base/assertions.h"
 
 using namespace BinaryNinja;
@@ -804,9 +805,23 @@ bool DebugRegistersWidget::canPaste()
 }
 
 
-void DebugRegistersWidget::onDoubleClicked()
+void DebugRegistersWidget::onDoubleClicked(const QModelIndex& index)
 {
-	jump();
+	// Navigate to the address the register holds, opening it in the other pane when it is
+	// a different kind of thing (code vs data) than what the current pane shows, so the
+	// view the user is looking at stays put (see NavigateToAddress and issue #1134).
+	if (!index.isValid())
+		return;
+
+	auto sourceIndex = m_filter->mapToSource(index);
+	if (!sourceIndex.isValid())
+		return;
+
+	auto reg = m_model->getRow(sourceIndex.row());
+	uint64_t value = (uint64_t)reg.value();
+
+	if (m_controller->GetData())
+		NavigateToAddress(this, m_controller->GetData(), value);
 }
 
 

ui/registerswidget.h

@@ -210,7 +210,7 @@ private slots:
 	void editValue();
 	void goToPrevRegisterWrite();
 	void goToNextRegisterWrite();
-	void onDoubleClicked();
+	void onDoubleClicked(const QModelIndex& index);
 	void hoverTimerEvent();
 
 public slots:

ui/threadframes.cpp

@@ -15,6 +15,7 @@ limitations under the License.
 */
 
 #include "threadframes.h"
+#include "debuggeruicommon.h"
 #include <algorithm>
 
 FrameItem::~FrameItem()
@@ -735,6 +736,16 @@ ThreadFramesWidget::ThreadFramesWidget(QWidget* parent, ViewFrame* frame, Binary
 	m_menu.addAction(actionName, "Options", MENU_ORDER_NORMAL);
 	m_actionHandler.bindAction(actionName, UIAction([this]() { copyAllFrames(); }));
 
+	// Force navigation into the currently focused pane (double-click instead picks the
+	// pane by content type; see NavigateToAddress).
+	actionName = QString::fromStdString("Navigate in Current Pane");
+	UIAction::registerAction(actionName);
+	m_menu.addAction(actionName, "Options", MENU_ORDER_FIRST);
+	m_actionHandler.bindAction(actionName, UIAction([this]() { navigateInCurrentPane(); }, [this]() {
+		uint64_t addr = 0;
+		return navigationAddressForSelection(addr);
+	}));
+
 	// TODO: set as active thread action?
 
 	connect(this, &QTreeView::doubleClicked, this, &ThreadFramesWidget::onDoubleClicked);
@@ -849,30 +860,50 @@ void ThreadFramesWidget::onDoubleClicked()
 	}
 
 	uint64_t addrToJump = 0;
-	switch (column)
+	if (!navigationAddressForSelection(addrToJump))
+		return;
+
+	// Navigate to the target, opening it in the other pane when it is a different kind
+	// of thing (code vs data) than the current pane shows (see NavigateToAddress, #1134).
+	if (m_debugger->GetData())
+		NavigateToAddress(this, m_debugger->GetData(), addrToJump);
+}
+
+
+bool ThreadFramesWidget::navigationAddressForSelection(uint64_t& addr)
+{
+	QModelIndexList sel = selectionModel()->selectedIndexes();
+	if (sel.empty())
+		return false;
+
+	const QModelIndex& index = sel[0];
+	FrameItem* frameItem = static_cast<FrameItem*>(index.internalPointer());
+	if (!frameItem || !frameItem->isFrame())
+		return false;
+
+	switch (index.column())
 	{
 	case ThreadFrameModel::FunctionColumn:
 	case ThreadFrameModel::PcColumn:
-		addrToJump = frameItem->framePc();
-		break;
+		addr = frameItem->framePc();
+		return true;
 	case ThreadFrameModel::SpColumn:
-		addrToJump = frameItem->sp();
-		break;
+		addr = frameItem->sp();
+		return true;
 	case ThreadFrameModel::FpColumn:
-		addrToJump = frameItem->fp();
-		break;
+		addr = frameItem->fp();
+		return true;
+	default:
+		return false;
 	}
+}
 
-	UIContext* context = UIContext::contextForWidget(this);
-	if (!context)
-		return;
 
-	ViewFrame* frame = context->getCurrentViewFrame();
-	if (!frame)
-		return;
-
-	if (m_debugger->GetData())
-		frame->navigate(m_debugger->GetData(), addrToJump, true, true);
+void ThreadFramesWidget::navigateInCurrentPane()
+{
+	uint64_t addr = 0;
+	if (navigationAddressForSelection(addr) && m_debugger->GetData())
+		NavigateToAddressInCurrentPane(this, m_debugger->GetData(), addr);
 }