Sanitize halt and hardware state handling after CPU dispatch.

.vscode/settings.json

@@ -25,6 +25,7 @@
         "string": "cpp",
         "string_view": "cpp",
         "system_error": "cpp",
-        "vector": "cpp"
+        "vector": "cpp",
+        "stdexcept": "cpp"
     }
 }

src/debugger/Debugger.cxx

@@ -781,6 +781,19 @@ void Debugger::unlockBankswitchState()
   myConsole.cartridge().unlockBank();
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Debugger::update()
+{
+  if (myDialog) {
+    myDialog->setDirty();
+
+    // loadConfig is redeclared static in DebuggerDialog, hence the cast
+    static_cast<Dialog*>(myDialog)->loadConfig();
+
+    myDialog->drawDialog();
+  }
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 Debugger::BuiltinFunction Debugger::ourBuiltinFunctions[NUM_BUILTIN_FUNCS] = {
   // left joystick:

src/debugger/Debugger.hxx

@@ -240,6 +240,11 @@ class Debugger : public DialogContainer
     void lockBankswitchState();
     void unlockBankswitchState();
 
+    /**
+      Update debugger status.
+    */
+    void update();
+
   private:
     /**
       Save state of each debugger subsystem and, by default, mark all

src/emucore/M6502.cxx

@@ -215,6 +215,30 @@ void M6502::updateStepStateByInstruction()
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool M6502::execute(uInt32 number)
+{
+  const ExecuteResult result = _execute(number);
+
+  // Debugger hack: this ensures that stepping a "STA WSYNC" will actually end at the
+  // beginning of the next line (otherwise, the next instruction would be stepped in order for
+  // the halt to take effect). This is safe because as we know that the next cycle will be a read
+  // cycle anyway.
+  handleHalt();
+
+  // Make sure that the hardware state matches the current system clock. This is necessary
+  // to ensure that the state is displayed correctly in the debugger. The performance impact
+  // on emulation is negligible as M6502::execute is called only once per frame.
+  mySystem->tia().updateEmulation();
+  mySystem->m6532().updateEmulation();
+
+#ifdef DEBUGGER_SUPPORT
+  if (result == ExecuteResult::debuggerTrap && myDebugger) myDebugger->update();
+#endif
+
+  return result != ExecuteResult::failure;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+inline M6502::ExecuteResult M6502::_execute(uInt32 number)
 {
   // Clear all of the execution status bits except for the fatal error bit
   myExecutionStatus &= FatalErrorBit;
@@ -236,13 +260,13 @@ bool M6502::execute(uInt32 number)
         myJustHitReadTrapFlag = myJustHitWriteTrapFlag = false;
         if(myDebugger && myDebugger->start(myHitTrapInfo.message, myHitTrapInfo.address, read))
         {
-          return true;
+          return ExecuteResult::debuggerTrap;
         }
       }
 
       if(myBreakPoints.isInitialized() && myBreakPoints.isSet(PC))
         if(myDebugger && myDebugger->start("BP: ", PC))
-          return true;
+          return ExecuteResult::debuggerTrap;
 
       int cond = evalCondBreaks();
       if(cond > -1)
@@ -250,7 +274,7 @@ bool M6502::execute(uInt32 number)
         stringstream msg;
         msg << "CBP[" << Common::Base::HEX2 << cond << "]: " << myCondBreakNames[cond];
         if(myDebugger && myDebugger->start(msg.str()))
-          return true;
+          return ExecuteResult::debuggerTrap;
       }
 
       cond = evalCondSaveStates();
@@ -303,31 +327,22 @@ bool M6502::execute(uInt32 number)
     // See if execution has been stopped
     if(myExecutionStatus & StopExecutionBit)
     {
-      // Debugger hack: this ensures that stepping a "STA WSYNC" will actually end at the
-      // beginning of the next line (otherwise, the next instruction would be stepped in order for
-      // the halt to take effect). This is safe because as we know that the next cycle will be a read
-      // cycle anyway.
-      handleHalt();
-
       // Yes, so answer that everything finished fine
-      return true;
+      return ExecuteResult::success;
     }
 
     // See if a fatal error has occured
     if(myExecutionStatus & FatalErrorBit)
     {
       // Yes, so answer that something when wrong
-      return false;
+      return ExecuteResult::failure;
     }
 
     // See if we've executed the specified number of instructions
     if(number == 0)
     {
-      // See above
-      handleHalt();
-
       // Yes, so answer that everything finished fine
-      return true;
+      return ExecuteResult::success;
     }
   }
 }

src/emucore/M6502.hxx

@@ -320,6 +320,21 @@ class M6502 : public Serializable
     */
     void updateStepStateByInstruction();
 
+    /**
+      The dispatch result.
+    */
+    enum ExecuteResult {
+      success,
+      failure,
+      debuggerTrap
+    };
+
+    /**
+      This is the actual dispatch function that does the grunt work. M6502::execute
+      wraps it and makes sure that any pending halt is processed before returning.
+    */
+    ExecuteResult _execute(uInt32 number);
+
   private:
     /**
       Bit fields used to indicate that certain conditions need to be

src/emucore/tia/TIA.cxx

@@ -1064,8 +1064,7 @@ TIA& TIA::updateScanline()
 {
   // Update frame by one scanline at a time
   uInt32 line = scanlines();
-  while (line == scanlines() && mySystem->m6502().execute(1))
-    updateEmulation();
+  while (line == scanlines() && mySystem->m6502().execute(1));
 
   return *this;
 }
@@ -1074,8 +1073,7 @@ TIA& TIA::updateScanline()
 TIA& TIA::updateScanlineByStep()
 {
   // Update frame by one CPU instruction/color clock
-  if (mySystem->m6502().execute(1))
-    updateEmulation();
+  mySystem->m6502().execute(1);
 
   return *this;
 }
@@ -1085,8 +1083,7 @@ TIA& TIA::updateScanlineByTrace(int target)
 {
   uInt32 count = 100;  // only try up to 100 steps
   while (mySystem->m6502().getPC() != target && count-- &&
-         mySystem->m6502().execute(1))
-    updateEmulation();
+         mySystem->m6502().execute(1));
 
   return *this;
 }