From 57aeb7e0560a3028a2420cc32242ae757b1e031e Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Fri, 1 May 2026 13:04:34 +0200 Subject: [PATCH 1/2] Refactor section creation errors, and simplify output for single errors --- include/asm/warning.hpp | 6 +- src/asm/section.cpp | 127 ++++++++++++------ src/asm/warning.cpp | 10 ++ test/asm/duplicate-section-after-literal.err | 4 +- test/asm/duplicate-section.err | 4 +- test/asm/fragment-align-mismatch.err | 4 +- test/asm/fragment-mismatch.err | 4 +- test/asm/incompatible-alignment.err | 4 +- test/asm/multiple-section-errors.asm | 3 + test/asm/multiple-section-errors.err | 6 + test/asm/section-union-mismatch.err | 4 +- test/asm/section-union.err | 4 +- test/asm/union-mismatch.err | 4 +- test/link/section-fragment/align-conflict.out | 4 +- test/link/section-union/align-conflict.out | 4 +- .../link/section-union/align-ofs-conflict.out | 4 +- test/link/section-union/bad-types.out | 4 +- test/link/section-union/bank-conflict.out | 4 +- test/link/section-union/different-ofs.out | 4 +- test/link/section-union/org-conflict.out | 4 +- 20 files changed, 125 insertions(+), 87 deletions(-) create mode 100644 test/asm/multiple-section-errors.asm create mode 100644 test/asm/multiple-section-errors.err diff --git a/include/asm/warning.hpp b/include/asm/warning.hpp index f2f37c730..1a09121b9 100644 --- a/include/asm/warning.hpp +++ b/include/asm/warning.hpp @@ -61,13 +61,17 @@ extern Diagnostics warnings; void warning(WarningID id, char const *fmt, ...); // Used for errors that compromise the whole assembly process by affecting the -// following code, potencially making the assembler generate errors caused by +// following code, potentially making the assembler generate errors caused by // the first one and unrelated to the code that the assembler complains about. // It is also used when the assembler goes into an invalid state (for example, // when it fails to allocate memory). [[gnu::format(printf, 1, 2), noreturn]] void fatal(char const *fmt, ...); +// Used for fatal errors that handle their own backtrace output. +[[noreturn]] +void fatalNoTrace(std::function callback); + // Used for errors that make it impossible to assemble correctly, but don't // affect the following code. The code will fail to assemble but the user will // get a list of all errors at the end, making it easier to fix all of them at diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 24dd943d1..33b060176 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -114,16 +115,29 @@ Section *sect_FindSectionByName(std::string const &name) { return index ? §ions[*index] : nullptr; } -#define sectError(...) \ - do { \ - error(__VA_ARGS__); \ - ++nbSectErrors; \ - } while (0) +static std::vector sectErrors; -static unsigned int - mergeSectUnion(Section §, uint32_t org, uint8_t alignment, uint16_t alignOffset) { - unsigned int nbSectErrors = 0; +void sectError(char const *fmt, ...) { + std::string result; + va_list args1, args2; + va_start(args1, fmt); + va_copy(args2, args1); + int len = vsnprintf(nullptr, 0, fmt, args1); + va_end(args1); + if (len < 0) { + // LCOV_EXCL_START + va_end(args2); + fatal("Error describing the error that occurred when merging a section"); + // LCOV_EXCL_STOP + } else if (len > 0) { + result.resize(len); + vsnprintf(result.data(), len + 1, fmt, args2); + } + va_end(args2); + sectErrors.push_back(result); +} +static void mergeSectUnion(Section §, uint32_t org, uint8_t alignment, uint16_t alignOffset) { assume(alignment < 16); // Should be ensured by the caller uint32_t alignSize = 1u << alignment; uint32_t alignMask = alignSize - 1; @@ -136,11 +150,15 @@ static unsigned int // If both are fixed, they must be the same if (sect.org != UINT32_MAX && sect.org != org) { sectError( - "Section already declared as fixed at different address $%04" PRIx32, sect.org + "Section \"%s\" already declared as fixed at different address $%04" PRIx32, + sect.name.c_str(), + sect.org ); } else if (sect.align != 0 && ((org - sect.alignOfs) & sectAlignMask)) { sectError( - "Section already declared as aligned to %" PRIu32 " bytes (offset %" PRIu16 ")", + "Section \"%s\" already declared as aligned to %" PRIu32 " bytes (offset %" PRIu16 + ")", + sect.name.c_str(), sectAlignSize, sect.alignOfs ); @@ -154,15 +172,17 @@ static unsigned int if (sect.org != UINT32_MAX) { if ((sect.org - alignOffset) & alignMask) { sectError( - "Section already declared as fixed at incompatible address $%04" PRIx32, + "Section \"%s\" already declared as fixed at incompatible address $%04" PRIx32, + sect.name.c_str(), sect.org ); } // Check if alignment offsets are compatible } else if ((alignOffset & sectAlignMask) != (sect.alignOfs & alignMask)) { sectError( - "Section already declared with incompatible %" PRIu32 + "Section \"%s\" already declared with incompatible %" PRIu32 "-byte alignment (offset %" PRIu16 ")", + sect.name.c_str(), sectAlignSize, sect.alignOfs ); @@ -172,14 +192,9 @@ static unsigned int sect.alignOfs = alignOffset; } } - - return nbSectErrors; } -static unsigned int - mergeFragments(Section §, uint32_t org, uint8_t alignment, uint16_t alignOffset) { - unsigned int nbSectErrors = 0; - +static void mergeFragments(Section §, uint32_t org, uint8_t alignment, uint16_t alignOffset) { assume(alignment < 16); // Should be ensured by the caller uint32_t alignSize = 1u << alignment; uint32_t alignMask = alignSize - 1; @@ -197,11 +212,15 @@ static unsigned int // If both are fixed, they must be the same if (sect.org != UINT32_MAX && sect.org != curOrg) { sectError( - "Section already declared as fixed at incompatible address $%04" PRIx32, sect.org + "Section \"%s\" already declared as fixed at incompatible address $%04" PRIx32, + sect.name.c_str(), + sect.org ); } else if (sect.align != 0 && ((curOrg - sect.alignOfs) & sectAlignMask)) { sectError( - "Section already declared as aligned to %" PRIu32 " bytes (offset %" PRIu16 ")", + "Section \"%s\" already declared as aligned to %" PRIu32 " bytes (offset %" PRIu16 + ")", + sect.name.c_str(), sectAlignSize, sect.alignOfs ); @@ -215,15 +234,17 @@ static unsigned int if (uint32_t curOfs = (alignOffset - sect.size) & alignMask; sect.org != UINT32_MAX) { if ((sect.org - curOfs) & alignMask) { sectError( - "Section already declared as fixed at incompatible address $%04" PRIx32, + "Section \"%s\" already declared as fixed at incompatible address $%04" PRIx32, + sect.name.c_str(), sect.org ); } // Check if alignment offsets are compatible } else if ((curOfs & sectAlignMask) != (sect.alignOfs & alignMask)) { sectError( - "Section already declared with incompatible %" PRIu32 + "Section \"%s\" already declared with incompatible %" PRIu32 "-byte alignment (offset %" PRIu16 ")", + sect.name.c_str(), sectAlignSize, sect.alignOfs ); @@ -233,8 +254,6 @@ static unsigned int sect.alignOfs = curOfs; } } - - return nbSectErrors; } static void mergeSections( @@ -246,23 +265,36 @@ static void mergeSections( uint16_t alignOffset, SectionModifier mod ) { - unsigned int nbSectErrors = 0; + sectErrors.clear(); + + auto sectAlreadyDefinedCallback = [§]() { + fprintf(stderr, "Section \"%s\" already defined\n", sect.name.c_str()); + fstk_TraceCurrent(); + fputs(" and also:\n", stderr); + sect.src->printBacktrace(sect.fileLine); + }; if (type != sect.type) { sectError( - "Section already exists but with type `%s`", sectionTypeInfo[sect.type].name.c_str() + "Section \"%s\" already exists but with type `%s`", + sect.name.c_str(), + sectionTypeInfo[sect.type].name.c_str() ); } if (sect.modifier != mod) { - sectError("Section already declared as `SECTION %s`", sectionModNames[sect.modifier]); + sectError( + "Section \"%s\" already declared as `SECTION %s`", + sect.name.c_str(), + sectionModNames[sect.modifier] + ); } else { switch (mod) { case SECTION_UNION: case SECTION_FRAGMENT: { - unsigned int (*merge)(Section &, uint32_t, uint8_t, uint16_t) = + void (*merge)(Section &, uint32_t, uint8_t, uint16_t) = mod == SECTION_UNION ? mergeSectUnion : mergeFragments; - nbSectErrors += merge(sect, org, alignment, alignOffset); + merge(sect, org, alignment, alignOffset); // If the section's bank is unspecified, override it if (sect.bank == UINT32_MAX) { @@ -270,26 +302,41 @@ static void mergeSections( } // If both specify a bank, it must be the same one else if (bank != UINT32_MAX && sect.bank != bank) { - sectError("Section already declared with different bank %" PRIu32, sect.bank); + sectError( + "Section \"%s\" already declared with different bank %" PRIu32, + sect.name.c_str(), + sect.bank + ); } break; } case SECTION_NORMAL: - errorNoTrace([&]() { - fputs("Section already defined\n", stderr); - fstk_TraceCurrent(); - fputs(" and also:\n", stderr); - sect.src->printBacktrace(sect.fileLine); - ++nbSectErrors; - }); + // The empty string is a sentinel for the `sectAlreadyDefinedCallback` error, + // since it cannot be preformatted as a string + sectError(""); break; } } - if (nbSectErrors) { + if (size_t nbSectErrors = sectErrors.size(); nbSectErrors == 1) { + // If there was only one error, print it as a fatal error + if (std::string const &message = sectErrors[0]; message.empty()) { + fatalNoTrace(sectAlreadyDefinedCallback); + } else { + fatal("%s", message.c_str()); + } + } else if (nbSectErrors > 1) { + // If there were multiple errors, print each of them, followed by a fatal summary error + for (std::string const &message : sectErrors) { + if (message.empty()) { + errorNoTrace(sectAlreadyDefinedCallback); + } else { + error("%s", message.c_str()); + } + } fatal( - "Cannot create section \"%s\" (%u error%s)", + "Cannot create section \"%s\" (%zu error%s)", sect.name.c_str(), nbSectErrors, nbSectErrors == 1 ? "" : "s" @@ -297,8 +344,6 @@ static void mergeSections( } } -#undef sectError - static Section *createSection( std::string const &name, SectionType type, diff --git a/src/asm/warning.cpp b/src/asm/warning.cpp index 964723835..41cde1c0c 100644 --- a/src/asm/warning.cpp +++ b/src/asm/warning.cpp @@ -131,6 +131,16 @@ void fatal(char const *fmt, ...) { exit(1); } +[[noreturn]] +void fatalNoTrace(std::function callback) { + style_Set(stderr, STYLE_RED, true); + fputs("FATAL: ", stderr); + style_Reset(stderr); + callback(); + + exit(1); +} + void requireZeroErrors() { if (warnings.nbErrors != 0) { style_Set(stderr, STYLE_RED, true); diff --git a/test/asm/duplicate-section-after-literal.err b/test/asm/duplicate-section-after-literal.err index e86cd1ed7..91415ba68 100644 --- a/test/asm/duplicate-section-after-literal.err +++ b/test/asm/duplicate-section-after-literal.err @@ -1,6 +1,4 @@ -error: Section already defined +FATAL: Section "dup" already defined at duplicate-section-after-literal.asm(7) and also: at duplicate-section-after-literal.asm(5) -FATAL: Cannot create section "dup" (1 error) - at duplicate-section-after-literal.asm(7) diff --git a/test/asm/duplicate-section.err b/test/asm/duplicate-section.err index 681d4ffcf..551a1221e 100644 --- a/test/asm/duplicate-section.err +++ b/test/asm/duplicate-section.err @@ -1,6 +1,4 @@ -error: Section already defined +FATAL: Section "sec" already defined at duplicate-section.asm(4) and also: at duplicate-section.asm(2) -FATAL: Cannot create section "sec" (1 error) - at duplicate-section.asm(4) diff --git a/test/asm/fragment-align-mismatch.err b/test/asm/fragment-align-mismatch.err index 00812fd4b..e7d2b3a00 100644 --- a/test/asm/fragment-align-mismatch.err +++ b/test/asm/fragment-align-mismatch.err @@ -1,4 +1,2 @@ -error: Section already declared as fixed at incompatible address $c002 - at fragment-align-mismatch.asm(2) -FATAL: Cannot create section "aligned" (1 error) +FATAL: Section "aligned" already declared as fixed at incompatible address $c002 at fragment-align-mismatch.asm(2) diff --git a/test/asm/fragment-mismatch.err b/test/asm/fragment-mismatch.err index 00ccbea51..69709c997 100644 --- a/test/asm/fragment-mismatch.err +++ b/test/asm/fragment-mismatch.err @@ -1,4 +1,2 @@ -error: Section already declared as fixed at incompatible address $0000 - at fragment-mismatch.asm(2) -FATAL: Cannot create section "test" (1 error) +FATAL: Section "test" already declared as fixed at incompatible address $0000 at fragment-mismatch.asm(2) diff --git a/test/asm/incompatible-alignment.err b/test/asm/incompatible-alignment.err index aca908fd8..4358ccc74 100644 --- a/test/asm/incompatible-alignment.err +++ b/test/asm/incompatible-alignment.err @@ -1,4 +1,2 @@ -error: Section already declared with incompatible 256-byte alignment (offset 0) - at incompatible-alignment.asm(8) -FATAL: Cannot create section "Test" (1 error) +FATAL: Section "Test" already declared with incompatible 256-byte alignment (offset 0) at incompatible-alignment.asm(8) diff --git a/test/asm/multiple-section-errors.asm b/test/asm/multiple-section-errors.asm new file mode 100644 index 000000000..96294204f --- /dev/null +++ b/test/asm/multiple-section-errors.asm @@ -0,0 +1,3 @@ +SECTION FRAGMENT "test", ROM0[$0000] + +SECTION FRAGMENT "test", ROMX[$4000] diff --git a/test/asm/multiple-section-errors.err b/test/asm/multiple-section-errors.err new file mode 100644 index 000000000..57b7e0e7f --- /dev/null +++ b/test/asm/multiple-section-errors.err @@ -0,0 +1,6 @@ +error: Section "test" already exists but with type `ROM0` + at multiple-section-errors.asm(3) +error: Section "test" already declared as fixed at incompatible address $0000 + at multiple-section-errors.asm(3) +FATAL: Cannot create section "test" (2 errors) + at multiple-section-errors.asm(3) diff --git a/test/asm/section-union-mismatch.err b/test/asm/section-union-mismatch.err index 7fcffe4c7..2f4c4268f 100644 --- a/test/asm/section-union-mismatch.err +++ b/test/asm/section-union-mismatch.err @@ -1,4 +1,2 @@ -error: Section already declared as fixed at different address $c000 - at section-union-mismatch.asm(2) -FATAL: Cannot create section "test" (1 error) +FATAL: Section "test" already declared as fixed at different address $c000 at section-union-mismatch.asm(2) diff --git a/test/asm/section-union.err b/test/asm/section-union.err index 111f8d369..df16f90ce 100644 --- a/test/asm/section-union.err +++ b/test/asm/section-union.err @@ -1,4 +1,2 @@ -error: Section already declared as `SECTION UNION` - at section-union.asm(37) -FATAL: Cannot create section "test" (1 error) +FATAL: Section "test" already declared as `SECTION UNION` at section-union.asm(37) diff --git a/test/asm/union-mismatch.err b/test/asm/union-mismatch.err index ee618d2a1..1dd89ca00 100644 --- a/test/asm/union-mismatch.err +++ b/test/asm/union-mismatch.err @@ -1,4 +1,2 @@ -error: Section already declared as fixed at incompatible address $c001 - at union-mismatch.asm(2) -FATAL: Cannot create section "fixed" (1 error) +FATAL: Section "fixed" already declared as fixed at incompatible address $c001 at union-mismatch.asm(2) diff --git a/test/link/section-fragment/align-conflict.out b/test/link/section-fragment/align-conflict.out index 707a4a5f7..f2a02e191 100644 --- a/test/link/section-fragment/align-conflict.out +++ b/test/link/section-fragment/align-conflict.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting alignment" is defined with 4-byte alignment (offset at section-fragment/align-conflict.asm(7) Linking aborted with 1 error --- -error: Section already declared as aligned to 4 bytes (offset 0) - at (18) -FATAL: Cannot create section "conflicting alignment" (1 error) +FATAL: Section "conflicting alignment" already declared as aligned to 4 bytes (offset 0) at (18) diff --git a/test/link/section-union/align-conflict.out b/test/link/section-union/align-conflict.out index d2ec94e74..70cdf033b 100644 --- a/test/link/section-union/align-conflict.out +++ b/test/link/section-union/align-conflict.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting alignment" is defined with 4-byte alignment (offset at section-union/align-conflict.asm(7) Linking aborted with 1 error --- -error: Section already declared as aligned to 4 bytes (offset 0) - at (18) -FATAL: Cannot create section "conflicting alignment" (1 error) +FATAL: Section "conflicting alignment" already declared as aligned to 4 bytes (offset 0) at (18) diff --git a/test/link/section-union/align-ofs-conflict.out b/test/link/section-union/align-ofs-conflict.out index 8f06c930b..bd70a2c1a 100644 --- a/test/link/section-union/align-ofs-conflict.out +++ b/test/link/section-union/align-ofs-conflict.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting alignment" is defined with 8-byte alignment (offset at section-union/align-ofs-conflict.asm(7) Linking aborted with 1 error --- -error: Section already declared with incompatible 8-byte alignment (offset 7) - at (18) -FATAL: Cannot create section "conflicting alignment" (1 error) +FATAL: Section "conflicting alignment" already declared with incompatible 8-byte alignment (offset 7) at (18) diff --git a/test/link/section-union/bad-types.out b/test/link/section-union/bad-types.out index aa508e29f..03f8895d6 100644 --- a/test/link/section-union/bad-types.out +++ b/test/link/section-union/bad-types.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting types" is defined with type `HRAM`, but also with ty at section-union/bad-types.asm(7) Linking aborted with 1 error --- -error: Section already exists but with type `HRAM` - at (18) -FATAL: Cannot create section "conflicting types" (1 error) +FATAL: Section "conflicting types" already exists but with type `HRAM` at (18) diff --git a/test/link/section-union/bank-conflict.out b/test/link/section-union/bank-conflict.out index 79579984b..bea83d579 100644 --- a/test/link/section-union/bank-conflict.out +++ b/test/link/section-union/bank-conflict.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting banks" is defined with bank 4, but also with bank 1 at section-union/bank-conflict.asm(5) Linking aborted with 1 error --- -error: Section already declared with different bank 4 - at (14) -FATAL: Cannot create section "conflicting banks" (1 error) +FATAL: Section "conflicting banks" already declared with different bank 4 at (14) diff --git a/test/link/section-union/different-ofs.out b/test/link/section-union/different-ofs.out index 845e97631..3c6106277 100644 --- a/test/link/section-union/different-ofs.out +++ b/test/link/section-union/different-ofs.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting alignment" is defined with 8-byte alignment (offset at section-union/different-ofs.asm(7) Linking aborted with 1 error --- -error: Section already declared with incompatible 8-byte alignment (offset 7) - at (18) -FATAL: Cannot create section "conflicting alignment" (1 error) +FATAL: Section "conflicting alignment" already declared with incompatible 8-byte alignment (offset 7) at (18) diff --git a/test/link/section-union/org-conflict.out b/test/link/section-union/org-conflict.out index 56293db87..c2247b7cd 100644 --- a/test/link/section-union/org-conflict.out +++ b/test/link/section-union/org-conflict.out @@ -4,7 +4,5 @@ FATAL: Section "conflicting address" is defined with address $beef, but also wit at section-union/org-conflict.asm(7) Linking aborted with 1 error --- -error: Section already declared as fixed at different address $beef - at (16) -FATAL: Cannot create section "conflicting address" (1 error) +FATAL: Section "conflicting address" already declared as fixed at different address $beef at (16) From bd9d0051bc4b5d444d923f0f05160b72db1666ef Mon Sep 17 00:00:00 2001 From: Rangi Date: Mon, 11 May 2026 09:00:14 -0400 Subject: [PATCH 2/2] Factor out `sectAlreadyDefinedError` --- src/asm/section.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 33b060176..9514adfb9 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -273,6 +273,11 @@ static void mergeSections( fputs(" and also:\n", stderr); sect.src->printBacktrace(sect.fileLine); }; + auto sectAlreadyDefinedError = []() { + // The empty string is a sentinel for the `sectAlreadyDefinedCallback` error, + // since it cannot be preformatted as a string + sectErrors.push_back(""); + }; if (type != sect.type) { sectError( @@ -312,9 +317,7 @@ static void mergeSections( } case SECTION_NORMAL: - // The empty string is a sentinel for the `sectAlreadyDefinedCallback` error, - // since it cannot be preformatted as a string - sectError(""); + sectAlreadyDefinedError(); break; } }