From 534a4efee45b873dc984c298ea0efc2a0a4103bd Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Mon, 1 Sep 2025 15:35:53 -0400 Subject: [PATCH] Add 0/1/2 warning levels to `rgblink -Wtruncation` (#1816) --- include/link/warning.hpp | 7 +++- man/rgblink.1 | 13 ++++++- src/link/patch.cpp | 58 +++++++++++++++++------------ src/link/warning.cpp | 6 ++- test/link/test.sh | 16 ++++++++ test/link/truncation/level1/a.asm | 6 +++ test/link/truncation/level1/out.err | 2 + test/link/truncation/level2/a.asm | 6 +++ test/link/truncation/level2/out.err | 4 ++ 9 files changed, 90 insertions(+), 28 deletions(-) create mode 100644 test/link/truncation/level1/a.asm create mode 100644 test/link/truncation/level1/out.err create mode 100644 test/link/truncation/level2/a.asm create mode 100644 test/link/truncation/level2/out.err diff --git a/include/link/warning.hpp b/include/link/warning.hpp index c30d8bb5..a19f7a16 100644 --- a/include/link/warning.hpp +++ b/include/link/warning.hpp @@ -27,11 +27,14 @@ enum WarningID { WARNING_OBSOLETE, // Obsolete/deprecated things WARNING_SHIFT, // Undefined `SHIFT` behavior WARNING_SHIFT_AMOUNT, // Strange `SHIFT` amount - WARNING_TRUNCATION, // Implicit truncation loses some bits NB_PLAIN_WARNINGS, - NB_WARNINGS = NB_PLAIN_WARNINGS, + // Implicit truncation loses some bits + WARNING_TRUNCATION_1 = NB_PLAIN_WARNINGS, + WARNING_TRUNCATION_2, + + NB_WARNINGS, }; extern Diagnostics warnings; diff --git a/man/rgblink.1 b/man/rgblink.1 index d073fc53..3ff995a4 100644 --- a/man/rgblink.1 +++ b/man/rgblink.1 @@ -318,11 +318,20 @@ This warning is enabled by Warn when a shift's operand is negative or greater than 32. This warning is enabled by .Fl Wall . -.It Fl Wno-truncation +.It Fl Wtruncation= Warn when an implicit truncation (for example, .Ic db to an 8-bit value) loses some bits. -This occurs when an N-bit value is 2**N or greater, or less than -2**N. +.Fl Wtruncation=0 +or +.Fl Wno-truncation +disables this warning. +.Fl Wtruncation=1 +warns when an N-bit value is 2**N or greater, or less than -2**N. +.Fl Wtruncation=2 +or just +.Fl Wtruncation +also warns when an N-bit value is less than -2**(N-1), which will not fit in two's complement encoding. .El .Sh EXAMPLES All you need for a basic ROM is an object file, which can be made into a ROM image like so: diff --git a/src/link/patch.cpp b/src/link/patch.cpp index f551e3a2..385d1557 100644 --- a/src/link/patch.cpp +++ b/src/link/patch.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -503,6 +504,30 @@ void patch_CheckAssertions() { } } +static void checkPatchSize(Patch const &patch, int32_t v, uint8_t n) { + static constexpr unsigned m = CHAR_BIT * sizeof(int); + if (n < m && (v < -(1 << n) || v >= 1 << n)) { + diagnosticAt( + patch, + WARNING_TRUNCATION_1, + "Value $%" PRIx32 "%s is not %u-bit", + v, + v < 0 ? " (may be negative?)" : "", + n + ); + return; + } else if (n < m + 1 && v < -(1 << (n - 1))) { + diagnosticAt( + patch, + WARNING_TRUNCATION_2, + "Value $%" PRIx32 "%s is not %u-bit", + v, + v < 0 ? " (may be negative?)" : "", + n + ); + } +} + // Applies all of a section's patches to a data section static void applyFilePatches(Section §ion, Section &dataSection) { verbosePrint(VERB_INFO, "Patching section \"%s\"...\n", section.name.c_str()); @@ -510,23 +535,19 @@ static void applyFilePatches(Section §ion, Section &dataSection) { int32_t value = computeRPNExpr(patch, *section.fileSymbols); uint16_t offset = patch.offset + section.offset; - struct { - uint8_t size; - int32_t min; - int32_t max; - } const types[PATCHTYPE_INVALID] = { - {1, -128, 255 }, // PATCHTYPE_BYTE - {2, -32768, 65536 }, // PATCHTYPE_WORD - {4, INT32_MIN, INT32_MAX}, // PATCHTYPE_LONG - {1, 0, 0 }, // PATCHTYPE_JR + uint8_t typeSizes[PATCHTYPE_INVALID] = { + 1, // PATCHTYPE_BYTE + 2, // PATCHTYPE_WORD + 4, // PATCHTYPE_LONG + 1, // PATCHTYPE_JR }; - auto const &type = types[patch.type]; + uint8_t typeSize = typeSizes[patch.type]; - if (dataSection.data.size() < offset + type.size) { + if (dataSection.data.size() < offset + typeSize) { errorAt( patch, "Patch would write %zu bytes past the end of section \"%s\" (%zu bytes long)", - offset + type.size - dataSection.data.size(), + offset + typeSize - dataSection.data.size(), dataSection.name.c_str(), dataSection.data.size() ); @@ -547,17 +568,8 @@ static void applyFilePatches(Section §ion, Section &dataSection) { dataSection.data[offset] = jumpOffset & 0xFF; } else { // Patch a certain number of bytes - if (value < type.min || value > type.max) { - diagnosticAt( - patch, - WARNING_TRUNCATION, - "Value $%" PRIx32 "%s is not %u-bit", - value, - value < 0 ? " (may be negative?)" : "", - type.size * 8U - ); - } - for (uint8_t i = 0; i < type.size; ++i) { + checkPatchSize(patch, value, typeSize * 8); + for (uint8_t i = 0; i < typeSize; ++i) { dataSection.data[offset + i] = value & 0xFF; value >>= 8; } diff --git a/src/link/warning.cpp b/src/link/warning.cpp index 3bcd4c87..7b6293ea 100644 --- a/src/link/warning.cpp +++ b/src/link/warning.cpp @@ -26,9 +26,13 @@ Diagnostics warnings = { {"obsolete", LEVEL_DEFAULT }, {"shift", LEVEL_ALL }, {"shift-amount", LEVEL_ALL }, + // Parametric warnings {"truncation", LEVEL_DEFAULT }, + {"truncation", LEVEL_EVERYTHING}, + }, + .paramWarnings = { + {WARNING_TRUNCATION_1, WARNING_TRUNCATION_2, 2}, }, - .paramWarnings = {}, .state = DiagnosticsState(), .nbErrors = 0, }; diff --git a/test/link/test.sh b/test/link/test.sh index 10af49c9..f2f6a4a9 100755 --- a/test/link/test.sh +++ b/test/link/test.sh @@ -439,6 +439,22 @@ rgblinkQuiet "$otemp" "$gbtemp" "$gbtemp2" "$outtemp" "$outtemp2" 2>"$outtemp3" tryDiff "$test"/out.err "$outtemp3" evaluateTest +test="truncation/level1" +startTest +"$RGBASM" -o "$otemp" "$test"/a.asm +continueTest +rgblinkQuiet -Wtruncation=1 -o "$gbtemp" "$otemp" 2>"$outtemp" +tryDiff "$test"/out.err "$outtemp" +evaluateTest + +test="truncation/level2" +startTest +"$RGBASM" -o "$otemp" "$test"/a.asm +continueTest +rgblinkQuiet -Wtruncation=2 -o "$gbtemp" "$otemp" 2>"$outtemp" +tryDiff "$test"/out.err "$outtemp" +evaluateTest + if [[ "$failed" -eq 0 ]]; then echo "${bold}${green}All ${tests} tests passed!${rescolors}${resbold}" else diff --git a/test/link/truncation/level1/a.asm b/test/link/truncation/level1/a.asm new file mode 100644 index 00000000..f1c30b4a --- /dev/null +++ b/test/link/truncation/level1/a.asm @@ -0,0 +1,6 @@ +section "rom", rom0 +ld bc, -wLabel +ld de, -(wLabel * 2) + +section "ram", wram0 +wLabel:: diff --git a/test/link/truncation/level1/out.err b/test/link/truncation/level1/out.err new file mode 100644 index 00000000..4b33068e --- /dev/null +++ b/test/link/truncation/level1/out.err @@ -0,0 +1,2 @@ +warning: Value $fffe8000 (may be negative?) is not 16-bit [-Wtruncation] + at truncation/level1/a.asm(3) diff --git a/test/link/truncation/level2/a.asm b/test/link/truncation/level2/a.asm new file mode 100644 index 00000000..f1c30b4a --- /dev/null +++ b/test/link/truncation/level2/a.asm @@ -0,0 +1,6 @@ +section "rom", rom0 +ld bc, -wLabel +ld de, -(wLabel * 2) + +section "ram", wram0 +wLabel:: diff --git a/test/link/truncation/level2/out.err b/test/link/truncation/level2/out.err new file mode 100644 index 00000000..eec42e89 --- /dev/null +++ b/test/link/truncation/level2/out.err @@ -0,0 +1,4 @@ +warning: Value $fffe8000 (may be negative?) is not 16-bit [-Wtruncation] + at truncation/level2/a.asm(3) +warning: Value $ffff4000 (may be negative?) is not 16-bit [-Wtruncation] + at truncation/level2/a.asm(2)