rgblink identifies patches' PC sections after reading all sections

Fixes #794

src/link/object.c

@@ -261,7 +261,7 @@ static void readSymbol(FILE *file, struct Symbol *symbol,
  * @param i The number of the patch to report in errors
  */
 static void readPatch(FILE *file, struct Patch *patch, char const *fileName, char const *sectName,
-		      uint32_t i, struct Section *fileSections[], struct FileStackNode fileNodes[])
+		      uint32_t i, struct FileStackNode fileNodes[])
 {
 	uint32_t nodeID;
 	uint8_t type;
@@ -279,8 +279,6 @@ static void readPatch(FILE *file, struct Patch *patch, char const *fileName, cha
 	tryReadlong(patch->pcSectionID, file,
 		    "%s: Unable to read \"%s\"'s patch #%" PRIu32 "'s PC offset: %s",
 		    fileName, sectName, i);
-	patch->pcSection = patch->pcSectionID == (uint32_t)-1 ? NULL
-							      : fileSections[patch->pcSectionID];
 	tryReadlong(patch->pcOffset, file,
 		    "%s: Unable to read \"%s\"'s patch #%" PRIu32 "'s PC offset: %s",
 		    fileName, sectName, i);
@@ -305,6 +303,16 @@ static void readPatch(FILE *file, struct Patch *patch, char const *fileName, cha
 		     feof(file) ? "Unexpected end of file" : strerror(errno));
 }
 
+/**
+ * Sets a patch's pcSection from its pcSectionID.
+ * @param patch The struct to fix
+ */
+static void linkPatchToPCSect(struct Patch *patch, struct Section *fileSections[])
+{
+	patch->pcSection = patch->pcSectionID == (uint32_t)-1 ? NULL
+							      : fileSections[patch->pcSectionID];
+}
+
 /**
  * Reads a section from a file.
  * @param file The file to read from
@@ -312,7 +320,7 @@ static void readPatch(FILE *file, struct Patch *patch, char const *fileName, cha
  * @param fileName The filename to report in errors
  */
 static void readSection(FILE *file, struct Section *section, char const *fileName,
-			struct Section *fileSections[], struct FileStackNode fileNodes[])
+			struct FileStackNode fileNodes[])
 {
 	int32_t tmp;
 	uint8_t byte;
@@ -389,12 +397,9 @@ static void readSection(FILE *file, struct Section *section, char const *fileNam
 			malloc(sizeof(*patches) * section->nbPatches + 1);
 
 		if (!patches)
-			err(1, "%s: Unable to read \"%s\"'s patches", fileName,
+			err(1, "%s: Unable to read \"%s\"'s patches", fileName, section->name);
-			    section->name);
+		for (uint32_t i = 0; i < section->nbPatches; i++)
-		for (uint32_t i = 0; i < section->nbPatches; i++) {
+			readPatch(file, &patches[i], fileName, section->name, i, fileNodes);
-			readPatch(file, &patches[i], fileName, section->name,
-				  i, fileSections, fileNodes);
-		}
 		section->patches = patches;
 	}
 }
@@ -436,13 +441,13 @@ static void linkSymToSect(struct Symbol const *symbol, struct Section *section)
  */
 static void readAssertion(FILE *file, struct Assertion *assert,
 			  char const *fileName, uint32_t i,
-			  struct Section *fileSections[], struct FileStackNode fileNodes[])
+			  struct FileStackNode fileNodes[])
 {
 	char assertName[sizeof("Assertion #4294967295")]; // UINT32_MAX
 
 	snprintf(assertName, sizeof(assertName), "Assertion #%" PRIu32, i);
 
-	readPatch(file, &assert->patch, fileName, assertName, 0, fileSections, fileNodes);
+	readPatch(file, &assert->patch, fileName, assertName, 0, fileNodes);
 	tryReadstr(assert->message, file, "%s: Cannot read assertion's message: %s",
 		   fileName);
 }
@@ -550,7 +555,7 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 			err(1, "%s: Couldn't create new section", fileName);
 
 		fileSections[i]->nextu = NULL;
