Fix recursive function call that shouldnot be recursive.

Actually add the section to the smart link queue.
Fix various smart linking issues
Smart link after the linking script.
Keep fully constrained sections with smart linking.
Fix modifing the section hash map while you are iterating over it

Remove the ffpad from the smart linking tests, it does not add anything while making the test script more complex and failing on Mac
Fix some smart linking tests and remove some obviously broken/wrong ones.
Add smart linking test to check if chained references are kept
Remove -x link flag, as we cannot test everything we want with that
Add a test to check if constrained sections are kept
Add wram smart linking test, which will fail.

Author: daid

src/link/assign.c

@@ -400,6 +400,10 @@ void assign_AssignSections(void)
 	/* Process linker script, if any */
 	processLinkerScript();
 
+	/* After the linker script check if we need to do smart linking
+	   and discard any sections */
+	sect_PerformSmartLink();
+
 	nbSectionsToAssign = 0;
 	sect_ForEach(categorizeSection, NULL);
 

src/link/main.c

@@ -296,7 +296,6 @@ int main(int argc, char *argv[])
 
 	/* then process them, */
 	obj_DoSanityChecks();
-	sect_PerformSmartLink();
 	assign_AssignSections();
 	obj_CheckAssertions();
 	assign_Cleanup();

src/link/object.c

@@ -382,6 +382,10 @@ static void readSection(FILE *file, struct Section *section, char const *fileNam
 				  i, fileSections, fileNodes);
 		}
 		section->patches = patches;
+	} else {
+		section->data = NULL;
+		section->nbPatches = 0;
+		section->patches = NULL;
 	}
 
 	section->smartLinked = false;

src/link/patch.c

