From 575b019659189d3290ebbb4f342517eec1d2aded Mon Sep 17 00:00:00 2001 From: harry Date: Sat, 4 Feb 2023 15:15:31 -0500 Subject: [PATCH] Changed core symbol table to have private data that can be accessed via methods. The goal is to control access to this data to prevent table lookups getting messed up when symbols are editted via the gui. --- src/debug.cpp | 2 +- src/debugsymboltable.cpp | 97 +++++++++++++++++------------- src/debugsymboltable.h | 97 ++++++++++++++++++++++++------ src/drivers/Qt/ConsoleDebugger.cpp | 24 ++++---- src/drivers/Qt/SymbolicDebug.cpp | 55 ++++++++--------- src/lua-engine.cpp | 2 +- 6 files changed, 174 insertions(+), 103 deletions(-) diff --git a/src/debug.cpp b/src/debug.cpp index f542e3c8..28636313 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -46,7 +46,7 @@ int offsetStringToInt(unsigned int type, const char* offsetBuffer) if (sym) { - return sym->ofs & 0xFFFF; + return sym->offset() & 0xFFFF; } int type = GIT_CART; diff --git a/src/debugsymboltable.cpp b/src/debugsymboltable.cpp index 76b4b0b8..b8f85441 100644 --- a/src/debugsymboltable.cpp +++ b/src/debugsymboltable.cpp @@ -39,13 +39,22 @@ debugSymbolPage_t::~debugSymbolPage_t(void) int debugSymbolPage_t::addSymbol( debugSymbol_t*sym ) { // Check if symbol already is loaded by that name or offset - if ( symMap.count( sym->ofs ) || symNameMap.count( sym->name ) ) + if ( symMap.count( sym->offset() ) ) + { + return -1; + } + if ( (sym->name().size() > 0) && symNameMap.count( sym->name() ) ) { return -1; } - symMap[ sym->ofs ] = sym; - symNameMap[ sym->name ] = sym; + symMap[ sym->offset() ] = sym; + + // Comment only lines don't need to have a name. + if (sym->name().size() > 0) + { + symNameMap[ sym->name() ] = sym; + } return 0; } @@ -53,13 +62,13 @@ int debugSymbolPage_t::addSymbol( debugSymbol_t*sym ) debugSymbol_t *debugSymbolPage_t::getSymbolAtOffset( int ofs ) { auto it = symMap.find( ofs ); - return it != symMap.end() ? it->second : NULL; + return it != symMap.end() ? it->second : nullptr; } //-------------------------------------------------------------- debugSymbol_t *debugSymbolPage_t::getSymbol( const std::string &name ) { auto it = symNameMap.find( name ); - return it != symNameMap.end() ? it->second : NULL; + return it != symNameMap.end() ? it->second : nullptr; } //-------------------------------------------------------------- int debugSymbolPage_t::deleteSymbolAtOffset( int ofs ) @@ -68,17 +77,19 @@ int debugSymbolPage_t::deleteSymbolAtOffset( int ofs ) if ( it != symMap.end() ) { - auto itName = symNameMap.find( it->second->name ); - - if ( itName != symNameMap.end() ) + if ( it->second->name().size() > 0 ) { - delete it->second; + auto itName = symNameMap.find( it->second->name() ); - symMap.erase(it); - symNameMap.erase(itName); - - return 0; + if ( itName != symNameMap.end() ) + { + symNameMap.erase(itName); + } } + delete it->second; + symMap.erase(it); + + return 0; } return -1; } @@ -106,7 +117,7 @@ int debugSymbolPage_t::save(void) romFile = getRomFile(); - if ( romFile == NULL ) + if ( romFile == nullptr ) { return -1; } @@ -140,7 +151,7 @@ int debugSymbolPage_t::save(void) fp = ::fopen( filename.c_str(), "w" ); - if ( fp == NULL ) + if ( fp == nullptr ) { printf("Error: Could not open file '%s' for writing\n", filename.c_str() ); return -1; @@ -152,7 +163,7 @@ int debugSymbolPage_t::save(void) sym = it->second; - i=0; j=0; c = sym->comment.c_str(); + i=0; j=0; c = sym->_comment.c_str(); while ( c[i] != 0 ) { @@ -167,7 +178,7 @@ int debugSymbolPage_t::save(void) } stmp[j] = 0; - fprintf( fp, "$%04X#%s#%s\n", sym->ofs, sym->name.c_str(), stmp ); + fprintf( fp, "$%04X#%s#%s\n", sym->offset(), sym->name().c_str(), stmp ); j=0; while ( c[i] != 0 ) @@ -208,7 +219,7 @@ void debugSymbolPage_t::print(void) { sym = it->second; - fprintf( fp, " Sym: $%04X '%s' \n", sym->ofs, sym->name.c_str() ); + fprintf( fp, " Sym: $%04X '%s' \n", sym->ofs, sym->name().c_str() ); } } //-------------------------------------------------------------- @@ -249,7 +260,7 @@ static int generateNLFilenameForBank(int bank, std::string &NLfilename) romFile = getRomFile(); - if ( romFile == NULL ) + if ( romFile == nullptr ) { return -1; } @@ -311,8 +322,8 @@ int debugSymbolTable_t::loadFileNL( int bank ) int i, j, ofs, lineNum = 0, literal = 0, array = 0; std::string fileName; char stmp[512], line[512]; - debugSymbolPage_t *page = NULL; - debugSymbol_t *sym = NULL; + debugSymbolPage_t *page = nullptr; + debugSymbol_t *sym = nullptr; FCEU::autoScopedLock alock(cs); //printf("Looking to Load Debug Bank: $%X \n", bank ); @@ -325,7 +336,7 @@ int debugSymbolTable_t::loadFileNL( int bank ) fp = ::fopen( fileName.c_str(), "r" ); - if ( fp == NULL ) + if ( fp == nullptr ) { return -1; } @@ -367,9 +378,9 @@ int debugSymbolTable_t::loadFileNL( int bank ) } j--; } - if ( sym != NULL ) + if ( sym != nullptr ) { - sym->comment.append( stmp ); + sym->_comment.append( stmp ); } } else if ( line[i] == '$' ) @@ -388,7 +399,7 @@ int debugSymbolTable_t::loadFileNL( int bank ) } stmp[j] = 0; - ofs = strtol( stmp, NULL, 16 ); + ofs = strtol( stmp, nullptr, 16 ); if ( line[i] == '/' ) { @@ -399,7 +410,7 @@ int debugSymbolTable_t::loadFileNL( int bank ) } stmp[j] = 0; - array = strtol( stmp, NULL, 16 ); + array = strtol( stmp, nullptr, 16 ); } if ( line[i] != '#' ) @@ -474,15 +485,15 @@ int debugSymbolTable_t::loadFileNL( int bank ) } i++; - sym = new debugSymbol_t(); + sym = new debugSymbol_t( ofs, stmp ); - if ( sym == NULL ) + if ( sym == nullptr ) { printf("Error: Failed to allocate memory for offset $%04X Name '%s' on Line %i of File %s\n", ofs, stmp, lineNum, fileName.c_str() ); continue; } sym->ofs = ofs; - sym->name.assign( stmp ); + sym->_name.assign( stmp ); while ( isspace( line[i] ) ) i++; @@ -507,11 +518,11 @@ int debugSymbolTable_t::loadFileNL( int bank ) j--; } - sym->comment.assign( stmp ); + sym->_comment.assign( stmp ); if ( array > 0 ) { - debugSymbol_t *arraySym = NULL; + debugSymbol_t *arraySym = nullptr; for (j=0; jofs = sym->ofs + j; sprintf( stmp, "[%i]", j ); - arraySym->name.assign( sym->name ); - arraySym->name.append( stmp ); - arraySym->comment.assign( sym->comment ); + arraySym->_name.assign( sym->name() ); + arraySym->_name.append( stmp ); + arraySym->_comment.assign( sym->comment() ); if ( page->addSymbol( arraySym ) ) { - printf("Error: Failed to add symbol for offset $%04X Name '%s' on Line %i of File %s\n", ofs, arraySym->name.c_str(), lineNum, fileName.c_str() ); - delete arraySym; arraySym = NULL; // Failed to add symbol + printf("Error: Failed to add symbol for offset $%04X Name '%s' on Line %i of File %s\n", ofs, arraySym->name().c_str(), lineNum, fileName.c_str() ); + delete arraySym; arraySym = nullptr; // Failed to add symbol } } } - delete sym; sym = NULL; // Delete temporary symbol + delete sym; sym = nullptr; // Delete temporary symbol } else { if ( page->addSymbol( sym ) ) { - printf("Error: Failed to add symbol for offset $%04X Name '%s' on Line %i of File %s\n", ofs, sym->name.c_str(), lineNum, fileName.c_str() ); - delete sym; sym = NULL; // Failed to add symbol + printf("Error: Failed to add symbol for offset $%04X Name '%s' on Line %i of File %s\n", ofs, sym->name().c_str(), lineNum, fileName.c_str() ); + delete sym; sym = nullptr; // Failed to add symbol } } } @@ -605,7 +616,7 @@ int debugSymbolTable_t::loadGameSymbols(void) this->save(); this->clear(); - if ( GameInfo != NULL ) + if ( GameInfo != nullptr ) { romSize = 16 + CHRsize[0] + PRGsize[0]; } @@ -696,7 +707,7 @@ debugSymbol_t *debugSymbolTable_t::getSymbolAtBankOffset( int bank, int ofs ) auto it = pageMap.find( bank ); - return it != pageMap.end() ? it->second->getSymbolAtOffset( ofs ) : NULL; + return it != pageMap.end() ? it->second->getSymbolAtOffset( ofs ) : nullptr; } //-------------------------------------------------------------- debugSymbol_t *debugSymbolTable_t::getSymbol( int bank, const std::string &name ) @@ -705,7 +716,7 @@ debugSymbol_t *debugSymbolTable_t::getSymbol( int bank, const std::string &name auto it = pageMap.find( bank ); - return it != pageMap.end() ? it->second->getSymbol( name ) : NULL; + return it != pageMap.end() ? it->second->getSymbol( name ) : nullptr; } //-------------------------------------------------------------- debugSymbol_t *debugSymbolTable_t::getSymbolAtAnyBank( const std::string &name ) @@ -722,7 +733,7 @@ debugSymbol_t *debugSymbolTable_t::getSymbolAtAnyBank( const std::string &name ) } } - return NULL; + return nullptr; } //-------------------------------------------------------------- void debugSymbolTable_t::save(void) diff --git a/src/debugsymboltable.h b/src/debugsymboltable.h index 31027c94..30c5a4ba 100644 --- a/src/debugsymboltable.h +++ b/src/debugsymboltable.h @@ -6,49 +6,100 @@ #include "utils/mutex.h" -struct debugSymbol_t -{ - int ofs; - std::string name; - std::string comment; +class debugSymbolPage_t; +class debugSymbolTable_t; +class debugSymbol_t +{ + public: debugSymbol_t(void) { ofs = 0; }; - debugSymbol_t( int ofs, const char *name, const char *comment = nullptr ) + debugSymbol_t( int ofs, const char *name = nullptr, const char *comment = nullptr ) { this->ofs = ofs; if (name) { - this->name.assign(name); + this->_name.assign(name); } if ( comment ) { - this->comment.assign( comment ); + this->_comment.assign( comment ); + } + } + + const std::string &name(void) + { + return _name; + } + + const std::string &comment(void) + { + return _comment; + } + + void commentAssign( std::string str ) + { + _comment.assign(str); + return; + } + + void commentAssign( const char *str ) + { + _comment.assign(str); + return; + } + + int offset(void) + { + return ofs; + } + + void setOffset( int o ) + { + if (o != ofs) + { + ofs = o; + } + } + + void updateName( const char *name, int arrayIndex = -1 ) + { + _name.assign( name ); + + trimTrailingSpaces(); + + if (arrayIndex >= 0) + { + char stmp[32]; + + sprintf( stmp, "[%i]", arrayIndex ); + + _name.append(stmp); } } void trimTrailingSpaces(void) { - while ( name.size() > 0 ) + while ( _name.size() > 0 ) { - if ( isspace( name.back() ) ) + if ( isspace( _name.back() ) ) { - name.pop_back(); + _name.pop_back(); } else { break; } } - while ( comment.size() > 0 ) + while ( _comment.size() > 0 ) { - if ( isspace( comment.back() ) ) + if ( isspace( _comment.back() ) ) { - comment.pop_back(); + _comment.pop_back(); } else { @@ -56,12 +107,20 @@ struct debugSymbol_t } } } + + private: + + int ofs; + std::string _name; + std::string _comment; + + friend class debugSymbolPage_t; + friend class debugSymbolTable_t; }; -struct debugSymbolPage_t +class debugSymbolPage_t { - int pageNum; - + public: debugSymbolPage_t(void); ~debugSymbolPage_t(void); @@ -77,8 +136,12 @@ struct debugSymbolPage_t debugSymbol_t *getSymbol( const std::string &name ); + private: + int pageNum; std::map symMap; std::map symNameMap; + + friend class debugSymbolTable_t; }; class debugSymbolTable_t diff --git a/src/drivers/Qt/ConsoleDebugger.cpp b/src/drivers/Qt/ConsoleDebugger.cpp index ddf8b560..bb6c6a8c 100644 --- a/src/drivers/Qt/ConsoleDebugger.cpp +++ b/src/drivers/Qt/ConsoleDebugger.cpp @@ -3677,13 +3677,13 @@ void QAsmView::updateAssemblyView(void) char stmp[256]; //printf("Debug symbol Found at $%04X \n", dbgSym->ofs ); - if ( dbgSym->name.size() > 0 ) + if ( dbgSym->name().size() > 0 ) { d = new dbg_asm_entry_t(); *d = *a; d->type = dbg_asm_entry_t::SYMBOL_NAME; - d->text.assign( " " + dbgSym->name ); + d->text.assign( " " + dbgSym->name() ); d->text.append( ":"); d->line = asmEntry.size(); @@ -3691,7 +3691,7 @@ void QAsmView::updateAssemblyView(void) } i=0; j=0; - c = dbgSym->comment.c_str(); + c = dbgSym->comment().c_str(); while ( c[i] != 0 ) { @@ -5181,15 +5181,15 @@ bool QAsmView::event(QEvent *event) showAddrDesc = (c.x() >= pcLocLinePos) && (c.x() < byteCodeLinePos) && opcodeValid; - if ( (c.x() > operandLinePos) && opcodeValid && (asmEntry[line]->sym.name.size() > 0) ) + if ( (c.x() > operandLinePos) && opcodeValid && (asmEntry[line]->sym.name().size() > 0) ) { - size_t subStrLoc = asmEntry[line]->text.find( asmEntry[line]->sym.name, operandLinePos ); + size_t subStrLoc = asmEntry[line]->text.find( asmEntry[line]->sym.name(), operandLinePos ); if ( (subStrLoc != std::string::npos) && (subStrLoc > static_cast(operandLinePos)) ) { //printf("Line:%i asmEntry DB Sym: %zi '%s'\n", line, subStrLoc, asmEntry[line]->sym.name.c_str() ); int symTextStart = subStrLoc; - int symTextEnd = subStrLoc + asmEntry[line]->sym.name.size(); + int symTextEnd = subStrLoc + asmEntry[line]->sym.name().size(); if ( (c.x() >= symTextStart) && (c.x() < symTextEnd) ) { @@ -5253,7 +5253,7 @@ bool QAsmView::event(QEvent *event) } else if ( showSymHexDecode ) { - sprintf( stmp, "$%04X", asmEntry[line]->sym.ofs ); + sprintf( stmp, "$%04X", asmEntry[line]->sym.offset() ); QToolTip::showText(helpEvent->globalPos(), tr(stmp), this ); } @@ -5757,15 +5757,15 @@ void QAsmView::mousePressEvent(QMouseEvent * event) { i = selChar; - if ( asmEntry[line]->sym.name.size() > 0 ) + if ( asmEntry[line]->sym.name().size() > 0 ) { - size_t subStrLoc = asmEntry[line]->text.find( asmEntry[line]->sym.name, operandLinePos ); + size_t subStrLoc = asmEntry[line]->text.find( asmEntry[line]->sym.name(), operandLinePos ); if ( (subStrLoc != std::string::npos) && (subStrLoc > static_cast(operandLinePos)) ) { //printf("Line:%i asmEntry DB Sym: %zi '%s'\n", line, subStrLoc, asmEntry[line]->sym.name.c_str() ); symTextStart = subStrLoc; - symTextEnd = subStrLoc + asmEntry[line]->sym.name.size(); + symTextEnd = subStrLoc + asmEntry[line]->sym.name().size(); } } @@ -5774,14 +5774,14 @@ void QAsmView::mousePressEvent(QMouseEvent * event) selAddrLine = line; selAddrChar = symTextStart; selAddrWidth = symTextEnd - symTextStart; - selAddrValue = addr = asmEntry[line]->sym.ofs; + selAddrValue = addr = asmEntry[line]->sym.offset(); selAddrType = 0; if ( selAddrWidth >= (int)sizeof(selAddrText) ) { selAddrWidth = sizeof(selAddrText)-1; } - strncpy( selAddrText, asmEntry[line]->sym.name.c_str(), selAddrWidth ); + strncpy( selAddrText, asmEntry[line]->sym.name().c_str(), selAddrWidth ); selAddrText[ selAddrWidth ] = 0; } else if ( isxdigit( asmEntry[line]->text[i] ) ) diff --git a/src/drivers/Qt/SymbolicDebug.cpp b/src/drivers/Qt/SymbolicDebug.cpp index 620db31c..0cbdcafd 100644 --- a/src/drivers/Qt/SymbolicDebug.cpp +++ b/src/drivers/Qt/SymbolicDebug.cpp @@ -62,7 +62,7 @@ debugSymbol_t *replaceSymbols( int flags, int addr, char *str ) { if ( flags & ASM_DEBUG_REPLACE ) { - strcpy( str, sym->name.c_str() ); + strcpy( str, sym->name().c_str() ); } else { @@ -74,7 +74,7 @@ debugSymbol_t *replaceSymbols( int flags, int addr, char *str ) { sprintf( str, "$%04X ", addr ); } - strcat( str, sym->name.c_str() ); + strcat( str, sym->name().c_str() ); } } else @@ -641,15 +641,15 @@ SymbolEditWindow::SymbolEditWindow(QWidget *parent) hbox->addWidget( okButton ); connect( okButton, SIGNAL(clicked(void)), this, SLOT(accept(void)) ); - connect( cancelButton, SIGNAL(clicked(void)), this, SLOT(reject(void)) ); + connect( cancelButton, SIGNAL(clicked(void)), this, SLOT(reject(void)) ); deleteBox->setEnabled( false ); okButton->setDefault(true); if ( sym != NULL ) { - nameEntry->setText( tr(sym->name.c_str()) ); - commentEntry->setPlainText( tr(sym->comment.c_str()) ); + nameEntry->setText( tr(sym->name().c_str()) ); + commentEntry->setPlainText( tr(sym->comment().c_str()) ); } setLayout( mainLayout ); @@ -667,16 +667,16 @@ SymbolEditWindow::~SymbolEditWindow(void) //-------------------------------------------------------------- void SymbolEditWindow::closeEvent(QCloseEvent *event) { - //printf("Symbolic Debug Close Window Event\n"); - done(0); + //printf("Symbolic Debug Close Window Event\n"); + done(0); deleteLater(); - event->accept(); + event->accept(); } //-------------------------------------------------------------- void SymbolEditWindow::closeWindow(void) { - //printf("Close Window\n"); - done(0); + //printf("Close Window\n"); + done(0); deleteLater(); } //-------------------------------------------------------------- @@ -765,8 +765,8 @@ void SymbolEditWindow::setSym( debugSymbol_t *symIn ) if ( sym != NULL ) { - nameEntry->setText( tr(sym->name.c_str()) ); - commentEntry->setPlainText( tr(sym->comment.c_str()) ); + nameEntry->setText( tr(sym->name().c_str()) ); + commentEntry->setPlainText( tr(sym->comment().c_str()) ); deleteBox->setEnabled( true ); determineArrayStart(); @@ -815,24 +815,22 @@ int SymbolEditWindow::exec(void) if ( deleteBox->isChecked() ) { - if ( sym != NULL ) + if ( sym != nullptr ) { debugSymbolTable.deleteSymbolAtBankOffset( b, a ); } } else { - if ( sym == NULL ) + if ( sym == nullptr ) { - sym = new debugSymbol_t(); - - sym->ofs = a; + sym = new debugSymbol_t(a); debugSymbolTable.addSymbolAtBankOffset( b, a, sym ); isNew = true; } - sym->ofs = a; + sym->setOffset(a); if ( (i == 0) || isNew || arrayNameOverWrite->isChecked() ) { @@ -842,7 +840,7 @@ int SymbolEditWindow::exec(void) { if ( isNew || arrayCommentOverWrite->isChecked() || (i == 0) ) { - sym->comment = commentEntry->toPlainText().toStdString(); + sym->commentAssign( commentEntry->toPlainText().toStdString() ); } } sym->trimTrailingSpaces(); @@ -860,17 +858,15 @@ int SymbolEditWindow::exec(void) } else if ( sym == NULL ) { - sym = new debugSymbol_t(); - sym->ofs = addr; - sym->name = nameEntry->text().toStdString(); - sym->comment = commentEntry->toPlainText().toStdString(); + sym = new debugSymbol_t( addr, nameEntry->text().toStdString().c_str(), + commentEntry->toPlainText().toStdString().c_str()); debugSymbolTable.addSymbolAtBankOffset( bank, addr, sym ); } else { - sym->name = nameEntry->text().toStdString(); - sym->comment = commentEntry->toPlainText().toStdString(); + sym->updateName( nameEntry->text().toStdString().c_str() ); + sym->commentAssign( commentEntry->toPlainText().toStdString().c_str() ); } sym->trimTrailingSpaces(); } @@ -980,13 +976,14 @@ void SymbolEditWindow::setSymNameWithArray(int idx) } // Reform with base string and new index. - sym->name.assign( stmp ); + sym->updateName( stmp, idx ); + //sym->name.assign( stmp ); - sym->trimTrailingSpaces(); + //sym->trimTrailingSpaces(); - sprintf( stmp, "[%i]", idx ); + //sprintf( stmp, "[%i]", idx ); - sym->name.append( stmp ); + //sym->name.append( stmp ); } //-------------------------------------------------------------- diff --git a/src/lua-engine.cpp b/src/lua-engine.cpp index 431b14f8..6aa16a52 100644 --- a/src/lua-engine.cpp +++ b/src/lua-engine.cpp @@ -5095,7 +5095,7 @@ static int debugger_getsymboloffset(lua_State *L) sym = debugSymbolTable.getSymbolAtAnyBank(name); } - lua_pushinteger(L, sym ? sym->ofs : -1); + lua_pushinteger(L, sym ? sym->offset() : -1); return 1; }