Consistently do & alignMask, not % alignSize

Also add more unrelated tests for coverage

src/asm/section.cpp

@@ -218,14 +218,8 @@ static unsigned int
 		}
 
 	} else if (alignment != 0) {
-		int32_t curOfs = (alignOffset - sect.size) % alignSize;
-
-		if (curOfs < 0) {
-			curOfs += alignSize;
-		}
-
 		// Make sure any fixed address given is compatible
-		if (sect.org != UINT32_MAX) {
+		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,
@@ -624,10 +618,10 @@ uint32_t sect_GetAlignBytes(uint8_t alignment, uint16_t offset) {
 		return 0;
 	}
 
-	// We need `(pcValue + curOffset + return value) % (1 << alignment) == offset`
+	// We need `(pcValue + curOffset + return value) & minAlignMask == offset`
 	uint16_t pcValue = isFixed ? sect->org : sect->alignOfs;
-	return static_cast<uint16_t>(offset - curOffset - pcValue)
+	uint32_t minAlignMask = (1u << std::min(alignment, curAlignment)) - 1;
-	       % (1u << std::min(alignment, curAlignment));
+	return static_cast<uint16_t>(offset - curOffset - pcValue) & minAlignMask;
 }
 
 void sect_AlignPC(uint8_t alignment, uint16_t offset) {
@@ -637,13 +631,15 @@ void sect_AlignPC(uint8_t alignment, uint16_t offset) {
 
 	assume(alignment <= 16); // Should be ensured by the caller
 	uint32_t alignSize = 1u << alignment;
+	uint32_t alignMask = alignSize - 1;
 
 	Section *sect = sect_GetSymbolSection();
 	assume(sect->align <= 16); // Left-shifting by 32 or more would be UB
 	uint32_t sectAlignSize = 1u << sect->align;
+	uint32_t sectAlignMask = sectAlignSize - 1;
 
 	if (sect->org != UINT32_MAX) {
-		if (uint32_t actualOffset = (sect->org + curOffset) % alignSize; actualOffset != offset) {
+		if (uint32_t actualOffset = (sect->org + curOffset) & alignMask; actualOffset != offset) {
 			error(
 			    "Section is misaligned (at PC = $%04" PRIx32 ", expected ALIGN[%" PRIu32
 			    ", %" PRIu32 "], got ALIGN[%" PRIu32 ", %" PRIu32 "])",
@@ -654,8 +650,8 @@ void sect_AlignPC(uint8_t alignment, uint16_t offset) {
 			    actualOffset
 			);
 		}
-	} else if (uint32_t actualOffset = (sect->alignOfs + curOffset) % alignSize;
+	} else if (uint32_t actualOffset = (sect->alignOfs + curOffset) & alignMask;
-	           sect->align != 0 && actualOffset % sectAlignSize != offset % sectAlignSize) {
+	           sect->align != 0 && (actualOffset & sectAlignMask) != (offset & sectAlignMask)) {
 		error(
 		    "Section is misaligned ($%04" PRIx32 " bytes into the section, expected ALIGN[%" PRIu32
 		    ", %" PRIu32 "], got ALIGN[%" PRIu32 ", %" PRIu32 "])",
@@ -672,8 +668,8 @@ void sect_AlignPC(uint8_t alignment, uint16_t offset) {
 		sect->org = offset - curOffset;
 	} else if (alignment > sect->align) {
 		sect->align = alignment;
-		// We need `(sect->alignOfs + curOffset) % alignSize == offset`
+		// We need `(sect->alignOfs + curOffset) & alignMask == offset`
-		sect->alignOfs = (offset - curOffset) % alignSize;
+		sect->alignOfs = (offset - curOffset) & alignMask;
 	}
 }
 
@@ -948,12 +944,11 @@ bool sect_BinaryFile(std::string const &name, uint32_t startPos) {
 		// The file is seekable; skip to the specified start position
 		fseek(file, startPos, SEEK_SET);
 	} else {
+		// LCOV_EXCL_START
 		if (errno != ESPIPE) {
-			// LCOV_EXCL_START
 			error(
 			    "Error determining size of `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno)
 			);
-			// LCOV_EXCL_STOP
 		}
 		// The file isn't seekable, so we'll just skip bytes one at a time
 		while (startPos--) {
@@ -964,6 +959,7 @@ bool sect_BinaryFile(std::string const &name, uint32_t startPos) {
 				return false;
 			}
 		}
+		// LCOV_EXCL_STOP
 	}
 
 	for (int byte; (byte = fgetc(file)) != EOF;) {
@@ -1013,12 +1009,11 @@ bool sect_BinaryFileSlice(std::string const &name, uint32_t startPos, uint32_t l
 		// The file is seekable; skip to the specified start position
 		fseek(file, startPos, SEEK_SET);
 	} else {
+		// LCOV_EXCL_START
 		if (errno != ESPIPE) {
-			// LCOV_EXCL_START
 			error(
 			    "Error determining size of `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno)
 			);
-			// LCOV_EXCL_STOP
 		}
 		// The file isn't seekable, so we'll just skip bytes one at a time
 		while (startPos--) {
@@ -1029,21 +1024,22 @@ bool sect_BinaryFileSlice(std::string const &name, uint32_t startPos, uint32_t l
 				return false;
 			}
 		}
+		// LCOV_EXCL_STOP
 	}
 
 	while (length--) {
 		if (int byte = fgetc(file); byte != EOF) {
 			writeByte(byte);
-		} else if (ferror(file)) {
 			// LCOV_EXCL_START
+		} else if (ferror(file)) {
 			error("Error reading `INCBIN` file \"%s\": %s", name.c_str(), strerror(errno));
-			// LCOV_EXCL_STOP
 		} else {
 			error(
 			    "Premature end of `INCBIN` file \"%s\" (%" PRId32 " bytes left to read)",
 			    name.c_str(),
 			    length + 1
 			);
+			// LCOV_EXCL_STOP
 		}
 	}
 	return false;

src/link/layout.cpp

@@ -128,6 +128,7 @@ void layout_AlignTo(uint32_t alignment, uint32_t alignOfs) {
 			layout_SetAddr(floatingAlignOffset);
 		} else {
 			uint32_t alignSize = 1u << alignment;
+			uint32_t alignMask = alignSize - 1;
 
 			if (alignOfs >= alignSize) {
 				scriptError(
@@ -139,8 +140,8 @@ void layout_AlignTo(uint32_t alignment, uint32_t alignOfs) {
 				return;
 			}
 
-			floatingAlignMask = alignSize - 1;
+			floatingAlignMask = alignMask;
-			floatingAlignOffset = alignOfs % alignSize;
+			floatingAlignOffset = alignOfs & alignMask;
 		}
 		return;
 	}
@@ -158,6 +159,7 @@ void layout_AlignTo(uint32_t alignment, uint32_t alignOfs) {
 
 	if (alignment < 16) {
 		uint32_t alignSize = 1u << alignment;
+		uint32_t alignMask = alignSize - 1;
 
 		if (alignOfs >= alignSize) {
 			scriptError(
@@ -170,7 +172,7 @@ void layout_AlignTo(uint32_t alignment, uint32_t alignOfs) {
 		}
 
 		assume(pc >= typeInfo.startAddr);
-		length %= alignSize;
+		length &= alignMask;
 	}
 
 	if (uint16_t offset = pc - typeInfo.startAddr; length > typeInfo.size - offset) {

src/link/section.cpp

@@ -54,7 +54,7 @@ static void checkAgainstFixedAddress(Section const &target, Section const &other
 	}
 }
 
-static bool checkAgainstFixedAlign(Section const &target, Section const &other, int32_t ofs) {
+static bool checkAgainstFixedAlign(Section const &target, Section const &other, uint32_t ofs) {
 	if (target.isAddressFixed) {
 		if ((target.org - ofs) & other.alignMask) {
 			fatalTwoAt(
@@ -107,10 +107,7 @@ static void checkFragmentCompat(Section &target, Section &other) {
 		target.isAddressFixed = true;
 		target.org = org;
 	} else if (other.isAlignFixed) {
-		int32_t ofs = (other.alignOfs - target.size) % (other.alignMask + 1);
+		uint32_t ofs = (other.alignOfs - target.size) & other.alignMask;
-		if (ofs < 0) {
-			ofs += other.alignMask + 1;
-		}
 		if (checkAgainstFixedAlign(target, other, ofs)) {
 			target.isAlignFixed = true;
 			target.alignMask = other.alignMask;

test/asm/deprecated-functions.asm

@@ -4,12 +4,25 @@ def s equs "Hello world!"
 assert strin(#s, "l") == strfind(#s, "l") + 1
 assert strrin(#s, "l") == strrfind(#s, "l") + 1
 
-assert !strcmp(strsub(#s, 7), strslice(#s, 6))
+assert strsub(#s, 7) === strslice(#s, 6)
-assert !strcmp(strsub(#s, 7, 5), strslice(#s, 6, 11))
+assert strsub(#s, 7, 5) === strslice(#s, 6, 11)
-assert !strcmp(strsub(#s, strlen(#s), 1), strslice(#s, strlen(#s) - 1, strlen(#s)))
+assert strsub(#s, strlen(#s), 1) === strslice(#s, strlen(#s) - 1, strlen(#s))
-assert !strcmp(strsub(#s, 7, 999), strslice(#s, 6, 999))
+assert strsub(#s, 7, 999) === strslice(#s, 6, 999)
 
-assert !strcmp(charsub(#s, 12), strchar(#s, 11))
+assert charsub(#s, 12) === strchar(#s, 11)
-assert !strcmp(charsub(#s, -1), strchar(#s, -1))
+assert charsub(#s, -1) === strchar(#s, -1)
-assert !strcmp(charsub(#s, -999), strchar(#s, -999))
+assert charsub(#s, -999) === strchar(#s, -999)
-assert !strcmp(charsub(#s, 999), strchar(#s, 999))
+assert charsub(#s, 999) === strchar(#s, 999)
+
+; characters:
+; 1: U+0061 a
+; 2: U+00E4 a with diaresis (0xC3 0xA4)
+; 3: U+0062 b
+; 4: invalid byte 0xA3
+; 5: U+0063 c
+; 6: incomplete U+6F22 kanji (0xE6 0xBC without 0xA2)
+def invalid equs "aäb<C3A4>c<EFBFBD><63>"
+
+def copy equs strsub(#invalid, 1)
+def past equs strsub(#invalid, 9, 1)
+def incomplete equs strsub(#invalid, 12, 1)

test/asm/deprecated-functions.err

@@ -30,3 +30,26 @@ warning: CHARSUB: Position 999 is past the end of the string [-Wbuiltin-args]
     at deprecated-functions.asm(15)
 warning: STRCHAR: Index 999 is past the end of the string [-Wbuiltin-args]
     at deprecated-functions.asm(15)
+warning: `STRSUB` is deprecated; use 0-indexed `STRSLICE` instead [-Wobsolete]
+    at deprecated-functions.asm(26)
+error: STRSUB: Invalid UTF-8 byte 0xA3
+    at deprecated-functions.asm(26)
+error: STRSUB: Incomplete UTF-8 character
+    at deprecated-functions.asm(26)
+warning: `STRSUB` is deprecated; use 0-indexed `STRSLICE` instead [-Wobsolete]
+    at deprecated-functions.asm(27)
+error: STRSUB: Invalid UTF-8 byte 0xA3
+    at deprecated-functions.asm(27)
+warning: STRSUB: Position 9 is past the end of the string [-Wbuiltin-args]
+    at deprecated-functions.asm(27)
+error: STRSUB: Incomplete UTF-8 character
+    at deprecated-functions.asm(27)
+warning: `STRSUB` is deprecated; use 0-indexed `STRSLICE` instead [-Wobsolete]
+    at deprecated-functions.asm(28)
+error: STRSUB: Invalid UTF-8 byte 0xA3
+    at deprecated-functions.asm(28)
+warning: STRSUB: Position 12 is past the end of the string [-Wbuiltin-args]
+    at deprecated-functions.asm(28)
+error: STRSUB: Incomplete UTF-8 character
+    at deprecated-functions.asm(28)
+Assembly aborted with 6 errors!

test/asm/incbin-start-bad.asm

@@ -0,0 +1,4 @@
+SECTION "Bad", ROM0
+
+INCBIN "data.bin", 999
+INCBIN "data.bin", 999, 1

test/asm/incbin-start-bad.err

@@ -0,0 +1,5 @@
+error: Specified start position is greater than length of file "data.bin"
+    at incbin-start-bad.asm(3)
+error: Specified start position is greater than length of file "data.bin"
+    at incbin-start-bad.asm(4)
+Assembly aborted with 2 errors!

test/link/section-fragment/align-conflict.asm

@@ -0,0 +1,10 @@
+IF !DEF(SECOND)
+	def ATTRS equs ",ALIGN[2]"
+ELSE
+	def ATTRS equs "[$1337]"
+ENDC
+
+SECTION FRAGMENT "conflicting alignment", ROM0 {ATTRS}
+	db
+
+	PURGE ATTRS

test/link/section-fragment/align-conflict.out

@@ -0,0 +1,10 @@
+FATAL: Section "conflicting alignment" is defined with 4-byte alignment (offset 0), but also with address $1337
+    at section-fragment/align-conflict.asm(7)
+    and also:
+    at section-fragment/align-conflict.asm(7)
+Linking aborted with 1 error
+---
+error: Section already declared as aligned to 4 bytes (offset 0)
+    at <stdin>(18)
+FATAL: Cannot create section "conflicting alignment" (1 error)
+    at <stdin>(18)

test/link/test.sh

@@ -401,7 +401,7 @@ tryDiff "$test"/out.err "$outtemp"
 tryCmpRom "$test"/ref.out.bin
 evaluateTest
 
-for i in section-union/*.asm; do
+for i in section-union/*.asm section-fragment/*.asm; do
 	test=${i%.asm}
 	startTest
 	"$RGBASM" -o "$otemp" "${test}.asm"