Skip to content

Commit cef49c8

Browse files
committed
Fix debugger step_over to use instruction-level stepping
- Changed step_over implementation from using breakpoints and full emulation to instruction-level stepping for precise control - Both DebuggerWidget and TCP server now use the same stepping approach - Step_over now properly executes JSR and steps through subroutine until return - Added debugStepped signal for UI synchronization - Connected DebuggerWidget to AtariEmulator signals for real-time updates - Fixed synchronization between TCP commands and debug widget display - Safety limit of 10,000 instructions prevents infinite loops in subroutines - Added documentation for debug.step_into and debug.step_over TCP commands This fixes the issue where step_over was using coarse frame-level stepping and ensures consistent behavior between UI and TCP debugging interfaces.
1 parent 1f0e01b commit cef49c8

File tree

6 files changed

+252
-20
lines changed

6 files changed

+252
-20
lines changed

TCP_SERVER_API.md

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -786,12 +786,14 @@ Single-step execution (one frame). Executes approximately 7,000-10,000 instructi
786786
echo '{"command": "debug.step"}' | nc localhost 6502
787787
```
788788

789-
#### `debug.step_instruction`
789+
#### `debug.step_instruction` / `debug.step_into`
790790

791-
Single-instruction stepping for precise debugging. Executes exactly one 6502 CPU instruction.
791+
Single-instruction stepping for precise debugging. Executes exactly one 6502 CPU instruction. Both commands are aliases and perform the same operation.
792792

793793
```bash
794794
echo '{"command": "debug.step_instruction"}' | nc localhost 6502
795+
# or
796+
echo '{"command": "debug.step_into"}' | nc localhost 6502
795797
```
796798

797799
**Response:**
@@ -811,8 +813,38 @@ echo '{"command": "debug.step_instruction"}' | nc localhost 6502
811813
- Requires emulation to be paused first
812814
- Advances PC by the exact instruction size (1-3 bytes)
813815
- Much more precise than `debug.step` for debugging
816+
- Will step INTO subroutines when encountering JSR instructions
814817
- Ideal for IDE integration and breakpoint debugging
815818

819+
#### `debug.step_over`
820+
821+
Step over subroutine calls. When encountering a JSR (Jump to Subroutine) instruction, executes the entire subroutine and returns to the instruction following the JSR. For non-JSR instructions, behaves like `step_into`.
822+
823+
```bash
824+
echo '{"command": "debug.step_over"}' | nc localhost 6502
825+
```
826+
827+
**Response:**
828+
```json
829+
{
830+
"type": "response",
831+
"status": "success",
832+
"result": {
833+
"stepped": true,
834+
"pc": "$2004",
835+
"step_over": true
836+
}
837+
}
838+
```
839+
840+
**Notes:**
841+
- Requires emulation to be paused first
842+
- When at JSR ($20): executes entire subroutine using instruction-level stepping
843+
- When not at JSR: executes single instruction like `step_into`
844+
- Uses instruction-level stepping internally (not frame stepping)
845+
- Safety limit of 10,000 instructions prevents infinite loops
846+
- Ideal for debugging without diving into subroutine implementation
847+
816848
#### `debug.load_xex_for_debug`
817849

818850
Load an XEX file and pause after loading completes, ready for debugging. The emulator steps through frames to ensure the XEX is fully loaded into memory before pausing. This allows setting breakpoints before program execution begins.

include/atariemulator.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ public slots:
296296
void breakpointsCleared();
297297
void executionPaused();
298298
void executionResumed();
299+
void debugStepped();
299300

300301
private:
301302
unsigned char convertQtKeyToAtari(int key, Qt::KeyboardModifiers modifiers);

include/debuggerwidget.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@ public slots:
5151

5252
private slots:
5353
void refreshDebugInfo();
54+
55+
// Emulator signal handlers
56+
void onEmulatorBreakpointHit(unsigned short address);
57+
void onEmulatorBreakpointAdded(unsigned short address);
58+
void onEmulatorBreakpointRemoved(unsigned short address);
59+
void onEmulatorBreakpointsCleared();
60+
void onEmulatorExecutionPaused();
61+
void onEmulatorExecutionResumed();
62+
void onEmulatorDebugStepped();
5463

5564
private:
5665
void setupUI();

src/atariemulator.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2118,6 +2118,7 @@ void AtariEmulator::stepOneFrame()
21182118
// Execute one frame manually when paused
21192119
processFrame();
21202120
qDebug() << "Stepped one frame - PC:" << QString("$%1").arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
2121+
emit debugStepped();
21212122
} else {
21222123
qDebug() << "Cannot step frame - emulation not paused";
21232124
}
@@ -2133,6 +2134,7 @@ void AtariEmulator::stepOneInstruction()
21332134
checkBreakpoints();
21342135

21352136
qDebug() << "Stepped one instruction - PC:" << QString("$%1").arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
2137+
emit debugStepped();
21362138
} else {
21372139
qDebug() << "Cannot step instruction - emulation not paused";
21382140
}

src/debuggerwidget.cpp

Lines changed: 137 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,17 @@ void DebuggerWidget::connectSignals()
261261
connect(m_removeBreakpointButton, &QPushButton::clicked, this, &DebuggerWidget::onRemoveBreakpointClicked);
262262
connect(m_clearBreakpointsButton, &QPushButton::clicked, this, &DebuggerWidget::clearAllBreakpoints);
263263
connect(m_breakpointListWidget, &QListWidget::itemSelectionChanged, this, &DebuggerWidget::onBreakpointSelectionChanged);
264+
265+
// Connect to emulator debugging signals
266+
if (m_emulator) {
267+
connect(m_emulator, &AtariEmulator::breakpointHit, this, &DebuggerWidget::onEmulatorBreakpointHit);
268+
connect(m_emulator, &AtariEmulator::breakpointAdded, this, &DebuggerWidget::onEmulatorBreakpointAdded);
269+
connect(m_emulator, &AtariEmulator::breakpointRemoved, this, &DebuggerWidget::onEmulatorBreakpointRemoved);
270+
connect(m_emulator, &AtariEmulator::breakpointsCleared, this, &DebuggerWidget::onEmulatorBreakpointsCleared);
271+
connect(m_emulator, &AtariEmulator::executionPaused, this, &DebuggerWidget::onEmulatorExecutionPaused);
272+
connect(m_emulator, &AtariEmulator::executionResumed, this, &DebuggerWidget::onEmulatorExecutionResumed);
273+
connect(m_emulator, &AtariEmulator::debugStepped, this, &DebuggerWidget::onEmulatorDebugStepped);
274+
}
264275
}
265276

266277
void DebuggerWidget::updateCPUState()
@@ -904,35 +915,43 @@ void DebuggerWidget::stepOverSubroutine()
904915
unsigned char opcode = MEMORY_mem[currentPC];
905916

906917
if (isSubroutineCall(opcode)) {
907-
// This is a JSR instruction - set breakpoint at return address
908-
m_stepOverBreakPC = currentPC + 3; // JSR is 3 bytes
918+
// This is a JSR instruction - execute it and then run until return address
919+
unsigned short returnAddress = currentPC + 3; // JSR is 3 bytes
909920

910-
qDebug() << QString("Step Over: JSR at $%1, setting break at $%2")
921+
qDebug() << QString("Step Over: JSR at $%1, target return at $%2")
911922
.arg(currentPC, 4, 16, QChar('0')).toUpper()
912-
.arg(m_stepOverBreakPC, 4, 16, QChar('0')).toUpper();
923+
.arg(returnAddress, 4, 16, QChar('0')).toUpper();
924+
925+
// Execute the JSR instruction first
926+
m_emulator->stepOneInstruction();
913927

914-
// Execute frames until we reach the breakpoint
915-
int maxFrames = 1000; // Safety limit
916-
int frameCount = 0;
928+
// Now we're inside the subroutine, step until we reach the return address
929+
// Use a safety limit to prevent infinite loops
930+
int maxSteps = 10000; // Maximum instructions to execute
931+
int stepCount = 0;
917932

918-
while (frameCount < maxFrames && CPU_regPC != m_stepOverBreakPC) {
919-
m_emulator->stepOneFrame();
920-
frameCount++;
933+
while (CPU_regPC != returnAddress && stepCount < maxSteps) {
934+
m_emulator->stepOneInstruction();
935+
stepCount++;
921936

922-
// Update display every 10 frames to show progress
923-
if (frameCount % 10 == 0) {
937+
// Update display periodically for long subroutines
938+
if (stepCount % 100 == 0) {
924939
refreshDebugInfo();
925-
QCoreApplication::processEvents(); // Allow UI updates
940+
QCoreApplication::processEvents();
926941
}
927942
}
928943

929-
if (frameCount >= maxFrames) {
930-
qDebug() << "Step Over: Timeout reached, subroutine may not return";
944+
if (stepCount >= maxSteps) {
945+
qDebug() << QString("Step Over: Timeout after %1 steps, subroutine may not return")
946+
.arg(stepCount);
931947
} else {
932-
qDebug() << QString("Step Over: Completed after %1 frames, PC at $%2")
933-
.arg(frameCount)
948+
qDebug() << QString("Step Over: Completed after %1 steps, PC at $%2")
949+
.arg(stepCount)
934950
.arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
935951
}
952+
953+
// Refresh the display
954+
refreshDebugInfo();
936955
} else {
937956
// Not a subroutine call, just step one instruction
938957
stepSingleInstruction();
@@ -1065,4 +1084,105 @@ void DebuggerWidget::checkBreakpoints()
10651084
{
10661085
// This functionality has been moved to the core emulator
10671086
// to ensure breakpoints work consistently across all interfaces (TCP, Debug window)
1087+
}
1088+
1089+
// Handler for when a breakpoint is hit
1090+
void DebuggerWidget::onEmulatorBreakpointHit(unsigned short address)
1091+
{
1092+
qDebug() << QString("Debugger: Breakpoint hit at $%1").arg(address, 4, 16, QChar('0')).toUpper();
1093+
1094+
// Update the UI to reflect the paused state (handled by onEmulatorExecutionPaused)
1095+
// Refresh debug info to show current state
1096+
refreshDebugInfo();
1097+
}
1098+
1099+
// Handler for when a breakpoint is added (e.g., from TCP)
1100+
void DebuggerWidget::onEmulatorBreakpointAdded(unsigned short address)
1101+
{
1102+
qDebug() << QString("Debugger: Breakpoint added at $%1").arg(address, 4, 16, QChar('0')).toUpper();
1103+
1104+
// Update our local breakpoint list from the emulator
1105+
m_breakpoints = m_emulator->getBreakpoints();
1106+
updateBreakpointList();
1107+
1108+
// Refresh disassembly to show new breakpoint indicator
1109+
updateDisassemblyView();
1110+
}
1111+
1112+
// Handler for when a breakpoint is removed (e.g., from TCP)
1113+
void DebuggerWidget::onEmulatorBreakpointRemoved(unsigned short address)
1114+
{
1115+
qDebug() << QString("Debugger: Breakpoint removed at $%1").arg(address, 4, 16, QChar('0')).toUpper();
1116+
1117+
// Update our local breakpoint list from the emulator
1118+
m_breakpoints = m_emulator->getBreakpoints();
1119+
updateBreakpointList();
1120+
1121+
// Refresh disassembly to remove breakpoint indicator
1122+
updateDisassemblyView();
1123+
}
1124+
1125+
// Handler for when all breakpoints are cleared (e.g., from TCP)
1126+
void DebuggerWidget::onEmulatorBreakpointsCleared()
1127+
{
1128+
qDebug() << "Debugger: All breakpoints cleared";
1129+
1130+
// Clear our local breakpoint list
1131+
m_breakpoints.clear();
1132+
updateBreakpointList();
1133+
1134+
// Refresh disassembly to remove breakpoint indicators
1135+
updateDisassemblyView();
1136+
}
1137+
1138+
// Handler for when execution is paused (e.g., from TCP or breakpoint hit)
1139+
void DebuggerWidget::onEmulatorExecutionPaused()
1140+
{
1141+
qDebug() << "Debugger: Execution paused";
1142+
1143+
// Update internal state
1144+
m_isRunning = false;
1145+
1146+
// Update button states
1147+
if (m_stepIntoButton && m_stepOverButton && m_runButton && m_pauseButton) {
1148+
m_stepIntoButton->setEnabled(true);
1149+
m_stepOverButton->setEnabled(true);
1150+
m_runButton->setEnabled(true);
1151+
m_pauseButton->setEnabled(false);
1152+
}
1153+
1154+
// Stop the refresh timer
1155+
m_refreshTimer->stop();
1156+
1157+
// Immediately refresh the debug info to show current state
1158+
refreshDebugInfo();
1159+
}
1160+
1161+
// Handler for when execution is resumed (e.g., from TCP)
1162+
void DebuggerWidget::onEmulatorExecutionResumed()
1163+
{
1164+
qDebug() << "Debugger: Execution resumed";
1165+
1166+
// Update internal state
1167+
m_isRunning = true;
1168+
1169+
// Update button states
1170+
if (m_stepIntoButton && m_stepOverButton && m_runButton && m_pauseButton) {
1171+
m_stepIntoButton->setEnabled(false);
1172+
m_stepOverButton->setEnabled(false);
1173+
m_runButton->setEnabled(false);
1174+
m_pauseButton->setEnabled(true);
1175+
}
1176+
1177+
// Start the refresh timer for continuous updates
1178+
m_refreshTimer->start();
1179+
}
1180+
1181+
// Handler for when a debug step occurs (from TCP or UI)
1182+
void DebuggerWidget::onEmulatorDebugStepped()
1183+
{
1184+
qDebug() << "Debugger: Step executed";
1185+
1186+
// Immediately refresh all debug information
1187+
refreshDebugInfo();
10681188
}

src/tcpserver.cpp

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <QBuffer>
2222
#include <QImageReader>
2323
#include <QDateTime>
24+
#include <QThread>
2425

2526
extern "C" {
2627
// Access to CPU registers for debug commands
@@ -1418,8 +1419,9 @@ void TCPServer::handleDebugCommand(QTcpSocket* client, const QJsonObject& reques
14181419
"Emulation must be paused to step");
14191420
}
14201421

1421-
} else if (subCommand == "step_instruction") {
1422+
} else if (subCommand == "step_instruction" || subCommand == "step_into") {
14221423
// Single-instruction stepping (precise debugging)
1424+
// step_into is an alias for step_instruction - both step into subroutines naturally
14231425
if (m_emulator->isEmulationPaused()) {
14241426
m_emulator->stepOneInstruction();
14251427

@@ -1441,6 +1443,72 @@ void TCPServer::handleDebugCommand(QTcpSocket* client, const QJsonObject& reques
14411443
"Emulation must be paused to step");
14421444
}
14431445

1446+
} else if (subCommand == "step_over") {
1447+
// Step over subroutine calls
1448+
if (m_emulator->isEmulationPaused()) {
1449+
unsigned short currentPC = CPU_regPC;
1450+
unsigned char opcode = MEMORY_mem[currentPC];
1451+
1452+
// Check if this is a JSR instruction (0x20)
1453+
if (opcode == 0x20) {
1454+
// JSR instruction - use instruction-level stepping to execute subroutine
1455+
unsigned short returnAddress = currentPC + 3; // JSR is 3 bytes
1456+
1457+
qDebug() << QString("TCP step_over: JSR at $%1, target return at $%2")
1458+
.arg(currentPC, 4, 16, QChar('0')).toUpper()
1459+
.arg(returnAddress, 4, 16, QChar('0')).toUpper();
1460+
1461+
// Execute the JSR instruction first
1462+
m_emulator->stepOneInstruction();
1463+
1464+
// Now we're inside the subroutine, step until we reach the return address
1465+
// Use a safety limit to prevent infinite loops
1466+
int maxSteps = 10000; // Maximum instructions to execute
1467+
int stepCount = 0;
1468+
1469+
while (CPU_regPC != returnAddress && stepCount < maxSteps) {
1470+
m_emulator->stepOneInstruction();
1471+
stepCount++;
1472+
1473+
// Process events periodically for long subroutines
1474+
if (stepCount % 100 == 0) {
1475+
QCoreApplication::processEvents();
1476+
}
1477+
}
1478+
1479+
if (stepCount >= maxSteps) {
1480+
qDebug() << QString("TCP step_over: Timeout after %1 steps, subroutine may not return")
1481+
.arg(stepCount);
1482+
} else {
1483+
qDebug() << QString("TCP step_over: Completed after %1 steps, PC=$%2")
1484+
.arg(stepCount)
1485+
.arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
1486+
}
1487+
} else {
1488+
// Not a JSR, just step one instruction
1489+
m_emulator->stepOneInstruction();
1490+
qDebug() << QString("TCP step_over: Not JSR (opcode=$%1), single step")
1491+
.arg(opcode, 2, 16, QChar('0')).toUpper();
1492+
}
1493+
1494+
QJsonObject result;
1495+
result["stepped"] = true;
1496+
result["pc"] = QString("$%1").arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
1497+
result["step_over"] = true;
1498+
1499+
sendResponse(client, requestId, true, result);
1500+
1501+
// Send event to all clients
1502+
QJsonObject eventData;
1503+
eventData["stepped"] = true;
1504+
eventData["pc"] = QString("$%1").arg(CPU_regPC, 4, 16, QChar('0')).toUpper();
1505+
eventData["step_over"] = true;
1506+
sendEventToAllClients("debug_stepped", eventData);
1507+
} else {
1508+
sendResponse(client, requestId, false, QJsonValue(),
1509+
"Emulation must be paused to step");
1510+
}
1511+
14441512
} else if (subCommand == "disassemble") {
14451513
// Disassemble memory at specified address
14461514
int address = params["address"].toInt(CPU_regPC);

0 commit comments

Comments
 (0)