From f47ce337bf51a8278db47d7698281cb91d17f797 Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Tue, 27 Feb 2024 11:28:12 -0500 Subject: [PATCH] Use `std::vector` for reading object file symbols --- include/link/patch.hpp | 3 ++- include/link/sdas_obj.hpp | 4 +++- include/link/section.hpp | 2 +- src/link/object.cpp | 31 ++++++++++--------------------- src/link/patch.cpp | 14 +++++--------- src/link/sdas_obj.cpp | 35 ++++++++++------------------------- 6 files changed, 31 insertions(+), 58 deletions(-) diff --git a/include/link/patch.hpp b/include/link/patch.hpp index 1b197d0d..7dafd639 100644 --- a/include/link/patch.hpp +++ b/include/link/patch.hpp @@ -6,6 +6,7 @@ #include #include +#include #include "link/section.hpp" @@ -15,7 +16,7 @@ struct Assertion { struct Patch patch; // Also used for its `.type` char *message; // This would be redundant with `.section->fileSymbols`... but `section` is sometimes NULL! - struct Symbol **fileSymbols; + std::vector *fileSymbols; }; /* diff --git a/include/link/sdas_obj.hpp b/include/link/sdas_obj.hpp index cd096bf5..9f25809e 100644 --- a/include/link/sdas_obj.hpp +++ b/include/link/sdas_obj.hpp @@ -5,9 +5,11 @@ #define RGBDS_LINK_SDAS_OBJ_H #include +#include struct FileStackNode; +struct Symbol; -void sdobj_ReadFile(struct FileStackNode const *fileName, FILE *file); +void sdobj_ReadFile(struct FileStackNode const *fileName, FILE *file, std::vector &fileSymbols); #endif // RGBDS_LINK_SDAS_OBJ_H diff --git a/include/link/section.hpp b/include/link/section.hpp index 41b7200a..3ea3af26 100644 --- a/include/link/section.hpp +++ b/include/link/section.hpp @@ -47,7 +47,7 @@ struct Section { uint8_t *data; // Array of size `size` std::vector *patches; // Extra info computed during linking - struct Symbol **fileSymbols; + std::vector *fileSymbols; uint32_t nbSymbols; struct Symbol **symbols; struct Section *nextu; // The next "component" of this unionized sect diff --git a/src/link/object.cpp b/src/link/object.cpp index ec4bc247..e5d41449 100644 --- a/src/link/object.cpp +++ b/src/link/object.cpp @@ -24,13 +24,7 @@ #include "linkdefs.hpp" #include "version.hpp" -struct SymbolList { - size_t nbSymbols; - struct Symbol **symbolList; -}; - -static std::deque symbolLists; - +static std::deque> symbolLists; static std::vector> nodes; static std::deque assertions; @@ -478,7 +472,9 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) if (!where.name) fatal(NULL, 0, "Failed to duplicate \"%s\"'s name: %s", fileName, strerror(errno)); - sdobj_ReadFile(&where, file); + std::vector &fileSymbols = symbolLists.emplace_front(); + + sdobj_ReadFile(&where, file, fileSymbols); return; } @@ -516,13 +512,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) readFileStackNode(file, nodes[fileID], i, fileName); // This file's symbols, kept to link sections to them - struct Symbol **fileSymbols = (struct Symbol **)malloc(sizeof(*fileSymbols) * nbSymbols + 1); - - if (!fileSymbols) - err("Failed to get memory for %s's symbols", fileName); - - symbolLists.push_front({ .nbSymbols = nbSymbols, .symbolList = fileSymbols }); - + std::vector &fileSymbols = symbolLists.emplace_front(nbSymbols); std::vector nbSymPerSect(nbSections, 0); verbosePrint("Reading %" PRIu32 " symbols...\n", nbSymbols); @@ -553,7 +543,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) fileSections[i]->nextu = NULL; readSection(file, fileSections[i], fileName, nodes[fileID]); - fileSections[i]->fileSymbols = fileSymbols; + fileSections[i]->fileSymbols = &fileSymbols; if (nbSymPerSect[i]) { fileSections[i]->symbols = (struct Symbol **)malloc(nbSymPerSect[i] * sizeof(*fileSections[i]->symbols)); @@ -606,7 +596,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) readAssertion(file, &assertion, fileName, i, nodes[fileID]); linkPatchToPCSect(&assertion.patch, fileSections); - assertion.fileSymbols = fileSymbols; + assertion.fileSymbols = &fileSymbols; } fclose(file); @@ -670,10 +660,9 @@ void obj_Cleanup(void) sect_ForEach(freeSection); sect_CleanupSections(); - for (struct SymbolList &list : symbolLists) { - for (size_t i = 0; i < list.nbSymbols; i++) - freeSymbol(list.symbolList[i]); - free(list.symbolList); + for (std::vector &fileSymbols : symbolLists) { + for (struct Symbol *symbol : fileSymbols) + freeSymbol(symbol); } for (struct Assertion &assert : assertions) diff --git a/src/link/patch.cpp b/src/link/patch.cpp index 6107fd60..0e466feb 100644 --- a/src/link/patch.cpp +++ b/src/link/patch.cpp @@ -56,11 +56,10 @@ static uint32_t getRPNByte(uint8_t const **expression, int32_t *size, return *(*expression)++; } -static struct Symbol const *getSymbol(struct Symbol const * const *symbolList, - uint32_t index) +static struct Symbol const *getSymbol(std::vector const &symbolList, uint32_t index) { assert(index != (uint32_t)-1); // PC needs to be handled specially, not here - struct Symbol const *symbol = symbolList[index]; + struct Symbol *symbol = symbolList[index]; // If the symbol is defined elsewhere... if (symbol->type == SYMTYPE_IMPORT) @@ -78,7 +77,7 @@ static struct Symbol const *getSymbol(struct Symbol const * const *symbolList, * errors caused by the value should be suppressed. */ static int32_t computeRPNExpr(struct Patch const *patch, - struct Symbol const * const *fileSymbols) + std::vector const &fileSymbols) { // Small shortcut to avoid a lot of repetition #define popRPN() popRPN(patch->src, patch->lineNo) @@ -416,8 +415,7 @@ void patch_CheckAssertions(std::deque &assertions) verbosePrint("Checking assertions...\n"); for (struct Assertion &assert : assertions) { - int32_t value = computeRPNExpr(&assert.patch, - (struct Symbol const * const *)assert.fileSymbols); + int32_t value = computeRPNExpr(&assert.patch, *assert.fileSymbols); enum AssertionType type = (enum AssertionType)assert.patch.type; if (!isError && !value) { @@ -455,9 +453,7 @@ static void applyFilePatches(struct Section *section, struct Section *dataSectio { verbosePrint("Patching section \"%s\"...\n", section->name); for (struct Patch &patch : *section->patches) { - int32_t value = computeRPNExpr(&patch, - (struct Symbol const * const *) - section->fileSymbols); + int32_t value = computeRPNExpr(&patch, *section->fileSymbols); uint16_t offset = patch.offset + section->offset; // `jr` is quite unlike the others... diff --git a/src/link/sdas_obj.cpp b/src/link/sdas_obj.cpp index ccd2352c..18c09014 100644 --- a/src/link/sdas_obj.cpp +++ b/src/link/sdas_obj.cpp @@ -144,7 +144,7 @@ enum RelocFlags { | 1 << RELOC_EXPR24 | 1 << RELOC_BANKBYTE, }; -void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) { +void sdobj_ReadFile(struct FileStackNode const *where, FILE *file, std::vector &fileSymbols) { size_t bufLen = 256; char *line = (char *)malloc(bufLen); char const *token; @@ -234,11 +234,7 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) { uint16_t writeIndex; }; std::vector fileSections; - struct Symbol **fileSymbols = (struct Symbol **)malloc(sizeof(*fileSymbols) * expectedNbSymbols); size_t nbSections = 0, nbSymbols = 0; - - if (!fileSymbols) - fatal(where, lineNo, "Failed to alloc file symbols table: %s", strerror(errno)); size_t nbBytes = 0; // How many bytes are in `data`, including the ADDR_SIZE "header" bytes size_t dataCapacity = 16 + ADDR_SIZE; // SDCC object files usually contain 16 bytes per T line uint8_t *data = (uint8_t *)malloc(sizeof(*data) * dataCapacity); @@ -262,10 +258,11 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) { expectedNbAreas); fileSections.resize(nbSections + 1); fileSections[nbSections].writeIndex = 0; -#define curSection (fileSections[nbSections].section) - curSection = (struct Section *)malloc(sizeof(*curSection)); + struct Section *curSection = (struct Section *)malloc(sizeof(*curSection)); + if (!curSection) fatal(where, lineNo, "Failed to alloc new area: %s", strerror(errno)); + fileSections[nbSections].section = curSection; getToken(line, "'A' line is too short"); assert(strlen(token) != 0); // This should be impossible, tokens are non-empty @@ -357,36 +354,24 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) { if (!curSection->patches) fatal(where, lineNo, "Failed to alloc new area's patches: %s", strerror(errno)); - curSection->fileSymbols = fileSymbols; // IDs are instead per-section + curSection->fileSymbols = &fileSymbols; // IDs are instead per-section curSection->nbSymbols = 0; curSection->symbols = NULL; // Will be allocated on demand as well curSection->nextu = NULL; -#undef curSection + ++nbSections; break; } - case 'S': + case 'S': { if (nbSymbols == expectedNbSymbols) warning(where, lineNo, "Got more 'S' lines than the expected %" PRIu32, expectedNbSymbols); - // `realloc` is dangerous, as sections contain a pointer to `fileSymbols`. - // We can try to be nice, but if the pointer moves, it's game over! - if (nbSymbols >= expectedNbSymbols) { - struct Symbol **newFileSymbols = (struct Symbol **)realloc(fileSymbols, - sizeof(*fileSymbols) * (nbSymbols + 1)); + struct Symbol *symbol = (struct Symbol *)malloc(sizeof(*symbol)); - if (!newFileSymbols) - fatal(where, lineNo, "Failed to alloc extra symbols: %s", - strerror(errno)); - if (newFileSymbols != fileSymbols) - fatal(where, lineNo, "Failed to handle extra 'S' lines (pointer moved)"); - // No need to assign, obviously - } -#define symbol (fileSymbols[nbSymbols]) - symbol = (struct Symbol *)malloc(sizeof(*symbol)); if (!symbol) fatal(where, lineNo, "Failed to alloc symbol: %s", strerror(errno)); + fileSymbols.push_back(symbol); // Init other members symbol->objFileName = where->name; @@ -453,12 +438,12 @@ void sdobj_ReadFile(struct FileStackNode const *where, FILE *file) { section->name, strerror(errno)); section->symbols[section->nbSymbols - 1] = symbol; } -#undef symbol expectEol("'S' line is too long"); ++nbSymbols; break; + } case 'T': // Now, time to parse the data!