-		readSection(file, fileSections[i], fileName, fileSections, nodes[fileID].nodes);
+		readSection(file, fileSections[i], fileName, nodes[fileID].nodes);
 		fileSections[i]->fileSymbols = fileSymbols;
 		if (nbSymPerSect[i]) {
 			fileSections[i]->symbols = malloc(nbSymPerSect[i]
@@ -568,7 +573,15 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 
 	free(nbSymPerSect);
 
-	/* Give symbols pointers to their sections */
+	/* Give patches' PC section pointers to their sections */
+	for (uint32_t i = 0; i < nbSections; i++) {
+		if (sect_HasData(fileSections[i]->type)) {
+			for (uint32_t j = 0; j < fileSections[i]->nbPatches; j++)
+				linkPatchToPCSect(&fileSections[i]->patches[j], fileSections);
+		}
+	}
+
+	/* Give symbols' section pointers to their sections */
 	for (uint32_t i = 0; i < nbSymbols; i++) {
 		int32_t sectionID = fileSymbols[i]->sectionID;
 
@@ -600,7 +613,8 @@ void obj_ReadFile(char const *fileName, unsigned int fileID)
 
 		if (!assertion)
 			err(1, "%s: Couldn't create new assertion", fileName);
-		readAssertion(file, assertion, fileName, i, fileSections, nodes[fileID].nodes);
+		readAssertion(file, assertion, fileName, i, nodes[fileID].nodes);
+		linkPatchToPCSect(&assertion->patch, fileSections);
 		assertion->fileSymbols = fileSymbols;
 		assertion->next = assertions;
 		assertions = assertion;

test/link/load-fragment-jr.asm

@@ -0,0 +1,12 @@
+SECTION "main", ROM0
+LOAD FRAGMENT "test", SRAM
+ENDL
+
+; The RPN patch for 'jr Label' in section "alt" refers to section "test",
+; but the object file puts section "test" after section "alt".
+; This case needs to be handled when identifying patches' PC sections.
+SECTION "alt", ROM0
+LOAD FRAGMENT "test", SRAM
+	jr Label
+Label:
+ENDL

test/link/test.sh

@@ -39,7 +39,7 @@ tryCmp () {
 	fi
 }
 
-rgblink() {
+rgblinkQuiet () {
 	out="$(env $RGBLINK "$@")" || return $?
 	if [[ -n "$out" ]]; then
 		echo "$bold${red}Linking shouldn't produce anything on stdout!$rescolors$resbold"
@@ -55,13 +55,13 @@ for i in *.asm; do
 	ran_flag=
 	for flag in '-d' '-t' '-w'; do
 		if [ -f ${i%.asm}-no${flag}.out ]; then
-			rgblink -o $gbtemp $otemp > $outtemp 2>&1
+			rgblinkQuiet -o $gbtemp $otemp > $outtemp 2>&1
 			tryDiff ${i%.asm}-no${flag}.out $outtemp
 			rc=$(($? || $rc))
 			ran_flag=1
 		fi
 		if [ -f ${i%.asm}${flag}.out ]; then
-			rgblink ${flag} -o $gbtemp $otemp > $outtemp 2>&1
+			rgblinkQuiet ${flag} -o $gbtemp $otemp > $outtemp 2>&1
 			tryDiff ${i%.asm}${flag}.out $outtemp
 			rc=$(($? || $rc))
 			ran_flag=1
@@ -73,7 +73,7 @@ for i in *.asm; do
 
 	# Other tests have several linker scripts
 	find . -name "${i%.asm}*.link" | while read script; do
-		rgblink -l $script -o $gbtemp $otemp > $outtemp 2>&1
+		rgblinkQuiet -l $script -o $gbtemp $otemp > $outtemp 2>&1
 		tryDiff ${script%.link}.out $outtemp
 		rc=$(($? || $rc))
 		ran_flag=1
@@ -83,7 +83,7 @@ for i in *.asm; do
 	fi
 
 	# The rest of the tests just links a file, and maybe checks the binary
-	rgblink -o $gbtemp $otemp > $outtemp 2>&1
+	rgblinkQuiet -o $gbtemp $otemp > $outtemp 2>&1
 	if [ -f ${i%.asm}.out ]; then
 		tryDiff ${i%.asm}.out $outtemp
 		rc=$(($? || $rc))
@@ -103,7 +103,7 @@ i="bank-const.asm"
 startTest
 $RGBASM -o $otemp bank-const/a.asm
 $RGBASM -o $gbtemp2 bank-const/b.asm
-rgblink -o $gbtemp $gbtemp2 $otemp > $outtemp 2>&1
+rgblinkQuiet -o $gbtemp $gbtemp2 $otemp > $outtemp 2>&1
 tryDiff bank-const/out.err $outtemp
 rc=$(($? || $rc))
 
@@ -111,7 +111,7 @@ for i in fragment-align/*; do
 	startTest
 	$RGBASM -o $otemp $i/a.asm
 	$RGBASM -o $gbtemp2 $i/b.asm
-	rgblink -o $gbtemp $otemp $gbtemp2 2>$outtemp
+	rgblinkQuiet -o $gbtemp $otemp $gbtemp2 2>$outtemp
 	tryDiff $i/out.err $outtemp
 	rc=$(($? || $rc))
 	if [[ -f $i/out.gb ]]; then
@@ -124,16 +124,16 @@ done
 i="high-low.asm"
 startTest
 $RGBASM -o $otemp high-low/a.asm
-rgblink -o $gbtemp $otemp
+rgblinkQuiet -o $gbtemp $otemp
 $RGBASM -o $otemp high-low/b.asm
-rgblink -o $gbtemp2 $otemp
+rgblinkQuiet -o $gbtemp2 $otemp
 tryCmp $gbtemp $gbtemp2
 rc=$(($? || $rc))
 
 i="overlay.asm"
 startTest
 $RGBASM -o $otemp overlay/a.asm
-rgblink -o $gbtemp -t -O overlay/overlay.gb $otemp > $outtemp 2>&1
+rgblinkQuiet -o $gbtemp -t -O overlay/overlay.gb $otemp > $outtemp 2>&1
 # This test does not trim its output with 'dd' because it needs to verify the correct output size
 tryDiff overlay/out.err $outtemp
 rc=$(($? || $rc))
@@ -144,7 +144,7 @@ i="section-union/good.asm"
 startTest
 $RGBASM -o $otemp section-union/good/a.asm
 $RGBASM -o $gbtemp2 section-union/good/b.asm
-rgblink -o $gbtemp -l section-union/good/script.link $otemp $gbtemp2
+rgblinkQuiet -o $gbtemp -l section-union/good/script.link $otemp $gbtemp2
 dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/good/ref.out.bin)) > $otemp 2>/dev/null
 tryCmp section-union/good/ref.out.bin $otemp
 rc=$(($? || $rc))
@@ -153,7 +153,7 @@ i="section-union/fragments.asm"
 startTest
 $RGBASM -o $otemp section-union/fragments/a.asm
 $RGBASM -o $gbtemp2 section-union/fragments/b.asm
-rgblink -o $gbtemp $otemp $gbtemp2
+rgblinkQuiet -o $gbtemp $otemp $gbtemp2
 dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/fragments/ref.out.bin)) > $otemp 2>/dev/null
 tryCmp section-union/fragments/ref.out.bin $otemp
 rc=$(($? || $rc))
@@ -162,7 +162,7 @@ for i in section-union/*.asm; do
 	startTest
 	$RGBASM -o $otemp $i
 	$RGBASM -o $gbtemp2 $i -DSECOND
-	if rgblink $otemp $gbtemp2 2>$outtemp; then
+	if rgblinkQuiet $otemp $gbtemp2 2>$outtemp; then
 		echo -e "${bold}${red}$i didn't fail to link!${rescolors}${resbold}"
 		rc=1
 	fi