Merge remote-tracking branch 'origin/master' into blendsplitter

Keeping up to date with the latest work on the BreakpointViewer

Author: turbor

src/BreakpointViewer.cpp

@@ -1,6 +1,7 @@
 #include "BreakpointViewer.h"
 #include "Convert.h"
 #include "CommClient.h"
+#include "DebugSession.h"
 #include "OpenMSXConnection.h"
 #include "ScopedAssign.h"
 #include "ranges.h"
@@ -17,17 +18,18 @@ enum TableColumns {
 	ENABLED = 0,
 	WP_TYPE = 1,
 	LOCATION = 2,
-	BP_ADDRESS = 2,
 	WP_REGION = 2,
 	T_CONDITION = 3,
 	SLOT = 4,
 	SEGMENT = 5,
-	ID = 6
+	ID = 6,
+	REAL_ADDRESS = 7,
 };
 
-BreakpointViewer::BreakpointViewer(QWidget* parent)
-	: QTabWidget(parent),
-	  ui(new Ui::BreakpointViewer)
+BreakpointViewer::BreakpointViewer(DebugSession& session, QWidget* parent)
+	:  QTabWidget(parent),
+	  ui(new Ui::BreakpointViewer),
+	  debugSession(session)
 {
 	setupUi(this);
 
@@ -39,9 +41,10 @@ BreakpointViewer::BreakpointViewer(QWidget* parent)
 	connect(btnRemoveCn, &QPushButton::clicked, this, &BreakpointViewer::on_btnRemoveCn_clicked);
 
 	bpTableWidget->horizontalHeader()->setHighlightSections(false);
-	bpTableWidget->sortByColumn(BP_ADDRESS, Qt::AscendingOrder);
+	bpTableWidget->sortByColumn(LOCATION, Qt::AscendingOrder);
 	bpTableWidget->setColumnHidden(WP_TYPE, true);
 	bpTableWidget->setColumnHidden(ID, true);
+	bpTableWidget->setColumnHidden(REAL_ADDRESS, true);
 	bpTableWidget->resizeColumnsToContents();
 	bpTableWidget->setSortingEnabled(true);
 	connect(bpTableWidget, &QTableWidget::itemPressed, this, &BreakpointViewer::on_itemPressed);
@@ -51,6 +54,7 @@ BreakpointViewer::BreakpointViewer(QWidget* parent)
 
 	wpTableWidget->horizontalHeader()->setHighlightSections(false);
 	wpTableWidget->setColumnHidden(ID, true);
+	wpTableWidget->setColumnHidden(REAL_ADDRESS, true);
 	wpTableWidget->sortByColumn(WP_REGION, Qt::AscendingOrder);
 	wpTableWidget->resizeColumnsToContents();
 	wpTableWidget->setSortingEnabled(true);
@@ -254,7 +258,7 @@ void BreakpointViewer::setBreakpointChecked(BreakpointRef::Type type, int row, Q
 	table->setSortingEnabled(oldValue);
 }
 
-void BreakpointViewer::setTextField(BreakpointRef::Type type, int row, int column, const QString& value)
+void BreakpointViewer::setTextField(BreakpointRef::Type type, int row, int column, const QString& value, const QString& tooltip)
 {
 	auto sa = ScopedAssign(userMode, false);
 
@@ -264,6 +268,7 @@ void BreakpointViewer::setTextField(BreakpointRef::Type type, int row, int colum
 
 	table->setSortingEnabled(false);
 	item->setText(value);
+	item->setToolTip(tooltip);
 	table->setSortingEnabled(oldValue);
 }
 
@@ -309,13 +314,24 @@ std::optional<uint8_t> BreakpointViewer::parseSegmentField(std::optional<int> in
 	return {};
 }
 
+std::optional<AddressRange> BreakpointViewer::parseSymbolOrValue(const QString& field) const
+{
+	if (Symbol* s = debugSession.symbolTable().getAddressSymbol(field)) {
+		return AddressRange{uint16_t(s->value())};
+	}
+	if (auto address = stringToValue<uint16_t>(field)) {
+		return AddressRange{*address};
+	}
+	return {};
+}
+
 static const char* ComboTypeNames[] = { "read_mem", "write_mem", "read_io", "write_io" };
 
 std::optional<AddressRange> BreakpointViewer::parseLocationField(
 	std::optional<int> index, BreakpointRef::Type type, const QString& field, const QString& comboTxt)
 {
 	if (type == BreakpointRef::BREAKPOINT) {
-		auto value = stringToValue<uint16_t>(field);
+		auto value = parseSymbolOrValue(field);
 		return value ? AddressRange{*value}
 		    : (index ? breakpoints->getBreakpoint(*index).range : std::optional<AddressRange>());
 	}
@@ -367,27 +383,32 @@ void BreakpointViewer::changeTableItem(BreakpointRef::Type type, QTableWidgetIte
 		case LOCATION: {
 			auto* model = table->model();
 			auto* combo = (QComboBox*) table->indexWidget(model->index(row, WP_TYPE));
-			int   adrLen;
+			int adrLen;
 
 			if (type == BreakpointRef::CONDITION) {
 				return;
 			} else if (type == BreakpointRef::WATCHPOINT) {
 				auto wType = static_cast<Breakpoint::Type>(combo->currentIndex() + 1);
 				adrLen = (wType == Breakpoint::WATCHPOINT_IOREAD || wType == Breakpoint::WATCHPOINT_IOWRITE)
-					? 2 : 4;
+				       ? 2 : 4;
 			} else {
 				adrLen = 4;
 			}
 
 			if (auto range = parseLocationField(index, type, item->text(), combo ? combo->currentText() : "")) {
 				auto [begin, end] = *range;
-				setTextField(type, row, LOCATION, QString("%1%2%3")
-				                                  .arg(hexValue(begin, adrLen))
-				                                  .arg(end ? ":" : "")
-				                                  .arg(end ? hexValue(*end, adrLen) : ""));
+				QString address = QString("%1%2%3")
+				        .arg(hexValue(begin, adrLen))
+				        .arg(end ? ":" : "")
+				        .arg(end ? hexValue(*end, adrLen) : "");
+				// Use a symbolic address in the location field if available
+				QString location = debugSession.symbolTable().getAddressSymbol(item->text()) ? item->text() : address;
+				setTextField(type, row, LOCATION, location, location != address ? address : "");
+				setTextField(type, row, REAL_ADDRESS, address);
 			} else {
 				enabled = false;
 				setTextField(type, row, LOCATION, "");
+				setTextField(type, row, REAL_ADDRESS, "");
 				setBreakpointChecked(type, row, Qt::Unchecked);
 			}
 			if (!enabled) return;
@@ -706,7 +727,7 @@ int BreakpointViewer::createTableRow(BreakpointRef::Type type, int row)
 		createComboBox(row);
 	}
 
-	// address
+	// location
 	auto* item2 = new QTableWidgetItem();
 	item2->setTextAlignment(Qt::AlignCenter);
 	table->setItem(row, LOCATION, item2);
@@ -742,6 +763,12 @@ int BreakpointViewer::createTableRow(BreakpointRef::Type type, int row)
 	item6->setText("");
 	table->setItem(row, ID, item6);
 
+	// real_address
+	auto* item7 = new QTableWidgetItem();
+	item7->setFlags(Qt::NoItemFlags);
+	item7->setText("");
+	table->setItem(row, REAL_ADDRESS, item7);
+
 	return row;
 }
 
@@ -762,15 +789,24 @@ void BreakpointViewer::fillTableRow(int index, BreakpointRef::Type type, int row
 		combo->setCurrentIndex(static_cast<int>(bp.type) - 1);
 	}
 
-	// location
-	int locLen = (bp.type == Breakpoint::WATCHPOINT_IOREAD
-	           || bp.type == Breakpoint::WATCHPOINT_IOWRITE) ? 2 : 4;
-	QString location = QString("%1%2%3").arg(hexValue(bp.range->start, locLen))
-	                   .arg(bp.range->end ? ":" : "")
-	                   .arg(bp.range->end ? hexValue(*bp.range->end, locLen) : "");
-
 	// location
 	auto* item2 = table->item(row, LOCATION);
+	QString location;
+
+	if (bp.type == Breakpoint::BREAKPOINT) {
+		if (Symbol* s = debugSession.symbolTable().getAddressSymbol(item2->text())) {
+			if (s->value() == bp.range->start) {
+				location = s->text();
+			}
+		}
+	}
+	if (location.isEmpty()) {
+		int locLen = (bp.type == Breakpoint::WATCHPOINT_IOREAD
+		           || bp.type == Breakpoint::WATCHPOINT_IOWRITE) ? 2 : 4;
+		location = QString("%1%2%3").arg(hexValue(bp.range->start, locLen))
+		               .arg(bp.range->end ? ":" : "")
+		               .arg(bp.range->end ? hexValue(*bp.range->end, locLen) : "");
+	}
 	item2->setText(location);
 
 	auto* item3 = table->item(row, T_CONDITION);
@@ -817,6 +853,27 @@ std::optional<Breakpoint> BreakpointViewer::parseTableRow(BreakpointRef::Type ty
 	return bp;
 }
 
+
+void BreakpointViewer::onSymbolTableChanged()
+{
+	for (int row = 0; row < bpTableWidget->rowCount(); ++row) {
+		auto* item = bpTableWidget->item(row, REAL_ADDRESS);
+
+		// scan tooltip validity
+		if (!item->text().isEmpty()) {
+			Symbol* s = debugSession.symbolTable().getAddressSymbol(item->text());
+			auto address = stringToValue<uint16_t>(item->text());
+			assert(address);
+
+			if (!s || *address != s->value()) {
+				setTextField(BreakpointRef::BREAKPOINT, row, LOCATION, item->text());
+				setTextField(BreakpointRef::BREAKPOINT, row, REAL_ADDRESS, "");
+			}
+		}
+	}
+}
+
+
 void BreakpointViewer::onAddBtnClicked(BreakpointRef::Type type)
 {
 	auto sa = ScopedAssign(userMode, false);

src/BreakpointViewer.h

@@ -10,6 +10,7 @@
 
 class QPaintEvent;
 class Breakpoints;
+class DebugSession;
 
 struct BreakpointRef {
 	enum Type { BREAKPOINT, WATCHPOINT, CONDITION, ALL } type;
@@ -24,9 +25,10 @@ class BreakpointViewer : public QTabWidget, private Ui::BreakpointViewer
 {
 	Q_OBJECT
 public:
-	BreakpointViewer(QWidget* parent = nullptr);
+	BreakpointViewer(DebugSession& session, QWidget* parent = nullptr);
 	void setBreakpoints(Breakpoints* bps);
 
+	void onSymbolTableChanged();
 	void on_btnAddBp_clicked();
 	void on_btnRemoveBp_clicked();
 	void on_btnAddWp_clicked();
@@ -42,7 +44,10 @@ class BreakpointViewer : public QTabWidget, private Ui::BreakpointViewer
 	void contentsUpdated(bool merge);
 
 private:
-	void setTextField(BreakpointRef::Type type, int row, int column, const QString& value);
+	void setTextField(BreakpointRef::Type type, int row, int column, const QString& value, const QString& tooltip = {});
+
+	std::optional<AddressRange> parseSymbolOrValue(const QString& field) const;
+
 	std::optional<AddressRange> parseLocationField(std::optional<int> index,
 	                                               BreakpointRef::Type type,
 	                                               const QString& field,
@@ -86,6 +91,8 @@ class BreakpointViewer : public QTabWidget, private Ui::BreakpointViewer
 
 private:
 	Ui::BreakpointViewer* ui;
+	DebugSession& debugSession;
+
 	QTableWidget* tables[BreakpointRef::ALL];
 	std::map<QString, BreakpointRef> maps[BreakpointRef::ALL];
 

src/BreakpointViewer.ui

@@ -115,6 +115,11 @@
         <string>#</string>
        </property>
       </column>
+      <column>
+       <property name="text">
+        <string>real_addr</string>
+       </property>
+      </column>
      </widget>
     </item>
     <item>
@@ -237,6 +242,11 @@
         <string>#</string>
        </property>
       </column>
+      <column>
+       <property name="text">
+        <string>real_addr</string>
+       </property>
+      </column>
      </widget>
     </item>
     <item>

src/CPURegsViewer.cpp

@@ -2,6 +2,7 @@
 #include "CPURegs.h"
 #include "CommClient.h"
 #include "OpenMSXConnection.h"
+#include "ranges.h"
 #include <QPainter>
 #include <QPaintEvent>
 #include <QMessageBox>
@@ -12,9 +13,9 @@ CPURegsViewer::CPURegsViewer(QWidget* parent)
 {
     setObjectName("CPURegsViewer");
 	// avoid UMR
-	memset(&regs,         0, sizeof(regs));
-	memset(&regsChanged,  0, sizeof(regsChanged));
-	memset(&regsModified, 0, sizeof(regsModified));
+	regs.fill(0);
+	regsChanged.fill(false);
+	regsModified.fill(false);
 
 	setFrameStyle(WinPanel | Sunken);
 	setFocusPolicy(Qt::StrongFocus);
@@ -174,8 +175,8 @@ void CPURegsViewer::setData(uint8_t* datPtr)
 
 	// reset modifications
 	cursorLoc = -1;
-	memset(&regsModified, 0, sizeof(regsModified));
-	memcpy(&regsCopy, &regs, sizeof(regs));
+	regsModified.fill(false);
+	regsCopy = regs;
 
 	update();
 
@@ -223,7 +224,7 @@ void CPURegsViewer::keyPressEvent(QKeyEvent* e)
 	int move = e->key();
 	if ((e->key() >= Qt::Key_0 && e->key() <= Qt::Key_9) ||
 	    (e->key() >= Qt::Key_A && e->key() <= Qt::Key_F)) {
-		// calculate numercial value
+		// calculate numerical value
 		int v = e->key() - Qt::Key_0;
 		if (v > 9) v -= Qt::Key_A - Qt::Key_0 - 10;
 		// modify value
@@ -351,26 +352,36 @@ void CPURegsViewer::applyModifications()
 	}
 	// update screen
 	update();
+
+	// update disasm window
+	if (regsChanged[CpuRegs::REG_PC]) {
+		emit pcChanged(regs[CpuRegs::REG_PC]);
+	}
 }
 
 void CPURegsViewer::cancelModifications()
 {
-	bool mod = false;
-	for (int i = 0; i < 14; ++i) mod |= regsModified[i];
-	if (!mod) return;
-
-	int ret = QMessageBox::warning(
-		this,
-		tr("CPU registers changes"),
-		tr("You made changes to the CPU registers.\n"
-		   "Do you want to apply your changes or ignore them?"),
-		QMessageBox::Apply | QMessageBox::Ignore,
-		QMessageBox::Ignore);
-	if (ret == QMessageBox::Ignore) {
-		memcpy(&regs, &regsCopy, sizeof(regs));
-		memset(&regsModified, 0, sizeof(regsModified));
-	} else {
-		applyModifications();
+	static bool isVisible = false;
+
+	if (!ranges::contains(regsModified, true)) return;
+
+	if (!isVisible) {
+		isVisible = true;
+		// this modal window blocks execution of next instruction
+		int ret = QMessageBox::warning(
+			this,
+			tr("CPU registers changes"),
+			tr("You made changes to the CPU registers.\n"
+			   "Do you want to apply your changes or ignore them?"),
+			QMessageBox::Apply | QMessageBox::Ignore,
+			QMessageBox::Ignore);
+		isVisible = false;
+		if (ret == QMessageBox::Ignore) {
+			regs = regsCopy;
+			regsModified.fill(false);
+		} else {
+			applyModifications();
+		}
 	}
 }
 

src/CPURegsViewer.h

@@ -2,6 +2,7 @@
 #define CPUREGSVIEWER_H
 
 #include <QFrame>
+#include <array>
 
 class QPaintEvent;
 
@@ -33,9 +34,10 @@ public slots:
 	int leftRegPos, leftValuePos, rightRegPos, rightValuePos;
 	int rowHeight;
 
-	int regs[16], regsCopy[16];
-	bool regsModified[16];
-	bool regsChanged[16];
+	std::array<int, 16> regs;
+	std::array<int, 16> regsCopy;
+	std::array<bool, 16> regsModified;
+	std::array<bool, 16> regsChanged;
 	int cursorLoc;
 
 	void drawValue(QPainter& p, int id, int x, int y);