@@ -567,6 +567,7 @@ void patch_FindRefdSections(struct Patch const *patch, void (*callback)(struct S
 			break;
 
 		case RPN_BANK_SYM:
+			symbolID = 0;
 			for (uint8_t shift = 0; shift < 32; shift += 8)
 				symbolID |= getRPNByte(&expression, &size,
 						    patch->src, patch->lineNo) << shift;
@@ -603,7 +604,10 @@ void patch_FindRefdSections(struct Patch const *patch, void (*callback)(struct S
 		case RPN_BANK_SELF:
 		case RPN_HRAM:
 		case RPN_RST:
+			break;
 		case RPN_CONST:
+			for (uint8_t shift = 0; shift < 32; shift += 8)
+				getRPNByte(&expression, &size, patch->src, patch->lineNo);
 			break;
 
 		case RPN_SYM:

src/link/section.c

@@ -368,22 +368,34 @@ void sect_AddSmartSection(char const *name)
 	nbSmartLinkNames++;
 }
 
+// Actually a stack, not a queue; however, the order in which sections are processed doesn't matter
+struct Section **smartLinkQueue;
+size_t queueSize = 0;
+size_t queueCapacity;
+
 static void smartSectionPurge(void *arg, void *ign)
 {
 	(void)ign;
 	struct Section *sect = arg;
 
 	if (!sect->smartLinked) {
 		verbosePrint("Dropping \"%s\" due to smart linking\n", sect->name);
-		sect_RemoveSection(sect->name);
+
+		if (queueSize == queueCapacity) {
+			if (queueCapacity == SIZE_MAX)
+				error(NULL, 0, "Smart linking queue capacity exceeded!");
+			else if (queueCapacity > SIZE_MAX / 2)
+				queueCapacity = SIZE_MAX;
+			else
+				queueCapacity *= 2;
+			smartLinkQueue = realloc(smartLinkQueue, sizeof(*smartLinkQueue) * queueCapacity);
+		}
+
+		smartLinkQueue[queueSize] = sect;
+		queueSize++;
 	}
 }
 
-// Actually a stack, not a queue; however, the order in which sections are processed doesn't matter
-struct Section **smartLinkQueue;
-size_t queueSize = 0;
-size_t queueCapacity;
-
 static void queueSmartSection(struct Section *sect)
 {
 	if (queueSize == queueCapacity) {
@@ -396,7 +408,12 @@ static void queueSmartSection(struct Section *sect)
 		smartLinkQueue = realloc(smartLinkQueue, sizeof(*smartLinkQueue) * queueCapacity);
 	}
 
-	sect->smartLinked = true;
+	if (!sect->smartLinked) {
+		sect->smartLinked = true;
+
+		smartLinkQueue[queueSize] = sect;
+		queueSize++;
+	}
 }
 
 void sect_LinkSection(struct Section const *sect)
@@ -406,6 +423,19 @@ void sect_LinkSection(struct Section const *sect)
 		patch_FindRefdSections(&sect->patches[i], queueSmartSection, (struct Symbol const * const *)sect->fileSymbols);
 }
 
+static void smartLinkConstrained(void *arg, void *ign)
+{
+	(void)ign;
+
+	struct Section *sect = arg;
+	/* if a section isn't smart linked yet, but is fully constrained
+	   by address and bank, we want to smart link it as well */
+	if (!sect->smartLinked && sect->isAddressFixed && sect->isBankFixed) {
+		sect->smartLinked = true;
+		sect_LinkSection(sect);
+	}
+}
+
 void sect_PerformSmartLink(void)
 {
 	// If smart linking wasn't requested, do nothing
@@ -414,7 +444,7 @@ void sect_PerformSmartLink(void)
 
 	// Assume that each section is going to link a new one...
 	queueCapacity = nbSmartLinkNames;
-	smartLinkQueue = malloc(queueSize * sizeof(*smartLinkQueue));
+	smartLinkQueue = malloc(queueCapacity * sizeof(*smartLinkQueue));
 	if (!smartLinkQueue)
 		error(NULL, 0,  "Smart linking allocation failed: %s", strerror(errno));
 
@@ -433,6 +463,8 @@ void sect_PerformSmartLink(void)
 	}
 	free(smartLinkNames);
 
+	hash_ForEach(sections, smartLinkConstrained, NULL);
+
 	// Also add sections referenced by assertions
 	struct Assertion const *assertion = obj_GetFirstAssertion();
 
@@ -444,7 +476,12 @@ void sect_PerformSmartLink(void)
 	// As long as the queue isn't empty, get the last one, and process it
 	while (queueSize)
 		sect_LinkSection(smartLinkQueue[--queueSize]);
-	free(smartLinkQueue);
 
+	// Find each section that needs to be removed and put it in the smartLinkQueue
 	hash_ForEach(sections, smartSectionPurge, NULL);
+
+	// Remove all entries that need to be purged. (cannot be done during hash_ForEach)
+	while (queueSize)
+		sect_RemoveSection(smartLinkQueue[--queueSize]->name);
+	free(smartLinkQueue);
 }

src/link/symbol.c

@@ -67,7 +67,7 @@ struct Symbol *sym_GetSymbol(char const *name)
 
 void sym_RemoveSymbol(char const *name)
 {
-	sym_RemoveSymbol(hash_RemoveElement(symbols, name));
+	hash_RemoveElement(symbols, name);
 }
 
 void sym_CleanupSymbols(void)

test/link/smart/assert.asm

@@ -3,14 +3,14 @@ SECTION "root", ROM0[0]
 
 	db Label
 
-SECTION "A", ROM0[1]
+SECTION "A", ROM0
 
 Label:
 	db 2, Label
 
 assert Unrefd == 3 ; The assertion will reference section "B"
 
-SECTION "B", ROM0[3]
+SECTION "B", ROM0
 
 Unrefd:
 	db Unrefd ; References itself, but unless referenced externally, that doesn't matter

test/link/smart/assert.bin

@@ -0,0 +1,16 @@
+SECTION "root", ROM0[0]
+    dw WRAMLabel
+
+; This section should be kept thanks to the reference from the WRAM section
+SECTION "A", ROM0
+Label:
+    db $01, $02
+.end:
+
+SECTION "wram", WRAM0
+WRAMLabel:
+    ds Label.end - Label
+
+SECTION "UnRef", ROM0
+UnRef:
+    db UnRef

test/link/smart/assert.smart.bin

@@ -0,0 +1,23 @@
+; If multiple sections are chained together by references
+; they all should be kept if they can be traced back to a root
+SECTION "root", ROM0[0]
+    db LabelA
+
+SECTION "A", ROM0
+LabelA:
+    db LabelB
+    ds 3, $EE
+
+SECTION "B", ROM0
+LabelB:
+    db LabelC
+    ds 2, $EE
+
+SECTION "C", ROM0
+LabelC:
+    db $ff
+    ds 1, $EE
+
+SECTION "unref", ROM0
+UnRef:
+    db UnRef

test/link/smart/chain-wram.asm

@@ -3,14 +3,14 @@ SECTION "root", ROM0[0]
 
 	db Label
 
-SECTION "A", ROM0[1]
+SECTION "A", ROM0
 
 Label:
 	db 2, Label
 
-SECTION "B", ROM0[3]
+SECTION "B", ROM0
 
 Unrefd:
 	db Unrefd ; References itself, but unless referenced externally, that doesn't matter
 
-assert Unrefd == 3 ; The assertion should reference section "B"... but it'll be resolved by RGBASM
+	assert Label == 1 ; This assert will be ignored when smart linking, as nothing references section "B"

test/link/smart/chain-wram.bin

@@ -0,0 +1,15 @@
+; Fully constrained sections need to be kept
+SECTION "root", ROM0[0]
+
+
+; This section is fully constrained and should be kept
+SECTION "01", ROM0[1]
+    db $01
+
+; This section is not fully constrained and should not be kept, bank is unknown.
+SECTION "02", ROMX[$4002]
+    db $02
+
+; This section is fully constrained and should be kept
+SECTION "03", ROMX[$4003], BANK[1]
+    db $03

test/link/smart/chain-wram.smart.bin

@@ -1,5 +1,5 @@
 
-SECTION "A", ROM0[1]
+SECTION "A", ROM0
 
 Label:
 	db 2, Label
@@ -9,7 +9,7 @@ SECTION "root", ROM0[0]
 
 	db Label.end - Label
 
-SECTION "B", ROM0[3]
+SECTION "B", ROM0
 
 Unrefd:
 	db Unrefd

test/link/smart/chain.asm

@@ -3,12 +3,12 @@ SECTION "root", ROM0[0]
 
 	db Label
 
-SECTION "A", ROM0[1]
+SECTION "A", ROM0
 
 Label:
 	db 2, Label
 
-SECTION "B", ROM0[3]
+SECTION "B", ROM0
 
 Unrefd:
 	db Unrefd ; References itself, but unless referenced externally, that doesn't matter

test/link/smart/chain.bin

@@ -97,21 +97,18 @@ for i in *.asm; do
 	fi
 done
 
-ffpad() { # Pads $1 to size $3 into $2 with 0xFF bytes
-	dd if=/dev/zero count=1 ibs=$3 2>/dev/null | tr '\000' '\377' > $2
-	dd if="$1" of=$2 conv=notrunc 2>/dev/null
-}
 for i in smart/*.asm; do
 	startTest
 	$RGBASM -o $otemp $i
-	$RGBLINK -p 0xff -o $gbtemp $otemp
-	SIZE=$(wc -c $gbtemp | cut -d ' ' -f 1)
-	ffpad "${i%.asm}.bin" $gbtemp2 $SIZE
-	tryCmp $gbtemp2 $gbtemp
+	rm $gbtemp
+	$RGBLINK -o $gbtemp $otemp
+	tryCmp "${i%.asm}.bin" $gbtemp
 	rc=$(($? || $rc))
-	$RGBLINK -p 0xff -vs "root" -o $gbtemp $otemp
-	ffpad "${i%.asm}.smart.bin" $gbtemp2 $SIZE
-	tryCmp $gbtemp2 $gbtemp
+	rm $gbtemp
+	i="${i%.asm}.smart"
+	startTest
+	$RGBLINK -vs "root" -o $gbtemp $otemp
+	tryCmp "$i.bin" $gbtemp
 	rc=$(($? || $rc))
 done