Add 0/1/2 warning levels to rgblink -Wtruncation (#1816)

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<WarningLevel, WarningID> warnings;

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:

src/link/patch.cpp

@@ -4,6 +4,7 @@
 
 #include <deque>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdint.h>
 #include <variant>
 #include <vector>
@@ -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 &section, Section &dataSection) {
 	verbosePrint(VERB_INFO, "Patching section \"%s\"...\n", section.name.c_str());
@@ -510,23 +535,19 @@ static void applyFilePatches(Section &section, Section &dataSection) {
 		int32_t value = computeRPNExpr(patch, *section.fileSymbols);
 		uint16_t offset = patch.offset + section.offset;
 
-		struct {
+		uint8_t typeSizes[PATCHTYPE_INVALID] = {
-			uint8_t size;
+		    1, // PATCHTYPE_BYTE
-			int32_t min;
+		    2, // PATCHTYPE_WORD
-			int32_t max;
+		    4, // PATCHTYPE_LONG
-		} const types[PATCHTYPE_INVALID] = {
+		    1, // PATCHTYPE_JR
-		    {1, -128,      255      }, // PATCHTYPE_BYTE
-		    {2, -32768,    65536    }, // PATCHTYPE_WORD
-		    {4, INT32_MIN, INT32_MAX}, // PATCHTYPE_LONG
-		    {1, 0,         0        }, // 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 &section, Section &dataSection) {
 			dataSection.data[offset] = jumpOffset & 0xFF;
 		} else {
 			// Patch a certain number of bytes
-			if (value < type.min || value > type.max) {
+			checkPatchSize(patch, value, typeSize * 8);
-				diagnosticAt(
+			for (uint8_t i = 0; i < typeSize; ++i) {
-				    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) {
 				dataSection.data[offset + i] = value & 0xFF;
 				value >>= 8;
 			}

src/link/warning.cpp

@@ -26,9 +26,13 @@ Diagnostics<WarningLevel, WarningID> 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<WarningID>(),
     .nbErrors = 0,
 };

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

test/link/truncation/level1/a.asm

@@ -0,0 +1,6 @@
+section "rom", rom0
+ld bc, -wLabel
+ld de, -(wLabel * 2)
+
+section "ram", wram0
+wLabel::

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)

test/link/truncation/level2/a.asm

@@ -0,0 +1,6 @@
+section "rom", rom0
+ld bc, -wLabel
+ld de, -(wLabel * 2)
+
+section "ram", wram0
+wLabel::

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)