shithub: rgbds

Info  •  Files  •  Log  •  Branches

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

--- a/include/asm/warning.h

+++ b/include/asm/warning.h

@@ -48,7 +48,7 @@

  * 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 @@

  * 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 @@

  * 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

--- a/src/asm/lexer.c

+++ b/src/asm/lexer.c

@@ -746,7 +746,7 @@

 #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 @@

 	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) \

--- a/src/asm/parser.y

+++ b/src/asm/parser.y

@@ -276,16 +276,11 @@

 			dest[i++] = '%';

 			a++;

 			continue;

-		} else if (a == nbArgs) {

-			error("STRFMT: Not enough arguments for format spec\n", a + 1);

+		} else if (a >= nbArgs) {

+			// Will warn after formatting is done.

 			dest[i++] = '%';

 			a++;

 			continue;

-		} else if (a > nbArgs) {

-			// already warned for a == nbArgs

-			dest[i++] = '%';

-			a++;

-			continue;

 		}

 		struct StrFmtArg *arg = &args[a++];

@@ -301,6 +296,8 @@

 	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 @@

 void yyerror(char const *str)

 {

-	size_t len = strlen(str);

-	char *buf = malloc(len + 2);

-

-	memcpy(buf, str, len);

-	buf[len] = '\n';

-	buf[len + 1] = '\0';

-	error(buf);

-	free(buf);

+	error("%s\n", str);

 }

 // The CPU encodes instructions in a logical way, so most instructions actually follow patterns.

--- a/src/asm/symbol.c

+++ b/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 @@

 		 * 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] != '.')

-			error("Not currently in the scope of '%.*s'\n", parentLen, name);

+		if (labelScope[i] != '\0' || name[i] != '.') {

+			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 @@

 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];

--- a/test/asm/strfmt.err

+++ b/test/asm/strfmt.err

@@ -7,5 +7,5 @@

 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)!