reworked breakpoints to use hash map (and % $1fff addresses)

src/debugger/Debugger.cxx

@@ -191,6 +191,12 @@ string Debugger::autoExec(StringList* history)
   return buf.str();
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+BreakpointMap& Debugger::breakPoints() const
+{
+  return mySystem.m6502().breakPoints();
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 TrapArray& Debugger::readTraps() const
 {
@@ -326,10 +332,10 @@ int Debugger::trace()
 
     // set temporary breakpoint at target PC (if not existing already)
     Int8 bank = myCartDebug->getBank();
-    if(checkBreakPoint(targetPC, bank) == NOT_FOUND)
+    if(!checkBreakPoint(targetPC, bank))
     {
       // add temporary breakpoint and remove later
-      setBreakPoint(targetPC, bank, true, true);
+      setBreakPoint(targetPC, bank, true);
     }
 
     unlockSystem();
@@ -345,65 +351,44 @@ int Debugger::trace()
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-bool Debugger::setBreakPoint(uInt16 addr, Int8 bank, bool set, bool oneShot)
+bool Debugger::setBreakPoint(uInt16 addr, Int8 bank, bool oneShot)
 {
-  Int32 id = checkBreakPoint(addr, bank);
+  bool exists = checkBreakPoint(addr, bank);
 
-  if(set)
+  if(exists)
-  {
+    return false;
-    if(id != NOT_FOUND)
-      return false;
 
-    mySystem.m6502().addCondBreak(YaccParser::getResult(), getCondition(addr, bank), oneShot);
+  breakPoints().add(addr, bank, oneShot ? BreakpointMap::ONE_SHOT : 0);
-  }
-  else
-  {
-    if(id == NOT_FOUND)
-      return false;
-
-    m6502().delCondBreak(id);
-  }
   return true;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-Int32 Debugger::checkBreakPoint(uInt16 addr, Int8 bank)
+bool Debugger::clearBreakPoint(uInt16 addr, Int8 bank)
 {
-  string condition = getCondition(addr, bank);
+  bool exists = checkBreakPoint(addr, bank);
 
-  for(uInt32 i = 0; i < m6502().getCondBreakNames().size(); ++i)
+  if(!exists)
-    if(condition == m6502().getCondBreakNames()[i])
+    return false;
-      return i;
 
-  return NOT_FOUND;
+  breakPoints().erase(addr, bank);
+  return true;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-string Debugger::getCondition(uInt16 addr, Int8 bank)
+bool Debugger::checkBreakPoint(uInt16 addr, Int8 bank)
 {
-  stringstream condition;
+  return breakPoints().check(addr, bank);
-
-  condition << "((pc&1fff) == " << Base::HEX4 << (addr & 0x1fff) << ")";
-  if(bank != ANY_BANK && myCartDebug->bankCount() > 1)
-    condition << " && (_bank == " << Base::HEX1 << int(bank) << ")";
-
-  // parse and validate condition expression
-  int res = YaccParser::parse(condition.str());
-  if(res != 0)
-  {
-    cerr << "Invalid condition: " << condition.str() << " (" << res << ")" << endl;
-    return "";
-  }
-
-  return condition.str();
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool Debugger::toggleBreakPoint(uInt16 addr, Int8 bank)
 {
-  setBreakPoint(addr, bank, checkBreakPoint(addr, bank) == NOT_FOUND);
+  if(checkBreakPoint(addr, bank))
+    clearBreakPoint(addr, bank);
+  else
+    setBreakPoint(addr, bank);
 
-  return checkBreakPoint(addr, bank) != NOT_FOUND;
+  return breakPoints().check(addr, bank);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -631,6 +616,12 @@ uInt16 Debugger::unwindStates(const uInt16 numStates, string& message)
   return windStates(numStates, true, message);
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void Debugger::clearAllBreakPoints()
+{
+  breakPoints().clear();
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 void Debugger::clearAllTraps()
 {

src/debugger/Debugger.hxx

@@ -27,6 +27,7 @@ class TiaZoomWidget;
 class EditTextWidget;
 class RomWidget;
 class Expression;
+class BreakpointMap;
 class TrapArray;
 class PromptWidget;
 class ButtonWidget;
@@ -151,23 +152,39 @@ class Debugger : public DialogContainer
     RomWidget& rom() const              { return myDialog->rom();       }
     TiaOutputWidget& tiaOutput() const  { return myDialog->tiaOutput(); }
 
+    BreakpointMap& breakPoints() const;
+
     TrapArray& readTraps() const;
     TrapArray& writeTraps() const;
 
     /**
-      Sets or clears a breakpoint.
+      Sets a breakpoint.
 
-      Returns true if successfully set or cleared
+      Returns true if successfully set
     */
     bool setBreakPoint(uInt16 addr, Int8 bank = ANY_BANK,
-                       bool set = true, bool oneShot = false);
+                       bool oneShot = false);
+
+    /**
+      Clears a breakpoint.
+
+      Returns true if successfully cleared
+    */
+    bool clearBreakPoint(uInt16 addr, Int8 bank);
+
+    /**
+      Toggles a breakpoint
+
+      Returns new state of breakpoint
+    */
+    bool toggleBreakPoint(uInt16 addr, Int8 bank);
 
     /**
       Checks for a breakpoint.
 
-      Returns -1 if not existing, else the Id
+      Returns true if existing, else false
     */
-    Int32 checkBreakPoint(uInt16 addr, Int8 bank);
+    bool checkBreakPoint(uInt16 addr, Int8 bank);
 
     /**
       Run the debugger command and return the result.
@@ -295,8 +312,7 @@ class Debugger : public DialogContainer
     uInt16 rewindStates(const uInt16 numStates, string& message);
     uInt16 unwindStates(const uInt16 numStates, string& message);
 
-    bool toggleBreakPoint(uInt16 addr, Int8 bank);
+    void clearAllBreakPoints();
-    string getCondition(uInt16 addr, Int8 bank);
 
     void addReadTrap(uInt16 t);
     void addWriteTrap(uInt16 t);

src/debugger/DebuggerParser.cxx

@@ -647,6 +647,9 @@ string DebuggerParser::saveScriptFile(string file)
   for(const auto& w: myWatches)
     out << "watch " << w << endl;
 
+  for(const auto& bp: debugger.breakPoints().getBreakpoints())
+    out << "break " << Base::toString(bp.addr) << " " << Base::toString(bp.bank) << endl;
+
   StringList conds = debugger.m6502().getCondBreakNames();
   for(const auto& cond : conds)
     out << "breakif {" << cond << "}" << endl;
@@ -738,22 +741,40 @@ void DebuggerParser::executeBreak()
   else
   {
     bank = args[1];
-    if(bank < 0 || bank >= debugger.cartDebug().bankCount())
+    if(bank < -1 || bank >= debugger.cartDebug().bankCount())
     {
       commandResult << red("invalid bank");
       return;
     }
   }
+  if(bank > -1)
+  {
+    bool set = debugger.toggleBreakPoint(addr, bank);
 
-  bool set = debugger.toggleBreakPoint(addr, bank);
+    if(set)
-  debugger.rom().invalidate();
+      commandResult << "set";
+    else
+      commandResult << "cleared";
 
-  if(set)
+    commandResult << " breakpoint at $" << Base::HEX4 << addr << " in bank #" << std::dec << int(bank);
-    commandResult << "set";
+  }
   else
-    commandResult << "cleared";
+  {
+    for(int i = 0; i < debugger.cartDebug().bankCount(); ++i)
+    {
+      bool set = debugger.toggleBreakPoint(addr, i);
 
-  commandResult << " breakpoint at $" << Base::toString(addr) << " in bank $" << Base::HEX1 << int(bank);
+      if(i)
+        commandResult << endl;
+
+      if(set)
+        commandResult << "set";
+      else
+        commandResult << "cleared";
+
+      commandResult << " breakpoint at $" << Base::HEX4 << addr << " in bank #" << std::dec << int(i);
+    }
+  }
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -820,6 +841,7 @@ void DebuggerParser::executeCheat()
 // "clearbreaks"
 void DebuggerParser::executeClearbreaks()
 {
+  debugger.clearAllBreakPoints();
   debugger.m6502().clearCondBreaks();
   commandResult << "all breakpoints cleared";
 }
@@ -1419,7 +1441,34 @@ void DebuggerParser::executeJump()
 // "listbreaks"
 void DebuggerParser::executeListbreaks()
 {
+  stringstream buf;
   int count = 0;
+  uInt32 bankCount = debugger.cartDebug().bankCount();
+
+  for(uInt32 bank = 0; bank < bankCount; ++bank)
+  {
+    for(uInt32 addr = 0; addr <= 0x1fff; ++addr)
+    {
+      if(debugger.breakPoints().check(addr, bank))
+      {
+        if(!bankCount)
+        {
+          buf << debugger.cartDebug().getLabel(addr, true, 4) << " ";
+          if(!(++count % 8)) buf << endl;
+        }
+        else
+        {
+          if(count % 6)
+            buf << ", ";
+          buf << debugger.cartDebug().getLabel(addr, true, 4) << " #" << int(bank);
+          if(!(++count % 6)) buf << endl;
+        }
+      }
+    }
+  }
+  if(count)
+    commandResult << "breaks:" << endl << buf.str();
+
   StringList conds = debugger.m6502().getCondBreakNames();
 
   if(conds.size() > 0)

src/debugger/gui/Cart3EWidget.cxx

@@ -152,7 +152,6 @@ void Cartridge3EWidget::handleCommand(CommandSender* sender,
   invalidate();
 }
 
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 string Cartridge3EWidget::bankState()
 {
@@ -160,9 +159,9 @@ string Cartridge3EWidget::bankState()
 
   uInt16& bank = myCart.myCurrentBank;
   if(bank < 256)
-    buf << "ROM bank " << std::dec << bank % myNumRomBanks << ", RAM inactive";
+    buf << "ROM bank #" << std::dec << bank % myNumRomBanks << ", RAM inactive";
   else
-    buf << "ROM inactive, RAM bank " << bank % myNumRomBanks;
+    buf << "ROM inactive, RAM bank #" << std::dec << bank % myNumRomBanks;
 
   return buf.str();
 }

src/debugger/gui/Cart3FWidget.cxx

@@ -88,7 +88,7 @@ string Cartridge3FWidget::bankState()
 {
   ostringstream& buf = buffer();
 
-  buf << "Bank = " << std::dec << myCart.myCurrentBank << ", hotspot = $3F";
+  buf << "Bank = #" << std::dec << myCart.myCurrentBank << ", hotspot = $3F";
 
   return buf.str();
 }

src/debugger/gui/CartF0Widget.cxx

@@ -30,7 +30,7 @@ CartridgeF0Widget::CartridgeF0Widget(
 
   ostringstream info;
   info << "64K Megaboy F0 cartridge, 16 4K banks\n"
-       << "Startup bank = " << cart.startBank() << " or undetermined\n"
+       << "Startup bank = #" << cart.startBank() << " or undetermined\n"
        << "Bankswitch triggered by accessing $1FF0\n";
 
   // Eventually, we should query this from the debugger/disassembler
@@ -64,9 +64,9 @@ CartridgeF0Widget::CartridgeF0Widget(
   VarList::push_back(items, " 14");
   VarList::push_back(items, " 15");
   myBank =
-    new PopUpWidget(boss, _font, xpos, ypos-2, _font.getStringWidth(" 15 "),
+    new PopUpWidget(boss, _font, xpos, ypos-2, _font.getStringWidth(" 15"),
-                    myLineHeight, items, "Set bank ",
+                    myLineHeight, items, "Set bank   #",
-                    _font.getStringWidth("Set bank "), kBankChanged);
+                    _font.getStringWidth("Set bank   #"), kBankChanged);
   myBank->setTarget(this);
   addFocusWidget(myBank);
 }
@@ -102,7 +102,7 @@ string CartridgeF0Widget::bankState()
 {
   ostringstream& buf = buffer();
 
-  buf << "Bank = " << std::dec << myCart.getBank() << ", hotspot = $FFF0";
+  buf << "Bank = #" << std::dec << myCart.getBank() << ", hotspot = $FFF0";
 
   return buf.str();
 }

src/debugger/gui/RomListWidget.cxx

@@ -434,8 +434,7 @@ void RomListWidget::handleCommand(CommandSender* sender, int cmd, int data, int
     case CheckboxWidget::kCheckActionCmd:
       // We let the parent class handle this
       // Pass it as a kRLBreakpointChangedCmd command, since that's the intent
-      sendCommand(RomListWidget::kBPointChangedCmd, _currentPos+id,
+      sendCommand(RomListWidget::kBPointChangedCmd, _currentPos+id, 0);
-                  myCheckList[id]->getState());
       break;
 
     case GuiObject::kSetPositionCmd:
@@ -493,7 +492,7 @@ void RomListWidget::drawWidget(bool hilite)
 
     // Draw checkboxes for correct lines (takes scrolling into account)
     myCheckList[i]->setState(instance().debugger().
-                             checkBreakPoint(dlist[pos].address, instance().debugger().cartDebug().getBank()) != Debugger::NOT_FOUND);
+                             checkBreakPoint(dlist[pos].address, instance().debugger().cartDebug().getBank()));
 
     myCheckList[i]->setDirty();
     myCheckList[i]->draw();

src/debugger/gui/RomWidget.cxx

@@ -92,8 +92,7 @@ void RomWidget::handleCommand(CommandSender* sender, int cmd, int data, int id)
   {
     case RomListWidget::kBPointChangedCmd:
       // 'data' is the line in the disassemblylist to be accessed
-      // 'id' is the state of the breakpoint at 'data'
+      toggleBreak(data);
-      setBreak(data, id);
       // Refresh the romlist, since the breakpoint may not have
       // actually changed
       myRomList->setDirty();
@@ -164,15 +163,15 @@ void RomWidget::handleCommand(CommandSender* sender, int cmd, int data, int id)
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void RomWidget::setBreak(int disasm_line, bool state)
+void RomWidget::toggleBreak(int disasm_line)
 {
   const CartDebug::DisassemblyList& list =
       instance().debugger().cartDebug().disassembly().list;
   if(disasm_line >= int(list.size()))  return;
 
   if(list[disasm_line].address != 0 && list[disasm_line].bytes != "")
-    instance().debugger().setBreakPoint(list[disasm_line].address,
+    instance().debugger().toggleBreakPoint(list[disasm_line].address,
-                                        instance().debugger().cartDebug().getBank(), state);
+                                           instance().debugger().cartDebug().getBank());
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

src/debugger/gui/RomWidget.hxx

@@ -48,7 +48,7 @@ class RomWidget : public Widget, public CommandSender
     void handleCommand(CommandSender* sender, int cmd, int data, int id) override;
     void loadConfig() override;
 
-    void setBreak(int disasm_line, bool state);
+    void toggleBreak(int disasm_line);
     void setPC(int disasm_line);
     void runtoPC(int disasm_line);
     void patchROM(int disasm_line, const string& bytes,

src/debugger/module.mk

@@ -1,16 +1,17 @@
 MODULE := src/debugger
 
 MODULE_OBJS := \
-	src/debugger/Debugger.o \
+        src/debugger/BreakpointMap.o
-	src/debugger/DebuggerParser.o \
+        src/debugger/Debugger.o \
-	src/debugger/CartDebug.o \
+        src/debugger/DebuggerParser.o \
-	src/debugger/CpuDebug.o \
+        src/debugger/CartDebug.o \
-	src/debugger/DiStella.o \
+        src/debugger/CpuDebug.o \
-	src/debugger/RiotDebug.o \
+        src/debugger/DiStella.o \
-	src/debugger/TIADebug.o
+        src/debugger/RiotDebug.o \
+        src/debugger/TIADebug.o
 
 MODULE_DIRS += \
-	src/debugger
+        src/debugger
 
 # Include common rules 
 include $(srcdir)/common.rules

src/emucore/M6502.cxx

@@ -279,18 +279,35 @@ inline void M6502::_execute(uInt64 cycles, DispatchResult& result)
           return;
         }
 
+        if(myBreakPoints.isInitialized())
+        {
+          uInt8 bank = mySystem->cart().getBank();
+
+          if(myBreakPoints.check(PC, bank))
+          {
+            myLastBreakCycle = mySystem->cycles();
+            // disable a one-shot breakpoint
+            if(myBreakPoints.get(PC, bank) & BreakpointMap::ONE_SHOT)
+            {
+              myBreakPoints.erase(PC, bank);
+            }
+            else
+            {
+              ostringstream msg;
+
+              msg << "BP: $" << Common::Base::HEX4 << PC << ", bank #" << std::dec << int(bank);
+              result.setDebugger(currentCycles, msg.str());
+            }
+            return;
+          }
+        }
+
         int cond = evalCondBreaks();
         if(cond > -1)
         {
           ostringstream msg;
 
-          // one shot break (trace)?
+          msg << "CBP[" << Common::Base::HEX2 << cond << "]: " << myCondBreakNames[cond];
-          if(myCondBreakFlags[cond])
-          {
-            delCondBreak(cond);
-          }
-          else
-            msg << "BP[" << Common::Base::HEX2 << cond << "]: " << myCondBreakNames[cond];
 
           myLastBreakCycle = mySystem->cycles();
           result.setDebugger(currentCycles, msg.str());
@@ -540,7 +557,6 @@ uInt32 M6502::addCondBreak(Expression* e, const string& name, bool oneShot)
 {
   myCondBreaks.emplace_back(e);
   myCondBreakNames.push_back(name);
-  myCondBreakFlags.push_back(oneShot);
 
   updateStepStateByInstruction();
 
@@ -554,7 +570,6 @@ bool M6502::delCondBreak(uInt32 idx)
   {
     Vec::removeAt(myCondBreaks, idx);
     Vec::removeAt(myCondBreakNames, idx);
-    Vec::removeAt(myCondBreakFlags, idx);
 
     updateStepStateByInstruction();
 

src/emucore/M6502.hxx

@@ -30,6 +30,7 @@ class DispatchResult;
 
   #include "Expression.hxx"
   #include "TrapArray.hxx"
+  #include "BreakpointMap.hxx"
 #endif
 
 #include "bspf.hxx"
@@ -214,6 +215,8 @@ class M6502 : public Serializable
     TrapArray& readTraps() { return myReadTraps; }
     TrapArray& writeTraps() { return myWriteTraps; }
 
+    BreakpointMap& breakPoints() { return myBreakPoints; }
+
     // methods for 'breakif' handling
     uInt32 addCondBreak(Expression* e, const string& name, bool oneShot = false);
     bool delCondBreak(uInt32 idx);
@@ -435,9 +438,9 @@ class M6502 : public Serializable
     };
     HitTrapInfo myHitTrapInfo;
 
+    BreakpointMap myBreakPoints;
     vector<unique_ptr<Expression>> myCondBreaks;
     StringList myCondBreakNames;
-    BoolArray myCondBreakFlags;
     vector<unique_ptr<Expression>> myCondSaveStates;
     StringList myCondSaveStateNames;
     vector<unique_ptr<Expression>> myTrapConds;

src/windows/Stella.vcxproj

@@ -393,6 +393,7 @@
     <ClCompile Include="..\common\tv_filters\AtariNTSC.cxx" />
     <ClCompile Include="..\common\tv_filters\NTSCFilter.cxx" />
     <ClCompile Include="..\common\ZipHandler.cxx" />
+    <ClCompile Include="..\debugger\BreakpointMap.cxx" />
     <ClCompile Include="..\debugger\gui\AmigaMouseWidget.cxx" />
     <ClCompile Include="..\debugger\gui\AtariMouseWidget.cxx" />
     <ClCompile Include="..\debugger\gui\AtariVoxWidget.cxx" />
@@ -1098,6 +1099,7 @@
     <ClInclude Include="..\common\Variant.hxx" />
     <ClInclude Include="..\common\Vec.hxx" />
     <ClInclude Include="..\common\ZipHandler.hxx" />
+    <ClInclude Include="..\debugger\BreakpointMap.hxx" />
     <ClInclude Include="..\debugger\gui\AmigaMouseWidget.hxx" />
     <ClInclude Include="..\debugger\gui\AtariMouseWidget.hxx" />
     <ClInclude Include="..\debugger\gui\AtariVoxWidget.hxx" />

src/windows/Stella.vcxproj.filters

@@ -978,6 +978,9 @@
     <ClCompile Include="..\common\JoyMap.cxx">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="..\debugger\BreakpointMap.cxx">
+      <Filter>Source Files\debugger</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\common\bspf.hxx">
@@ -1997,6 +2000,9 @@
     <ClInclude Include="..\common\JoyMap.hxx">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="..\debugger\BreakpointMap.hxx">
+      <Filter>Header Files\debugger</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <None Include="stella.ico">