From 380c22f343a9658590c1c92b1ddc4dec36238663 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Sun, 27 Sep 2026 20:08:26 -0400 Subject: [PATCH] Add RGBLINK truncation warnings for `add sp, e8` and `ld hl, sp + e8` (#2179) --- include/asm/section.hpp | 1 + include/linkdefs.hpp | 1 + src/asm/parser.y | 4 ++-- src/asm/rpn.cpp | 3 +-- src/asm/section.cpp | 13 +++++++++++++ src/link/patch.cpp | 24 ++++++++++++++++++++++-- test/link/signed-byte-bad.asm | 13 +++++++++++++ test/link/signed-byte-bad.out | 12 ++++++++++++ test/link/signed-byte.asm | 6 ++++++ test/link/signed-byte.out | 0 test/link/signed-byte.out.bin | Bin 0 -> 135 bytes 11 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 test/link/signed-byte-bad.asm create mode 100644 test/link/signed-byte-bad.out create mode 100644 test/link/signed-byte.asm create mode 100644 test/link/signed-byte.out create mode 100644 test/link/signed-byte.out.bin diff --git a/include/asm/section.hpp b/include/asm/section.hpp index 92164830..b9eb4474 100644 --- a/include/asm/section.hpp +++ b/include/asm/section.hpp @@ -97,6 +97,7 @@ void sect_WordString(std::vector const &str); void sect_LongString(std::vector const &str); void sect_Skip(uint32_t skip, bool ds); void sect_RelByte(Expression const &expr, uint32_t pcShift); +void sect_RelSignedByte(Expression const &expr, uint32_t pcShift); void sect_RelBytes(uint32_t n, std::vector const &exprs); void sect_RelWord(Expression const &expr, uint32_t pcShift); void sect_RelLong(Expression const &expr, uint32_t pcShift); diff --git a/include/linkdefs.hpp b/include/linkdefs.hpp index f8dbcdae..2d0b3498 100644 --- a/include/linkdefs.hpp +++ b/include/linkdefs.hpp @@ -127,6 +127,7 @@ enum PatchType { PATCHTYPE_WORD, PATCHTYPE_LONG, PATCHTYPE_JR, + PATCHTYPE_SIGNED_BYTE, PATCHTYPE_INVALID }; diff --git a/src/asm/parser.y b/src/asm/parser.y index 4970995f..70200ca2 100644 --- a/src/asm/parser.y +++ b/src/asm/parser.y @@ -1928,7 +1928,7 @@ sm83_add: } | SM83_ADD MODE_SP COMMA reloc_8bit_signed { sect_ConstByte(0xE8); - sect_RelByte($4, 1); + sect_RelSignedByte($4, 1); sym_IncrementCYCLESValue(4); } ; @@ -2168,7 +2168,7 @@ sm83_ld: sm83_ld_hl: SM83_LD MODE_HL COMMA MODE_SP op_sp_offset { sect_ConstByte(0xF8); - sect_RelByte($5, 1); + sect_RelSignedByte($5, 1); sym_IncrementCYCLESValue(3); } | SM83_LD MODE_HL COMMA reloc_16bit { diff --git a/src/asm/rpn.cpp b/src/asm/rpn.cpp index 5dd7f29f..2458ebbe 100644 --- a/src/asm/rpn.cpp +++ b/src/asm/rpn.cpp @@ -505,8 +505,7 @@ bool checkNBit(int32_t v, uint8_t n, char const *name) { n == 8 && !name ? "; use `LOW()` to force 8-bit" : "" ); return false; - } - if (v < -(1 << (n - 1))) { + } else if (v < -(1 << (n - 1))) { warning( WARNING_TRUNCATION_2, "%s must be %u-bit%s", diff --git a/src/asm/section.cpp b/src/asm/section.cpp index 6b6d4e94..e03635a0 100644 --- a/src/asm/section.cpp +++ b/src/asm/section.cpp @@ -911,6 +911,19 @@ void sect_RelByte(Expression const &expr, uint32_t pcShift) { } } +void sect_RelSignedByte(Expression const &expr, uint32_t pcShift) { + if (!requireCodeSection()) { + return; + } + + if (!expr.isKnown()) { + createPatch(PATCHTYPE_SIGNED_BYTE, expr, pcShift); + writeByte(0); + } else { + writeByte(expr.value()); + } +} + void sect_RelBytes(uint32_t n, std::vector const &exprs) { if (!requireCodeSection()) { return; diff --git a/src/link/patch.cpp b/src/link/patch.cpp index 272d071e..2f1629f3 100644 --- a/src/link/patch.cpp +++ b/src/link/patch.cpp @@ -560,6 +560,23 @@ static void checkPatchSize(Patch const &patch, int32_t v, uint8_t n) { } } +static void checkSignedPatchSize(Patch const &patch, int32_t v, uint8_t n) { + assume(n != 0); // That doesn't make sense + assume(n < CHAR_BIT * sizeof(int) - 1); // Otherwise `1 << n` is UB + + if (v < -(1 << (n - 1)) || v >= 1 << (n - 1)) { + if (v < 0) { + diagnosticAt( + patch, WARNING_TRUNCATION_1, "Value -$%" PRIx32 " is not signed %u-bit", -v, n + ); + } else { + diagnosticAt( + patch, WARNING_TRUNCATION_1, "Value $%" PRIx32 " is not signed %u-bit", v, 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()); @@ -572,6 +589,7 @@ static void applyFilePatches(Section §ion, Section &dataSection) { 2, // PATCHTYPE_WORD 4, // PATCHTYPE_LONG 1, // PATCHTYPE_JR + 1, // PATCHTYPE_SIGNED_BYTE }; uint8_t typeSize = typeSizes[patch.type]; @@ -588,7 +606,7 @@ static void applyFilePatches(Section §ion, Section &dataSection) { rpnErrorAt(patch, "PC has no value outside of a section"); dataSection.data[offset] = 0; } else { - // A `jr` is *encoded* in ROM as a 1-byte (8-bit) offset, so here `typeSize == 8`, + // A `jr` is *encoded* in ROM as a 1-byte (8-bit) offset, so here `typeSize == 1`, // but the object's *value* size is a 16-bit absolute address, so we pass 16 here. checkPatchSize(patch, value, 16); // Offset is relative to the byte *after* the operand @@ -610,7 +628,9 @@ static void applyFilePatches(Section §ion, Section &dataSection) { } } else { // Patch a certain number of bytes - if (typeSize < sizeof(int)) { + if (patch.type == PATCHTYPE_SIGNED_BYTE) { + checkSignedPatchSize(patch, value, typeSize * 8); + } else if (typeSize < sizeof(int)) { checkPatchSize(patch, value, typeSize * 8); } for (uint8_t i = 0; i < typeSize; ++i) { diff --git a/test/link/signed-byte-bad.asm b/test/link/signed-byte-bad.asm new file mode 100644 index 00000000..77d72395 --- /dev/null +++ b/test/link/signed-byte-bad.asm @@ -0,0 +1,13 @@ +SECTION "limit", ROM0, ALIGN[8, 128] +Limit: +add sp, Limit +add sp, -Limit +ld hl, sp + Limit +ld hl, sp - Limit + +SECTION "test", ROMX +Invalid: +add sp, Invalid +add sp, -Invalid +ld hl, sp + Invalid +ld hl, sp - Invalid diff --git a/test/link/signed-byte-bad.out b/test/link/signed-byte-bad.out new file mode 100644 index 00000000..d24dc1f8 --- /dev/null +++ b/test/link/signed-byte-bad.out @@ -0,0 +1,12 @@ +warning: Value $80 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(5) +warning: Value $80 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(3) +warning: Value -$4000 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(13) +warning: Value $4000 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(12) +warning: Value -$4000 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(11) +warning: Value $4000 is not signed 8-bit [-Wtruncation] + at signed-byte-bad.asm(10) diff --git a/test/link/signed-byte.asm b/test/link/signed-byte.asm new file mode 100644 index 00000000..93277cea --- /dev/null +++ b/test/link/signed-byte.asm @@ -0,0 +1,6 @@ +SECTION "test", ROM0, ALIGN[8, 127] +Valid: +add sp, Valid +add sp, -Valid +ld hl, sp + Valid +ld hl, sp - Valid diff --git a/test/link/signed-byte.out b/test/link/signed-byte.out new file mode 100644 index 00000000..e69de29b diff --git a/test/link/signed-byte.out.bin b/test/link/signed-byte.out.bin new file mode 100644 index 0000000000000000000000000000000000000000..a1596effdb4cda7a72ab6a95fed7ab5ffc315306 GIT binary patch literal 135 TcmZQz7*Oz{{zc=D`X7w|8v_Nw literal 0 HcmV?d00001