From 75f1bcde31d8ac8a865a14f02d5df15c6321ffea Mon Sep 17 00:00:00 2001 From: ISSOtm Date: Mon, 3 May 2021 12:09:12 +0200 Subject: [PATCH] Make SECTION size overflow non-fatal Fixes #538 --- include/helpers.h | 2 + src/asm/section.c | 91 ++++++++++++++++++++++++-------------- test/asm/load-overflow.asm | 20 +++++++++ test/asm/load-overflow.err | 11 ++++- 4 files changed, 90 insertions(+), 34 deletions(-) diff --git a/include/helpers.h b/include/helpers.h index 726be0a9..3dd17884 100644 --- a/include/helpers.h +++ b/include/helpers.h @@ -18,6 +18,7 @@ #ifdef __GNUC__ // GCC or compatible #define format_(archetype, str_index, first_arg) \ __attribute__ ((format (archetype, str_index, first_arg))) + #define attr_(...) __attribute__ ((__VA_ARGS__)) // In release builds, define "unreachable" as such, but trap in debug builds #ifdef NDEBUG #define unreachable_ __builtin_unreachable @@ -27,6 +28,7 @@ #else // Unsupported, but no need to throw a fit #define format_(archetype, str_index, first_arg) + #define attr_(...) // This seems to generate similar code to __builtin_unreachable, despite different semantics // Note that executing this is undefined behavior (declared _Noreturn, but does return) static inline _Noreturn unreachable_(void) {} diff --git a/src/asm/section.c b/src/asm/section.c index bf7e25e0..c1656816 100644 --- a/src/asm/section.c +++ b/src/asm/section.c @@ -44,7 +44,7 @@ struct UnionStackEntry { /* * A quick check to see if we have an initialized section */ -static bool checksection(void) +attr_(warn_unused_result) static bool checksection(void) { if (currentSection) return true; @@ -57,7 +57,7 @@ static bool checksection(void) * A quick check to see if we have an initialized section that can contain * this much initialized data */ -static bool checkcodesection(void) +attr_(warn_unused_result) static bool checkcodesection(void) { if (!checksection()) return false; @@ -70,29 +70,43 @@ static bool checkcodesection(void) return false; } -static void checkSectionSize(struct Section const *sect, uint32_t size) +attr_(warn_unused_result) static bool checkSectionSize(struct Section const *sect, uint32_t size) { uint32_t maxSize = maxsize[sect->type]; - if (size > maxSize) - fatalerror("Section '%s' grew too big (max size = 0x%" PRIX32 - " bytes, reached 0x%" PRIX32 ").\n", sect->name, maxSize, size); + // If the new size is reasonable, keep going + if (size <= maxSize) + return true; + + error("Section '%s' grew too big (max size = 0x%" PRIX32 + " bytes, reached 0x%" PRIX32 ").\n", sect->name, maxSize, size); + return false; } /* * Check if the section has grown too much. */ -static void reserveSpace(uint32_t delta_size) +attr_(warn_unused_result) static bool reserveSpace(uint32_t delta_size) { /* - * This check is here to trap broken code that generates sections that - * are too big and to prevent the assembler from generating huge object - * files or trying to allocate too much memory. + * This check is here to trap broken code that generates sections that are too big and to + * prevent the assembler from generating huge object files or trying to allocate too much + * memory. * A check at the linking stage is still necessary. */ - checkSectionSize(currentSection, curOffset + loadOffset + delta_size); - if (currentLoadSection) - checkSectionSize(currentLoadSection, curOffset + delta_size); + + // If the section has already overflowed, skip the check to avoid erroring out ad nauseam + if (currentSection->size != UINT32_MAX + && !checkSectionSize(currentSection, curOffset + loadOffset + delta_size)) + // Mark the section as overflowed, to avoid repeating the error + currentSection->size = UINT32_MAX; + + if (currentLoadSection && currentLoadSection->size != UINT32_MAX + && !checkSectionSize(currentLoadSection, curOffset + delta_size)) + currentLoadSection->size = UINT32_MAX; + + return currentSection->size != UINT32_MAX + && (!currentLoadSection || currentLoadSection->size != UINT32_MAX); } struct Section *sect_FindSectionByName(const char *name) @@ -596,7 +610,8 @@ void sect_AbsByte(uint8_t b) { if (!checkcodesection()) return; - reserveSpace(1); + if (!reserveSpace(1)) + return; writebyte(b); } @@ -605,7 +620,8 @@ void sect_AbsByteGroup(uint8_t const *s, int32_t length) { if (!checkcodesection()) return; - reserveSpace(length); + if (!reserveSpace(length)) + return; while (length--) writebyte(*s++); @@ -615,7 +631,8 @@ void sect_AbsWordGroup(uint8_t const *s, int32_t length) { if (!checkcodesection()) return; - reserveSpace(length * 2); + if (!reserveSpace(length * 2)) + return; while (length--) writeword(*s++); @@ -625,7 +642,8 @@ void sect_AbsLongGroup(uint8_t const *s, int32_t length) { if (!checkcodesection()) return; - reserveSpace(length * 4); + if (!reserveSpace(length * 4)) + return; while (length--) writelong(*s++); @@ -638,17 +656,16 @@ void sect_Skip(int32_t skip, bool ds) { if (!checksection()) return; - reserveSpace(skip); - - if (!ds && sect_HasData(currentSection->type)) - warning(WARNING_EMPTY_DATA_DIRECTIVE, "%s directive without data in ROM\n", - (skip == 4) ? "DL" : (skip == 2) ? "DW" : "DB"); + if (!reserveSpace(skip)) + return; if (!sect_HasData(currentSection->type)) { growSection(skip); } else { - if (!checkcodesection()) - return; + if (!ds) + warning(WARNING_EMPTY_DATA_DIRECTIVE, "%s directive without data in ROM\n", + (skip == 4) ? "DL" : (skip == 2) ? "DW" : "DB"); + // We know we're in a code SECTION while (skip--) writebyte(fillByte); } @@ -661,7 +678,8 @@ void sect_String(char const *s) { if (!checkcodesection()) return; - reserveSpace(strlen(s)); + if (!reserveSpace(strlen(s))) + return; while (*s) writebyte(*s++); @@ -675,7 +693,8 @@ void sect_RelByte(struct Expression *expr, uint32_t pcShift) { if (!checkcodesection()) return; - reserveSpace(1); + if (!reserveSpace(1)) + return; if (!rpn_isKnown(expr)) { createPatch(PATCHTYPE_BYTE, expr, pcShift); @@ -694,7 +713,8 @@ void sect_RelBytes(uint32_t n, struct Expression *exprs, size_t size) { if (!checkcodesection()) return; - reserveSpace(n); + if (!reserveSpace(n)) + return; for (uint32_t i = 0; i < n; i++) { struct Expression *expr = &exprs[i % size]; @@ -719,7 +739,8 @@ void sect_RelWord(struct Expression *expr, uint32_t pcShift) { if (!checkcodesection()) return; - reserveSpace(2); + if (!reserveSpace(2)) + return; if (!rpn_isKnown(expr)) { createPatch(PATCHTYPE_WORD, expr, pcShift); @@ -738,7 +759,8 @@ void sect_RelLong(struct Expression *expr, uint32_t pcShift) { if (!checkcodesection()) return; - reserveSpace(2); + if (!reserveSpace(2)) + return; if (!rpn_isKnown(expr)) { createPatch(PATCHTYPE_LONG, expr, pcShift); @@ -757,7 +779,8 @@ void sect_PCRelByte(struct Expression *expr, uint32_t pcShift) { if (!checkcodesection()) return; - reserveSpace(1); + if (!reserveSpace(1)) + return; struct Symbol const *pc = sym_GetPC(); if (!rpn_IsDiffConstant(expr, pc)) { @@ -828,11 +851,12 @@ void sect_BinaryFile(char const *s, int32_t startPos) } fseek(f, startPos, SEEK_SET); - reserveSpace(fsize - startPos); + if (!reserveSpace(fsize - startPos)) + goto cleanup; } else { if (errno != ESPIPE) error("Error determining size of INCBIN file '%s': %s\n", - s, strerror(errno)); + s, strerror(errno)); /* The file isn't seekable, so we'll just skip bytes */ while (startPos--) (void)fgetc(f); @@ -867,7 +891,8 @@ void sect_BinaryFileSlice(char const *s, int32_t start_pos, int32_t length) return; if (length == 0) /* Don't even bother with 0-byte slices */ return; - reserveSpace(length); + if (!reserveSpace(length)) + return; char *fullPath = NULL; size_t size = 0; diff --git a/test/asm/load-overflow.asm b/test/asm/load-overflow.asm index 10a75adc..c3fe7f11 100644 --- a/test/asm/load-overflow.asm +++ b/test/asm/load-overflow.asm @@ -1,5 +1,25 @@ SECTION "Overflow", ROM0 ds $6000 LOAD "oops",WRAM0 + ; We might get an error for "oops", but it can also make sense to no-op the directive ds $2001 + ; We shouldn't get any more errors for "Overflow" nor "oops" + db ENDL + +SECTION "Moar overflow", ROM0 + ds $6001 +LOAD "hmm", WRAM0 + ds $2000 + ; Since the `ds` overflows "Moar overflow", it could be no-op'd, making this `db` not error + db +ENDL + +SECTION "Not overflowing", ROM0 + ds $5FFF +LOAD "lol", WRAM0 + ds $2000 + db + ; Since the LOAD block is overflowed, this may be no-op'd, not affecting the "parent" +ENDL + dw ; This, however... diff --git a/test/asm/load-overflow.err b/test/asm/load-overflow.err index 68bcfc5a..05e43a6c 100644 --- a/test/asm/load-overflow.err +++ b/test/asm/load-overflow.err @@ -1,2 +1,11 @@ -FATAL: load-overflow.asm(4): +ERROR: load-overflow.asm(5): Section 'Overflow' grew too big (max size = 0x8000 bytes, reached 0x8001). +ERROR: load-overflow.asm(5): + Section 'oops' grew too big (max size = 0x2000 bytes, reached 0x2001). +ERROR: load-overflow.asm(13): + Section 'Moar overflow' grew too big (max size = 0x8000 bytes, reached 0x8001). +ERROR: load-overflow.asm(22): + Section 'lol' grew too big (max size = 0x2000 bytes, reached 0x2001). +ERROR: load-overflow.asm(25): + Section 'Not overflowing' grew too big (max size = 0x8000 bytes, reached 0x8001). +error: Assembly aborted (5 errors)!