Fix error-related issues (#773)

* Mark `error` as a `format` function, to properly scan its format

* Fix the call to error() from parser.y:
  - Use '%s' to avoid passing an arbitrary format
  - Simplify yyerror overall

* Fix size parameter of %.*s format being an int... bonkers standard.

* Report the number of arguments required and provided on a STRFMT mismatch

* Add an assert to check for a very unlikely bug

include/asm/warning.h

@@ -48,7 +48,7 @@ void processWarningFlag(char const *flag);
  * Used to warn the user about problems that don't prevent the generation of
  * valid code.
  */
-void warning(enum WarningID id, const char *fmt, ...);
+void warning(enum WarningID id, const char *fmt, ...) format_(printf, 2, 3);
 
 /*
  * Used for errors that compromise the whole assembly process by affecting the
@@ -57,7 +57,7 @@ void warning(enum WarningID id, const char *fmt, ...);
  * It is also used when the assembler goes into an invalid state (for example,
  * when it fails to allocate memory).
  */
-_Noreturn void fatalerror(const char *fmt, ...);
+_Noreturn void fatalerror(const char *fmt, ...) format_(printf, 1, 2);
 
 /*
  * Used for errors that make it impossible to assemble correctly, but don't
@@ -65,6 +65,6 @@ _Noreturn void fatalerror(const char *fmt, ...);
  * get a list of all errors at the end, making it easier to fix all of them at
  * once.
  */
-void error(const char *fmt, ...);
+void error(const char *fmt, ...) format_(printf, 1, 2);
 
 #endif

src/asm/lexer.c

@@ -746,7 +746,7 @@ static void beginExpansion(size_t distance, uint8_t skip,
 #define LOOKUP_PRE_NEST(exp) (exp)->totalLen += size - skip
 #define LOOKUP_POST_NEST(exp) do { \
 	if (name && ++depth >= nMaxRecursionDepth) \
-		fatalerror("Recursion limit (%u) exceeded\n", nMaxRecursionDepth); \
+		fatalerror("Recursion limit (%zu) exceeded\n", nMaxRecursionDepth); \
 } while (0)
 	lookupExpansion(parent, distance);
 #undef LOOKUP_PRE_NEST
@@ -851,7 +851,7 @@ static int peekInternal(uint8_t distance)
 	assert(writeIndex + (size) <= LEXER_BUF_SIZE); \
 	nbCharsRead = read(lexerState->fd, &lexerState->buf[writeIndex], (size)); \
 	if (nbCharsRead == -1) \
-		fatalerror("Error while reading \"%s\": %s\n", lexerState->path, errno); \
+		fatalerror("Error while reading \"%s\": %s\n", lexerState->path, strerror(errno)); \
 	totalCharsRead += nbCharsRead; \
 	writeIndex += nbCharsRead; \
 	if (writeIndex == LEXER_BUF_SIZE) \

src/asm/parser.y

@@ -276,13 +276,8 @@ static void strfmt(char *dest, size_t destLen, char const *fmt, size_t nbArgs, s
 			dest[i++] = '%';
 			a++;
 			continue;
-		} else if (a == nbArgs) {
+		} else if (a >= nbArgs) {
-			error("STRFMT: Not enough arguments for format spec\n", a + 1);
+			// Will warn after formatting is done.
-			dest[i++] = '%';
-			a++;
-			continue;
-		} else if (a > nbArgs) {
-			// already warned for a == nbArgs
 			dest[i++] = '%';
 			a++;
 			continue;
@@ -301,6 +296,8 @@ static void strfmt(char *dest, size_t destLen, char const *fmt, size_t nbArgs, s
 
 	if (a < nbArgs)
 		error("STRFMT: %zu unformatted argument(s)\n", nbArgs - a);
+	else if (a > nbArgs)
+		error("STRFMT: Not enough arguments for format spec, got: %zu, need: %zu\n", nbArgs, a);
 
 	if (i > destLen - 1) {
 		warning(WARNING_LONG_STR, "STRFMT: String too long, got truncated\n");
@@ -366,14 +363,7 @@ static inline void failAssertMsg(enum AssertionType type, char const *msg)
 
 void yyerror(char const *str)
 {
-	size_t len = strlen(str);
+	error("%s\n", str);
-	char *buf = malloc(len + 2);
-
-	memcpy(buf, str, len);
-	buf[len] = '\n';
-	buf[len + 1] = '\0';
-	error(buf);
-	free(buf);
 }
 
 // The CPU encodes instructions in a logical way, so most instructions actually follow patterns.

src/asm/symbol.c

@@ -13,6 +13,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
@@ -537,8 +538,10 @@ struct Symbol *sym_AddLocalLabel(char const *name)
 		 * Check that `labelScope[i]` ended the check, guaranteeing that `name` is at least
 		 * as long, and then that this was the entirety of the `Parent` part of `name`.
 		 */
-		if (labelScope[i] != '\0' || name[i] != '.')
+		if (labelScope[i] != '\0' || name[i] != '.') {
-			error("Not currently in the scope of '%.*s'\n", parentLen, name);
+			assert(parentLen <= INT_MAX);
+			error("Not currently in the scope of '%.*s'\n", (int)parentLen, name);
+		}
 		if (strchr(&name[parentLen + 1], '.')) /* There will at least be a terminator */
 			fatalerror("'%s' is a nonsensical reference to a nested local label\n",
 				   name);
@@ -568,7 +571,7 @@ static uint32_t anonLabelID;
 struct Symbol *sym_AddAnonLabel(void)
 {
 	if (anonLabelID == UINT32_MAX) {
-		error("Only %" PRIu32 " anonymous labels can be created!");
+		error("Only %" PRIu32 " anonymous labels can be created!", anonLabelID);
 		return NULL;
 	}
 	char name[MAXSYMLEN + 1];

test/asm/strfmt.err

@@ -7,5 +7,5 @@ ERROR: strfmt.asm(22):
 ERROR: strfmt.asm(24):
     STRFMT: Invalid format spec for argument 1
 ERROR: strfmt.asm(26):
-    STRFMT: Not enough arguments for format spec
+    STRFMT: Not enough arguments for format spec, got: 1, need: 3
 error: Assembly aborted (5 errors)!