gbdev/rgbds
1
0
Fork 0
mirror of https://github.com/gbdev/rgbds.git synced 2025-11-20 18:22:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use std::unique_ptr for rgblink sections (#1337)

Browse Source
This commit is contained in:
Sylvie
2024-03-09 11:12:01 -05:00
committed by GitHub
parent 9890cf25b4
commit 4a7d333891
9 changed files with 102 additions and 114 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
include/link/section.hpp
View File

@@ -6,6 +6,7 @@
// GUIDELINE: external code MUST NOT BE AWARE of the data structure used! // GUIDELINE: external code MUST NOT BE AWARE of the data structure used!
#include <memory>
#include <stdint.h> #include <stdint.h>
#include <string> #include <string>
#include <vector> #include <vector>
@@ -50,7 +51,7 @@ struct Section {
// Extra info computed during linking // Extra info computed during linking
std::vector<Symbol> *fileSymbols; std::vector<Symbol> *fileSymbols;
std::vector<Symbol *> symbols; std::vector<Symbol *> symbols;
Section *nextu; // The next "component" of this unionized sect std::unique_ptr<Section> nextu; // The next "component" of this unionized sect
}; };
/* /*
@@ -64,7 +65,7 @@ void sect_ForEach(void (*callback)(Section &));
* Registers a section to be processed. * Registers a section to be processed.
* @param section The section to register. * @param section The section to register.
*/ */
void sect_AddSection(Section &section); void sect_AddSection(std::unique_ptr<Section> &&section);
/* /*
* Finds a section by its name. * Finds a section by its name.

2
src/link/assign.cpp
View File

@@ -57,7 +57,7 @@ static void initFreeSpace() {
static void assignSection(Section &section, MemoryLocation const &location) { static void assignSection(Section &section, MemoryLocation const &location) {
// Propagate the assigned location to all UNIONs/FRAGMENTs // Propagate the assigned location to all UNIONs/FRAGMENTs
// so `jr` patches in them will have the correct offset // so `jr` patches in them will have the correct offset
for (Section *next = &section; next != nullptr; next = next->nextu) { for (Section *next = &section; next != nullptr; next = next->nextu.get()) {
next->org = location.address; next->org = location.address;
next->bank = location.bank; next->bank = location.bank;
} }

17
src/link/main.cpp
View File

@@ -346,19 +346,6 @@ next:
exit(1); exit(1);
} }
static void freeSection(Section &section) {
Section *next = &section;
for (Section *nextu; next; next = nextu) {
nextu = next->nextu;
delete next;
};
}
static void freeSections() {
sect_ForEach(freeSection);
}
int main(int argc, char *argv[]) { int main(int argc, char *argv[]) {
// Parse options // Parse options
for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) { for (int ch; (ch = musl_getopt_long_only(argc, argv, optstring, longopts, nullptr)) != -1;) {
@@ -462,10 +449,6 @@ int main(int argc, char *argv[]) {
if (isDmgMode) if (isDmgMode)
sectionTypeInfo[SECTTYPE_VRAM].lastBank = 0; sectionTypeInfo[SECTTYPE_VRAM].lastBank = 0;
// Do cleanup before quitting, though.
// Mostly here to please tools such as `valgrind` so actual errors can be seen
atexit(freeSections);
// Read all object files first, // Read all object files first,
for (obj_Setup(argc - curArgIndex); curArgIndex < argc; curArgIndex++) for (obj_Setup(argc - curArgIndex); curArgIndex < argc; curArgIndex++)
obj_ReadFile(argv[curArgIndex], argc - curArgIndex - 1); obj_ReadFile(argv[curArgIndex], argc - curArgIndex - 1);

85
src/link/object.cpp
View File

@@ -7,7 +7,7 @@
#include <errno.h> #include <errno.h>
#include <inttypes.h> #include <inttypes.h>
#include <limits.h> #include <limits.h>
#include <new> #include <memory>
#include <stdint.h> #include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
@@ -329,8 +329,10 @@ static void readPatch(
* Sets a patch's pcSection from its pcSectionID. * Sets a patch's pcSection from its pcSectionID.
* @param patch The patch to fix * @param patch The patch to fix
*/ */
static void linkPatchToPCSect(Patch &patch, std::vector<Section *> const &fileSections) { static void
patch.pcSection = patch.pcSectionID != (uint32_t)-1 ? fileSections[patch.pcSectionID] : nullptr; linkPatchToPCSect(Patch &patch, std::vector<std::unique_ptr<Section>> const &fileSections) {
patch.pcSection =
patch.pcSectionID != (uint32_t)-1 ? fileSections[patch.pcSectionID].get() : nullptr;
} }
/* /*
@@ -469,10 +471,6 @@ static void readAssertion(
tryReadstring(assert.message, file, "%s: Cannot read assertion's message: %s", fileName); tryReadstring(assert.message, file, "%s: Cannot read assertion's message: %s", fileName);
} }
static Section *getMainSection(Section &section) {
return section.modifier != SECTION_NORMAL ? sect_GetSection(section.name) : &section;
}
void obj_ReadFile(char const *fileName, unsigned int fileID) { void obj_ReadFile(char const *fileName, unsigned int fileID) {
FILE *file; FILE *file;
@@ -571,47 +569,16 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
} }
// This file's sections, stored in a table to link symbols to them // This file's sections, stored in a table to link symbols to them
std::vector<Section *> fileSections(nbSections, nullptr); std::vector<std::unique_ptr<Section>> fileSections(nbSections);
verbosePrint("Reading %" PRIu32 " sections...\n", nbSections); verbosePrint("Reading %" PRIu32 " sections...\n", nbSections);
for (uint32_t i = 0; i < nbSections; i++) { for (uint32_t i = 0; i < nbSections; i++) {
// Read section // Read section
fileSections[i] = new (std::nothrow) Section(); fileSections[i] = std::make_unique<Section>();
if (!fileSections[i])
err("%s: Failed to create new section", fileName);
fileSections[i]->nextu = nullptr; fileSections[i]->nextu = nullptr;
readSection(file, *fileSections[i], fileName, nodes[fileID]); readSection(file, *fileSections[i], fileName, nodes[fileID]);
fileSections[i]->fileSymbols = &fileSymbols; fileSections[i]->fileSymbols = &fileSymbols;
fileSections[i]->symbols.reserve(nbSymPerSect[i]); fileSections[i]->symbols.reserve(nbSymPerSect[i]);
sect_AddSection(*fileSections[i]);
}
// Give patches' PC section pointers to their sections
for (uint32_t i = 0; i < nbSections; i++) {
if (sect_HasData(fileSections[i]->type)) {
for (Patch &patch : fileSections[i]->patches)
linkPatchToPCSect(patch, fileSections);
}
}
// Give symbols' section pointers to their sections
for (uint32_t i = 0; i < nbSymbols; i++) {
if (Label *label = std::get_if<Label>(&fileSymbols[i].data); label) {
Section *section = fileSections[label->sectionID];
// Give the section a pointer to the symbol as well
linkSymToSect(fileSymbols[i], *section);
if (section->modifier != SECTION_NORMAL) {
if (section->modifier == SECTION_FRAGMENT)
// Add the fragment's offset to the symbol's
label->offset += section->offset;
section = getMainSection(*section);
}
label->section = section;
}
} }
uint32_t nbAsserts; uint32_t nbAsserts;
@@ -626,6 +593,44 @@ void obj_ReadFile(char const *fileName, unsigned int fileID) {
assertion.fileSymbols = &fileSymbols; assertion.fileSymbols = &fileSymbols;
} }
// Give patches' PC section pointers to their sections
for (uint32_t i = 0; i < nbSections; i++) {
if (sect_HasData(fileSections[i]->type)) {
for (Patch &patch : fileSections[i]->patches)
linkPatchToPCSect(patch, fileSections);
}
}
// Give symbols' section pointers to their sections
for (uint32_t i = 0; i < nbSymbols; i++) {
if (Label *label = std::get_if<Label>(&fileSymbols[i].data); label) {
Section *section = fileSections[label->sectionID].get();
label->section = section;
// Give the section a pointer to the symbol as well
linkSymToSect(fileSymbols[i], *section);
}
}
// Calling `sect_AddSection` invalidates the contents of `fileSections`!
for (uint32_t i = 0; i < nbSections; i++)
sect_AddSection(std::move(fileSections[i]));
// Fix symbols' section pointers to component sections
// This has to run **after** all the `sect_AddSection()` calls,
// so that `sect_GetSection()` will work
for (uint32_t i = 0; i < nbSymbols; i++) {
if (Label *label = std::get_if<Label>(&fileSymbols[i].data); label) {
if (Section *section = label->section; section->modifier != SECTION_NORMAL) {
if (section->modifier == SECTION_FRAGMENT)
// Add the fragment's offset to the symbol's
label->offset += section->offset;
// Associate the symbol with the main section, not the "component" one
label->section = sect_GetSection(section->name);
}
}
}
fclose(file); fclose(file);
} }

6
src/link/output.cpp
View File

@@ -321,11 +321,11 @@ static void writeSymBank(SortedSections const &bankSections, SectionType type, u
for (auto it = bankSections.zeroLenSections.begin(); \ for (auto it = bankSections.zeroLenSections.begin(); \
it != bankSections.zeroLenSections.end(); \ it != bankSections.zeroLenSections.end(); \
it++) { \ it++) { \
for (Section const *sect = *it; sect; sect = sect->nextu) \ for (Section const *sect = *it; sect; sect = sect->nextu.get()) \
__VA_ARGS__ \ __VA_ARGS__ \
} \ } \
for (auto it = bankSections.sections.begin(); it != bankSections.sections.end(); it++) { \ for (auto it = bankSections.sections.begin(); it != bankSections.sections.end(); it++) { \
for (Section const *sect = *it; sect; sect = sect->nextu) \ for (Section const *sect = *it; sect; sect = sect->nextu.get()) \
__VA_ARGS__ \ __VA_ARGS__ \
} \ } \
} while (0) } while (0)
@@ -431,7 +431,7 @@ static void writeMapBank(SortedSections const &sectList, SectionType type, uint3
if (!noSymInMap) { if (!noSymInMap) {
// Also print symbols in the following "pieces" // Also print symbols in the following "pieces"
for (uint16_t org = sect->org; sect; sect = sect->nextu) { for (uint16_t org = sect->org; sect; sect = sect->nextu.get()) {
for (Symbol *sym : sect->symbols) for (Symbol *sym : sect->symbols)
// Space matches "\tSECTION: $xxxx ..." // Space matches "\tSECTION: $xxxx ..."
fprintf( fprintf(

2
src/link/patch.cpp
View File

@@ -519,7 +519,7 @@ static void applyPatches(Section &section) {
if (!sect_HasData(section.type)) if (!sect_HasData(section.type))
return; return;
for (Section *component = &section; component; component = component->nextu) for (Section *component = &section; component; component = component->nextu.get())
applyFilePatches(*component, section); applyFilePatches(*component, section);
} }

2
src/link/script.y
View File

@@ -579,7 +579,7 @@ static void placeSection(std::string const &name, bool isOptional) {
// Check that the linker script doesn't contradict what the code says. // Check that the linker script doesn't contradict what the code says.
if (section->type == SECTTYPE_INVALID) { if (section->type == SECTTYPE_INVALID) {
// SDCC areas don't have a type assigned yet, so the linker script is used to give them one. // SDCC areas don't have a type assigned yet, so the linker script is used to give them one.
for (Section *fragment = section; fragment; fragment = fragment->nextu) { for (Section *fragment = section; fragment; fragment = fragment->nextu.get()) {
fragment->type = activeType; fragment->type = activeType;
} }
} else if (section->type != activeType) { } else if (section->type != activeType) {

24
src/link/sdas_obj.cpp
View File

@@ -6,7 +6,7 @@
#include <ctype.h> #include <ctype.h>
#include <errno.h> #include <errno.h>
#include <inttypes.h> #include <inttypes.h>
#include <new> #include <memory>
#include <stdint.h> #include <stdint.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
@@ -236,7 +236,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
// Now, let's parse the rest of the lines as they come! // Now, let's parse the rest of the lines as they come!
struct FileSection { struct FileSection {
Section *section; std::unique_ptr<Section> section;
uint16_t writeIndex; uint16_t writeIndex;
}; };
std::vector<FileSection> fileSections; std::vector<FileSection> fileSections;
@@ -257,10 +257,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
warning( warning(
&where, lineNo, "Got more 'A' lines than the expected %" PRIu32, expectedNbAreas &where, lineNo, "Got more 'A' lines than the expected %" PRIu32, expectedNbAreas
); );
Section *curSection = new (std::nothrow) Section(); std::unique_ptr<Section> curSection = std::make_unique<Section>();
if (!curSection)
fatal(&where, lineNo, "Failed to alloc new area: %s", strerror(errno));
getToken(line.data(), "'A' line is too short"); getToken(line.data(), "'A' line is too short");
assert(strlen(token) != 0); // This should be impossible, tokens are non-empty assert(strlen(token) != 0); // This should be impossible, tokens are non-empty
@@ -271,8 +268,6 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
} }
char const *sectionName = token; // We'll deal with the section's name depending on type char const *sectionName = token; // We'll deal with the section's name depending on type
fileSections.push_back({.section = curSection, .writeIndex = 0});
expectToken("size", 'A'); expectToken("size", 'A');
getToken(nullptr, "'A' line is too short"); getToken(nullptr, "'A' line is too short");
@@ -345,6 +340,8 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
curSection->isAlignFixed = false; // No such concept! curSection->isAlignFixed = false; // No such concept!
curSection->fileSymbols = &fileSymbols; // IDs are instead per-section curSection->fileSymbols = &fileSymbols; // IDs are instead per-section
curSection->nextu = nullptr; curSection->nextu = nullptr;
fileSections.push_back({.section = std::move(curSection), .writeIndex = 0});
break; break;
} }
@@ -371,7 +368,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
if (int32_t value = parseNumber(where, lineNo, &token[3], numberType); if (int32_t value = parseNumber(where, lineNo, &token[3], numberType);
!fileSections.empty()) { !fileSections.empty()) {
// Symbols in sections are labels; their value is an offset // Symbols in sections are labels; their value is an offset
Section *section = fileSections.back().section; Section *section = fileSections.back().section.get();
if (section->isAddressFixed) { if (section->isAddressFixed) {
assert(value >= section->org && value <= section->org + section->size); assert(value >= section->org && value <= section->org + section->size);
value -= section->org; value -= section->org;
@@ -479,7 +476,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
fileSections.size() fileSections.size()
); );
assert(!fileSections.empty()); // There should be at least one, from the above check assert(!fileSections.empty()); // There should be at least one, from the above check
Section *section = fileSections[areaIdx].section; Section *section = fileSections[areaIdx].section.get();
uint16_t *writeIndex = &fileSections[areaIdx].writeIndex; uint16_t *writeIndex = &fileSections[areaIdx].writeIndex;
uint8_t writtenOfs = ADDR_SIZE; // Bytes before this have been written to `->data` uint8_t writtenOfs = ADDR_SIZE; // Bytes before this have been written to `->data`
uint16_t addr = data[0] | data[1] << 8; uint16_t addr = data[0] | data[1] << 8;
@@ -826,7 +823,7 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
nbSectionsToAssign += fileSections.size(); nbSectionsToAssign += fileSections.size();
for (FileSection &entry : fileSections) { for (FileSection &entry : fileSections) {
Section *section = entry.section; std::unique_ptr<Section> &section = entry.section;
// RAM sections can have a size, but don't get any data (they shouldn't have any) // RAM sections can have a size, but don't get any data (they shouldn't have any)
if (entry.writeIndex != section->size && entry.writeIndex != 0) if (entry.writeIndex != section->size && entry.writeIndex != 0)
@@ -839,13 +836,14 @@ void sdobj_ReadFile(FileStackNode const &where, FILE *file, std::vector<Symbol>
section->size section->size
); );
sect_AddSection(*section);
if (section->modifier == SECTION_FRAGMENT) { if (section->modifier == SECTION_FRAGMENT) {
// Add the fragment's offset to all of its symbols // Add the fragment's offset to all of its symbols
for (Symbol *symbol : section->symbols) for (Symbol *symbol : section->symbols)
symbol->label().offset += section->offset; symbol->label().offset += section->offset;
} }
// Calling `sect_AddSection` invalidates the contents of `fileSections`!
sect_AddSection(std::move(section));
} }
#undef expectEol #undef expectEol

73
src/link/section.cpp
View File

@@ -5,6 +5,7 @@
#include <assert.h> #include <assert.h>
#include <inttypes.h> #include <inttypes.h>
#include <map> #include <map>
#include <memory>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
#include <string> #include <string>
@@ -14,7 +15,7 @@
#include "link/main.hpp" #include "link/main.hpp"
std::map<std::string, Section *> sections; std::map<std::string, std::unique_ptr<Section>> sections;
void sect_ForEach(void (*callback)(Section &)) { void sect_ForEach(void (*callback)(Section &)) {
for (auto &it : sections) for (auto &it : sections)
@@ -139,53 +140,53 @@ static void checkFragmentCompat(Section &target, Section &other) {
} }
} }
static void mergeSections(Section &target, Section &other, SectionModifier mod) { static void mergeSections(Section &target, std::unique_ptr<Section> &&other, SectionModifier mod) {
// Common checks // Common checks
if (target.type != other.type) if (target.type != other->type)
errx( errx(
"Section \"%s\" is defined with conflicting types %s and %s", "Section \"%s\" is defined with conflicting types %s and %s",
other.name.c_str(), other->name.c_str(),
sectionTypeInfo[target.type].name.c_str(), sectionTypeInfo[target.type].name.c_str(),
sectionTypeInfo[other.type].name.c_str() sectionTypeInfo[other->type].name.c_str()
); );
if (other.isBankFixed) { if (other->isBankFixed) {
if (!target.isBankFixed) { if (!target.isBankFixed) {
target.isBankFixed = true; target.isBankFixed = true;
target.bank = other.bank; target.bank = other->bank;
} else if (target.bank != other.bank) { } else if (target.bank != other->bank) {
errx( errx(
"Section \"%s\" is defined with conflicting banks %" PRIu32 " and %" PRIu32, "Section \"%s\" is defined with conflicting banks %" PRIu32 " and %" PRIu32,
other.name.c_str(), other->name.c_str(),
target.bank, target.bank,
other.bank other->bank
); );
} }
} }
switch (mod) { switch (mod) {
case SECTION_UNION: case SECTION_UNION:
checkSectUnionCompat(target, other); checkSectUnionCompat(target, *other);
if (other.size > target.size) if (other->size > target.size)
target.size = other.size; target.size = other->size;
break; break;
case SECTION_FRAGMENT: case SECTION_FRAGMENT:
checkFragmentCompat(target, other); checkFragmentCompat(target, *other);
// Append `other` to `target` // Append `other` to `target`
// Note that the order in which fragments are stored in the `nextu` list does not // Note that the order in which fragments are stored in the `nextu` list does not
// really matter, only that offsets are properly computed // really matter, only that offsets are properly computed
other.offset = target.size; other->offset = target.size;
target.size += other.size; target.size += other->size;
// Normally we'd check that `sect_HasData`, but SDCC areas may be `_INVALID` here // Normally we'd check that `sect_HasData`, but SDCC areas may be `_INVALID` here
if (!other.data.empty()) { if (!other->data.empty()) {
target.data.insert(target.data.end(), RANGE(other.data)); target.data.insert(target.data.end(), RANGE(other->data));
// Adjust patches' PC offsets // Adjust patches' PC offsets
for (Patch &patch : other.patches) for (Patch &patch : other->patches)
patch.pcOffset += other.offset; patch.pcOffset += other->offset;
} else if (!target.data.empty()) { } else if (!target.data.empty()) {
assert(other.size == 0); assert(other->size == 0);
} }
break; break;
@@ -193,39 +194,39 @@ static void mergeSections(Section &target, Section &other, SectionModifier mod)
unreachable_(); unreachable_();
} }
other.nextu = target.nextu; other->nextu = std::move(target.nextu);
target.nextu = &other; target.nextu = std::move(other);
} }
void sect_AddSection(Section &section) { void sect_AddSection(std::unique_ptr<Section> &&section) {
// Check if the section already exists // Check if the section already exists
if (Section *other = sect_GetSection(section.name); other) { if (Section *other = sect_GetSection(section->name); other) {
if (section.modifier != other->modifier) if (section->modifier != other->modifier)
errx( errx(
"Section \"%s\" defined as %s and %s", "Section \"%s\" defined as %s and %s",
section.name.c_str(), section->name.c_str(),
sectionModNames[section.modifier], sectionModNames[section->modifier],
sectionModNames[other->modifier] sectionModNames[other->modifier]
); );
else if (section.modifier == SECTION_NORMAL) else if (section->modifier == SECTION_NORMAL)
errx("Section name \"%s\" is already in use", section.name.c_str()); errx("Section name \"%s\" is already in use", section->name.c_str());
else else
mergeSections(*other, section, section.modifier); mergeSections(*other, std::move(section), section->modifier);
} else if (section.modifier == SECTION_UNION && sect_HasData(section.type)) { } else if (section->modifier == SECTION_UNION && sect_HasData(section->type)) {
errx( errx(
"Section \"%s\" is of type %s, which cannot be unionized", "Section \"%s\" is of type %s, which cannot be unionized",
section.name.c_str(), section->name.c_str(),
sectionTypeInfo[section.type].name.c_str() sectionTypeInfo[section->type].name.c_str()
); );
} else { } else {
// If not, add it // If not, add it
sections[section.name] = &section; sections.emplace(section->name, std::move(section));
} }
} }
Section *sect_GetSection(std::string const &name) { Section *sect_GetSection(std::string const &name) {
auto search = sections.find(name); auto search = sections.find(name);
return search != sections.end() ? search->second : nullptr; return search != sections.end() ? search->second.get() : nullptr;
} }
static void doSanityChecks(Section &section) { static void doSanityChecks(Section &section) {