shithub: rgbds

--- a/src/link/patch.c

+++ b/src/link/patch.c

@@ -62,9 +62,17 @@

 	return (uint32_t)value << shiftamt;

-/* This is an "empty"-type stack */

+/*

+ * This is an "empty"-type stack. Apart from the actual values, we also remember

+ * whether the value is a placeholder inserted for error recovery. This allows

+ * us to avoid cascading errors.

+ *

+ * The best way to think about this is a stack of (value, errorFlag) pairs.

+ * They are only separated for reasons of memory efficiency.

+ */

 struct RPNStack {

-	int32_t *buf;

+	int32_t *values;

+	bool *errorFlags;

 	size_t size;

 	size_t capacity;

 } stack;

@@ -72,8 +80,9 @@

 static inline void initRPNStack(void)

 	stack.capacity = 64;

-	stack.buf = malloc(sizeof(*stack.buf) * stack.capacity);

-	if (!stack.buf)

+	stack.values = malloc(sizeof(*stack.values) * stack.capacity);

+	stack.errorFlags = malloc(sizeof(*stack.errorFlags) * stack.capacity);

+	if (!stack.values || !stack.errorFlags)

 		err(1, "Failed to init RPN stack");

@@ -82,7 +91,7 @@

 	stack.size = 0;

-static void pushRPN(int32_t value)

+static void pushRPN(int32_t value, bool comesFromError)

 	if (stack.size >= stack.capacity) {

 		static const size_t increase_factor = 2;

@@ -91,21 +100,28 @@

 			errx(1, "Overflow in RPN stack resize");

 		stack.capacity *= increase_factor;

-		stack.buf =

-			realloc(stack.buf, sizeof(*stack.buf) * stack.capacity);

+		stack.values =

+			realloc(stack.values, sizeof(*stack.values) * stack.capacity);

+		stack.errorFlags =

+			realloc(stack.errorFlags, sizeof(*stack.errorFlags) * stack.capacity);

/*

 		 * Static analysis tools complain that the capacity might become

 		 * zero due to overflow, but fail to realize that it's caught by

 		 * the overflow check above. Hence the stringent check below.

*/

-		if (!stack.buf || !stack.capacity)

+		if (!stack.values || !stack.errorFlags || !stack.capacity)

 			err(1, "Failed to resize RPN stack");

-	stack.buf[stack.size] = value;

+	stack.values[stack.size] = value;

+	stack.errorFlags[stack.size] = comesFromError;

 	stack.size++;

+// This flag tracks whether the RPN op that is currently being evaluated

+// has popped any values with the error flag set.

+static bool isError = false;

 static int32_t popRPN(struct FileStackNode const *node, uint32_t lineNo)

 	if (stack.size == 0)

@@ -112,12 +128,14 @@

 		fatal(node, lineNo, "Internal error, RPN stack empty");

 	stack.size--;

-	return stack.buf[stack.size];

+	isError |= stack.errorFlags[stack.size];

+	return stack.values[stack.size];

 static inline void freeRPNStack(void)

-	free(stack.buf);

+	free(stack.values);

+	free(stack.errorFlags);

 /* RPN operators */

@@ -149,6 +167,8 @@

  * @param patch The patch to compute the value of

  * @param section The section the patch is contained in

  * @return The patch's value

+ * @return isError Set if an error occurred during evaluation, and further

+ *                 errors caused by the value should be suppressed.

*/

 static int32_t computeRPNExpr(struct Patch const *patch,

 			      struct Symbol const * const *fileSymbols)

@@ -166,6 +186,8 @@

 						     patch->src, patch->lineNo);

 		int32_t value;

+		isError = false;

/*

 		 * Friendly reminder:

 		 * Be VERY careful with two `popRPN` in the same expression.

@@ -191,7 +213,9 @@

 		case RPN_DIV:

 			value = popRPN();

 			if (value == 0) {

-				error(patch->src, patch->lineNo, "Division by 0");

+				if (!isError)

+					error(patch->src, patch->lineNo, "Division by 0");

+				isError = true;

 				popRPN();

 				value = INT32_MAX;

 			} else {

@@ -201,7 +225,9 @@

 		case RPN_MOD:

 			value = popRPN();

 			if (value == 0) {

-				error(patch->src, patch->lineNo, "Modulo by 0");

+				if (!isError)

+					error(patch->src, patch->lineNo, "Modulo by 0");

+				isError = true;

 				popRPN();

 				value = 0;

 			} else {

@@ -280,11 +306,13 @@

 				error(patch->src, patch->lineNo,

 				      "Requested BANK() of symbol \"%s\", which was not found",

 				      fileSymbols[value]->name);

+				isError = true;

 				value = 1;

 			} else if (!symbol->section) {

 				error(patch->src, patch->lineNo,

 				      "Requested BANK() of non-label symbol \"%s\"",

 				      fileSymbols[value]->name);

+				isError = true;

 				value = 1;

 			} else {

 				value = symbol->section->bank;

@@ -302,6 +330,7 @@

 				error(patch->src, patch->lineNo,

 				      "Requested BANK() of section \"%s\", which was not found",

 				      name);

+				isError = true;

 				value = 1;

 			} else {

 				value = sect->bank;

@@ -312,6 +341,7 @@

 			if (!patch->pcSection) {

 				error(patch->src, patch->lineNo,

 				      "PC has no bank outside a section");

+				isError = true;

 				value = 1;

 			} else {

 				value = patch->pcSection->bank;

@@ -320,11 +350,13 @@

 		case RPN_HRAM:

 			value = popRPN();

-			if (value < 0

-			 || (value > 0xFF && value < 0xFF00)

-			 || value > 0xFFFF)

+			if (!isError && (value < 0

+				     || (value > 0xFF && value < 0xFF00)

+				     || value > 0xFFFF)) {

 				error(patch->src, patch->lineNo,

 				      "Value %" PRId32 " is not in HRAM range", value);

+				isError = true;

+			}

 			value &= 0xFF;

 			break;

@@ -333,9 +365,12 @@

 			/* Acceptable values are 0x00, 0x08, 0x10, ..., 0x38

 			 * They can be easily checked with a bitmask

*/

