Add RGBLINK truncation warnings for add sp, e8 and ld hl, sp + e8 (#2179)

include/asm/section.hpp

@@ -97,6 +97,7 @@ void sect_WordString(std::vector<int32_t> const &str);
 void sect_LongString(std::vector<int32_t> 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<Expression> const &exprs);
 void sect_RelWord(Expression const &expr, uint32_t pcShift);
 void sect_RelLong(Expression const &expr, uint32_t pcShift);

include/linkdefs.hpp

@@ -127,6 +127,7 @@ enum PatchType {
 	PATCHTYPE_WORD,
 	PATCHTYPE_LONG,
 	PATCHTYPE_JR,
+	PATCHTYPE_SIGNED_BYTE,
 
 	PATCHTYPE_INVALID
 };

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 {

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",

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<Expression> const &exprs) {
 	if (!requireCodeSection()) {
 		return;

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 &section, Section &dataSection) {
 	verbosePrint(VERB_INFO, "Patching section \"%s\"...\n", section.name.c_str());
@@ -572,6 +589,7 @@ static void applyFilePatches(Section &section, 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 &section, 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 &section, 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) {

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

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)

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