-			if (value & ~0x38)

-				error(patch->src, patch->lineNo,

-				      "Value %" PRId32 " is not a RST vector", value);

+			if (value & ~0x38) {

+				if (!isError)

+					error(patch->src, patch->lineNo,

+					      "Value %" PRId32 " is not a RST vector", value);

+				isError = true;

+			}

 			value |= 0xC7;

 			break;

@@ -357,6 +392,7 @@

 					error(patch->src, patch->lineNo,

 					      "PC has no value outside a section");

 					value = 0;

+					isError = true;

 				} else {

 					value = patch->pcOffset + patch->pcSection->org;

@@ -366,6 +402,7 @@

 				if (!symbol) {

 					error(patch->src, patch->lineNo,

 					      "Unknown symbol \"%s\"", fileSymbols[value]->name);

+					isError = true;

 				} else {

 					value = symbol->value;

 					/* Symbols attached to sections have offsets */

@@ -376,7 +413,7 @@

 			break;

-		pushRPN(value);

+		pushRPN(value, isError);

 	if (stack.size > 1)

@@ -383,6 +420,7 @@

 		error(patch->src, patch->lineNo,

 		      "RPN stack has %zu entries on exit, not 1", stack.size);

+	isError = false;

 	return popRPN();

 #undef popRPN

@@ -394,10 +432,12 @@

 	initRPNStack();

 	while (assert) {

-		if (!computeRPNExpr(&assert->patch,

-				    (struct Symbol const * const *)

-							assert->fileSymbols)) {

-			switch ((enum AssertionType)assert->patch.type) {

+		int32_t value = computeRPNExpr(&assert->patch,

+			(struct Symbol const * const *)assert->fileSymbols);

+		enum AssertionType type = assert->patch.type;

+		if (!isError && !value) {

+			switch (type) {

 			case ASSERT_FATAL:

 				fatal(assert->patch.src, assert->patch.lineNo, "%s",

 				      assert->message[0] ? assert->message

@@ -415,6 +455,11 @@

 							   : "assert failure");

 				break;

+		} else if (isError && type == ASSERT_FATAL) {

+			fatal(assert->patch.src, assert->patch.lineNo,

+			      "couldn't evaluate assertion%s%s",

+			      assert->message[0] ? ": " : "",

+			      assert->message);

 		struct Assertion *next = assert->next;

@@ -450,7 +495,7 @@

 							+ patch->pcOffset + 1;

 			int16_t jumpOffset = value - address;

-			if (jumpOffset < -128 || jumpOffset > 127)

+			if (!isError && (jumpOffset < -128 || jumpOffset > 127))

 				error(patch->src, patch->lineNo,

 				      "jr target out of reach (expected -129 < %" PRId16 " < 128)",

 				      jumpOffset);

@@ -467,8 +512,8 @@

 				[PATCHTYPE_LONG] = {4, INT32_MIN, INT32_MAX}

};

-			if (value < types[patch->type].min

-			 || value > types[patch->type].max)

+			if (!isError && (value < types[patch->type].min

+				      || value > types[patch->type].max))

 				error(patch->src, patch->lineNo,

 				      "Value %#" PRIx32 "%s is not %u-bit",

 				      value, value < 0 ? " (maybe negative?)" : "",

--- /dev/null

+++ b/test/link/cascading-errors-fatal-assert.asm

@@ -1,0 +1,2 @@

+	assert FATAL, UnknownSymbol == 42

+	assert WeDontReachHere

--- /dev/null

+++ b/test/link/cascading-errors-fatal-assert.out

@@ -1,0 +1,3 @@

+error: cascading-errors-fatal-assert.asm(1): Unknown symbol "UnknownSymbol"

+fatal: cascading-errors-fatal-assert.asm(1): couldn't evaluate assertion

+Linking aborted after 2 errors

--- /dev/null

+++ b/test/link/cascading-errors.asm

@@ -1,0 +1,19 @@

+SECTION "zero", ROM0[$0]

+Zero:

+; Pin the section such that a jr to 0 is out of range

+SECTION "test", ROM0[$1000]

+	;; XXX: the fallback value used is the index of the symbol (in the object file?)

+	;; Is this intended?

+	dw Bar

+	dw Foo / Bar

+	dw Foo / Zero

+	rst Foo

+	jr NonExist

+	ldh a, [hNonExist + $200]

+	assert Foo == 42

+	assert WARN, Bar == 42

--- /dev/null

+++ b/test/link/cascading-errors.out

@@ -1,0 +1,11 @@

+error: cascading-errors.asm(18): Unknown symbol "Foo"

+error: cascading-errors.asm(19): Unknown symbol "Bar"

+error: cascading-errors.asm(16): Unknown symbol "hNonExist"

+error: cascading-errors.asm(14): Unknown symbol "NonExist"

+error: cascading-errors.asm(12): Unknown symbol "Foo"

+error: cascading-errors.asm(10): Unknown symbol "Foo"

+error: cascading-errors.asm(10): Division by 0

+error: cascading-errors.asm(9): Unknown symbol "Foo"

+error: cascading-errors.asm(9): Unknown symbol "Bar"

+error: cascading-errors.asm(8): Unknown symbol "Bar"

+Linking failed with 10 errors