From e7d6ddf5935196c598165afecdeac035a106dc17 Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Wed, 24 Feb 2021 20:04:51 -0800 Subject: [PATCH] Fix linking tiny overlay files (#755) * Fix compatibility of rgblink -O and -t The -t "tiny mode" option makes ROM0 cover 0x8000 bytes, not 0x4000. The -O "overlay" option fills areas uncovered by sections with data from an overlay file. These needed to cooperate so that the calculated uncovered overlay size does not exceed the actual size of the ROM. Fixes #754 * Print link test names like asm tests do * Make the three test.sh scripts more similar --- src/link/output.c | 63 +++++++++++++++------ src/linkdefs.c | 4 +- test/asm/test.sh | 23 ++++++-- test/fix/test.sh | 14 +++-- test/link/bank-const/{err.out => out.err} | 0 test/link/overlay/a.asm | 2 + test/link/overlay/out.err | 0 test/link/overlay/out.gb | Bin 0 -> 32768 bytes test/link/overlay/overlay.gb | 1 + test/link/test.sh | 66 ++++++++++++++++------ 10 files changed, 123 insertions(+), 50 deletions(-) rename test/link/bank-const/{err.out => out.err} (100%) create mode 100644 test/link/overlay/a.asm create mode 100644 test/link/overlay/out.err create mode 100644 test/link/overlay/out.gb create mode 100644 test/link/overlay/overlay.gb diff --git a/src/link/output.c b/src/link/output.c index d9a1dbd1..ced97e85 100644 --- a/src/link/output.c +++ b/src/link/output.c @@ -17,7 +17,11 @@ #include "extern/err.h" -FILE * outputFile; +#include "linkdefs.h" + +#define BANK_SIZE 0x4000 + +FILE *outputFile; FILE *overlayFile; FILE *symFile; FILE *mapFile; @@ -103,43 +107,63 @@ struct Section const *out_OverlappingSection(struct Section const *section) /** * Performs sanity checks on the overlay file. + * @return The number of ROM banks in the overlay file */ -static void checkOverlay(void) +static uint32_t checkOverlaySize(void) { if (!overlayFile) - return; + return 0; if (fseek(overlayFile, 0, SEEK_END) != 0) { warnx("Overlay file is not seekable, cannot check if properly formed"); - return; + return 0; } long overlaySize = ftell(overlayFile); - if (overlaySize % 0x4000) - errx(1, "Overlay file must have a size multiple of 0x4000"); - /* Reset back to beginning */ fseek(overlayFile, 0, SEEK_SET); - uint32_t nbOverlayBanks = overlaySize / 0x4000 - 1; + if (overlaySize % BANK_SIZE) + errx(1, "Overlay file must have a size multiple of 0x4000"); - if (nbOverlayBanks < 1) + uint32_t nbOverlayBanks = overlaySize / BANK_SIZE; + + if (is32kMode && nbOverlayBanks != 2) + errx(1, "Overlay must be exactly 0x8000 bytes large"); + + if (nbOverlayBanks < 2) errx(1, "Overlay must be at least 0x8000 bytes large"); - if (nbOverlayBanks > sections[SECTTYPE_ROMX].nbBanks) { + return nbOverlayBanks; +} + +/** + * Expand sections[SECTTYPE_ROMX].banks to cover all the overlay banks. + * This ensures that writeROM will output each bank, even if some are not + * covered by any sections. + * @param nbOverlayBanks The number of banks in the overlay file + */ +static void coverOverlayBanks(uint32_t nbOverlayBanks) +{ + /* 2 if is32kMode, 1 otherwise */ + uint32_t nbRom0Banks = maxsize[SECTTYPE_ROM0] / BANK_SIZE; + /* Discount ROM0 banks to avoid outputting too much */ + uint32_t nbUncoveredBanks = nbOverlayBanks - nbRom0Banks > sections[SECTTYPE_ROMX].nbBanks + ? nbOverlayBanks - nbRom0Banks + : 0; + + if (nbUncoveredBanks > sections[SECTTYPE_ROMX].nbBanks) { sections[SECTTYPE_ROMX].banks = realloc(sections[SECTTYPE_ROMX].banks, - sizeof(*sections[SECTTYPE_ROMX].banks) * - nbOverlayBanks); + sizeof(*sections[SECTTYPE_ROMX].banks) * nbUncoveredBanks); if (!sections[SECTTYPE_ROMX].banks) err(1, "Failed to realloc banks for overlay"); - for (uint32_t i = sections[SECTTYPE_ROMX].nbBanks; - i < nbOverlayBanks; i++) { + for (uint32_t i = sections[SECTTYPE_ROMX].nbBanks; i < nbUncoveredBanks; i++) { sections[SECTTYPE_ROMX].banks[i].sections = NULL; sections[SECTTYPE_ROMX].banks[i].zeroLenSections = NULL; } - sections[SECTTYPE_ROMX].nbBanks = nbOverlayBanks; + sections[SECTTYPE_ROMX].nbBanks = nbUncoveredBanks; } } @@ -195,16 +219,19 @@ static void writeROM(void) outputFile = openFile(outputFileName, "wb"); overlayFile = openFile(overlayFileName, "rb"); - checkOverlay(); + uint32_t nbOverlayBanks = checkOverlaySize(); + + if (nbOverlayBanks > 0) + coverOverlayBanks(nbOverlayBanks); if (outputFile) { if (sections[SECTTYPE_ROM0].nbBanks > 0) writeBank(sections[SECTTYPE_ROM0].banks[0].sections, - 0x0000, 0x4000); + startaddr[SECTTYPE_ROM0], maxsize[SECTTYPE_ROM0]); for (uint32_t i = 0 ; i < sections[SECTTYPE_ROMX].nbBanks; i++) writeBank(sections[SECTTYPE_ROMX].banks[i].sections, - 0x4000, 0x4000); + startaddr[SECTTYPE_ROMX], maxsize[SECTTYPE_ROMX]); } closeFile(outputFile); diff --git a/src/linkdefs.c b/src/linkdefs.c index d71247eb..d45c0beb 100644 --- a/src/linkdefs.c +++ b/src/linkdefs.c @@ -13,11 +13,11 @@ uint16_t startaddr[] = { }; uint16_t maxsize[] = { - [SECTTYPE_ROM0] = 0x8000, + [SECTTYPE_ROM0] = 0x8000, // patched to 0x4000 if !is32kMode [SECTTYPE_ROMX] = 0x4000, [SECTTYPE_VRAM] = 0x2000, [SECTTYPE_SRAM] = 0x2000, - [SECTTYPE_WRAM0] = 0x2000, + [SECTTYPE_WRAM0] = 0x2000, // patched to 0x1000 if !isWRA0Mode [SECTTYPE_WRAMX] = 0x1000, [SECTTYPE_OAM] = 0x00A0, [SECTTYPE_HRAM] = 0x007F diff --git a/test/asm/test.sh b/test/asm/test.sh index 6286ea33..e791e8e1 100755 --- a/test/asm/test.sh +++ b/test/asm/test.sh @@ -17,12 +17,23 @@ red="$(tput setaf 1)" green="$(tput setaf 2)" orange="$(tput setaf 3)" rescolors="$(tput op)" + +RGBASM=../../rgbasm +RGBLINK=../../rgblink + tryDiff () { - diff -u --strip-trailing-cr $1 $2 || (echo "${bold}${red}${i%.asm}${variant}.$3 mismatch!${rescolors}${resbold}"; false) + if ! diff -u --strip-trailing-cr "$1" "$2"; then + echo "${bold}${red}${i%.asm}${variant}.$3 mismatch!${rescolors}${resbold}" + false + fi } tryCmp () { - cmp $1 $2 || (../../contrib/gbdiff.bash $1 $2; echo "${bold}${red}${i%.asm}${variant}.out.bin mismatch!${rescolors}${resbold}"; false) + if ! cmp "$1" "$2"; then + ../../contrib/gbdiff.bash "$1" "$2" + echo "${bold}${red}${i%.asm}${variant}.out.bin mismatch!${rescolors}${resbold}" + false + fi } # Add the version constants test, outputting the closest tag to the HEAD @@ -49,7 +60,7 @@ fi # Check whether to use '.simple.err' files if they exist # (rgbasm with pre-3.0 Bison just reports "syntax error") -../../rgbasm -Weverything -o $o syntax-error.asm > $output 2> $errput +$RGBASM -Weverything -o $o syntax-error.asm > $output 2> $errput cmp syntax-error.err $errput > /dev/null 2> /dev/null simple_error=$? if [ "$simple_error" -eq 1 ]; then @@ -64,7 +75,7 @@ for i in *.asm; do desired_errname=${i%.asm}.simple.err fi if [ -z "$variant" ]; then - ../../rgbasm -Weverything -o $o $i > $output 2> $errput + $RGBASM -Weverything -o $o $i > $output 2> $errput desired_output=${i%.asm}.out desired_errput=$desired_errname else @@ -78,7 +89,7 @@ for i in *.asm; do # stdin redirection makes the input an unseekable pipe - a scenario # that's harder to deal with and was broken when the feature was # first implemented. - cat $i | ../../rgbasm -Weverything -o $o - > $output 2> $errput + cat $i | $RGBASM -Weverything -o $o - > $output 2> $errput # Use two otherwise unused files for temp storage desired_output=$input @@ -97,7 +108,7 @@ for i in *.asm; do bin=${i%.asm}.out.bin if [ -f $bin ]; then - ../../rgblink -o $gb $o + $RGBLINK -o $gb $o dd if=$gb count=1 bs=$(printf %s $(wc -c < $bin)) > $output 2>/dev/null tryCmp $bin $output our_rc=$(($? || $our_rc)) diff --git a/test/fix/test.sh b/test/fix/test.sh index 5b0e13ea..9a8da723 100755 --- a/test/fix/test.sh +++ b/test/fix/test.sh @@ -16,6 +16,8 @@ red="$(tput setaf 1)" green="$(tput setaf 2)" rescolors="$(tput op)" +RGBFIX=./rgbfix + tryDiff () { if ! diff -u --strip-trailing-cr "$1" "$2"; then echo "${bold}${red}${3:-$1} mismatch!${rescolors}${resbold}" @@ -41,7 +43,7 @@ runTest () { fi if [[ -z "$variant" ]]; then cp "$2/$1.bin" out.gb - if [[ -n "$(eval ./rgbfix $flags out.gb '2>out.err')" ]]; then + if [[ -n "$(eval $RGBFIX $flags out.gb '2>out.err')" ]]; then echo "${bold}${red}Fixing $1 in-place shouldn't output anything on stdout!${rescolors}${resbold}" our_rc=1 fi @@ -50,14 +52,14 @@ runTest () { # Stop! This is not a Useless Use Of Cat. Using cat instead of # stdin redirection makes the input an unseekable pipe - a scenario # that's harder to deal with. - cat "$2/$1.bin" | eval ./rgbfix "$flags" '>out.gb' '2>out.err' + cat "$2/$1.bin" | eval $RGBFIX "$flags" '>out.gb' '2>out.err' subst='' fi - sed "s/$subst//g" "out.err" | tryDiff "$2/$1.err" - "$1.err${variant}" + sed "s/$subst//g" "out.err" | tryDiff "$2/$1.err" - "$1.err${variant}" our_rc=$(($? || $our_rc)) if [[ -r "$2/$1.gb" ]]; then - tryCmp "$2/$1.gb" "out.gb" "$1.gb${variant}" + tryCmp "$2/$1.gb" "out.gb" "$1.gb${variant}" our_rc=$(($? || $our_rc)) fi @@ -91,9 +93,9 @@ printf "\rDone! \n" # TODO: check MBC names # Check that RGBFIX errors out when inputting a non-existent file... -./rgbfix noexist 2>out.err +$RGBFIX noexist 2>out.err rc=$(($rc || $? != 1)) -tryDiff "$src/noexist.err" out.err noexist.err +tryDiff "$src/noexist.err" out.err noexist.err rc=$(($rc || $?)) exit $rc diff --git a/test/link/bank-const/err.out b/test/link/bank-const/out.err similarity index 100% rename from test/link/bank-const/err.out rename to test/link/bank-const/out.err diff --git a/test/link/overlay/a.asm b/test/link/overlay/a.asm new file mode 100644 index 00000000..621624c6 --- /dev/null +++ b/test/link/overlay/a.asm @@ -0,0 +1,2 @@ +SECTION "0", ROM0[0] +DS $8000 diff --git a/test/link/overlay/out.err b/test/link/overlay/out.err new file mode 100644 index 00000000..e69de29b diff --git a/test/link/overlay/out.gb b/test/link/overlay/out.gb new file mode 100644 index 0000000000000000000000000000000000000000..12f3be4dd3b5a2b5146f36630acbf7e99e490797 GIT binary patch literal 32768 zcmeIufdBvi0Dz$VsTV1P3IhfV7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd z0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwA zz<>b*1`HT5V8DO@0|pEjFkrxd0RsjM7%*VKfB^#r3>YwAz<>b*1`HT5V8DO@0|pEj GFkoPS00031 literal 0 HcmV?d00001 diff --git a/test/link/overlay/overlay.gb b/test/link/overlay/overlay.gb new file mode 100644 index 00000000..3bed79fb --- /dev/null +++ b/test/link/overlay/overlay.gb @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/link/test.sh b/test/link/test.sh index 7b1abf77..478c996b 100755 --- a/test/link/test.sh +++ b/test/link/test.sh @@ -1,37 +1,44 @@ #!/bin/bash + export LC_ALL=C set -o pipefail -otemp=$(mktemp) -gbtemp=$(mktemp) -gbtemp2=$(mktemp) -outtemp=$(mktemp) +otemp="$(mktemp)" +gbtemp="$(mktemp)" +gbtemp2="$(mktemp)" +outtemp="$(mktemp)" rc=0 trap "rm -f '$otemp' '$gbtemp' '$gbtemp2' '$outtemp'" EXIT -bold=$(tput bold) -resbold=$(tput sgr0) -red=$(tput setaf 1) -rescolors=$(tput op) +bold="$(tput bold)" +resbold="$(tput sgr0)" +red="$(tput setaf 1)" +green="$(tput setaf 2)" +rescolors="$(tput op)" + +RGBASM=../../rgbasm +RGBLINK=../../rgblink + +startTest () { + echo "$bold$green${i%.asm}...$rescolors$resbold" +} tryDiff () { - if ! diff -u --strip-trailing-cr $1 $2; then + if ! diff -u --strip-trailing-cr "$1" "$2"; then echo "${bold}${red}${i%.asm}.out mismatch!${rescolors}${resbold}" false fi } tryCmp () { - if ! cmp $1 $2; then - ../../contrib/gbdiff.bash $1 $2 + if ! cmp "$1" "$2"; then + ../../contrib/gbdiff.bash "$1" "$2" echo "${bold}${red}${i%.asm}.out.bin mismatch!${rescolors}${resbold}" false fi } -RGBASM=../../rgbasm -RGBLINK=../../rgblink rgblink() { out="$(env $RGBLINK "$@")" || return $? if [[ -n "$out" ]]; then @@ -41,6 +48,7 @@ rgblink() { } for i in *.asm; do + startTest $RGBASM -o $otemp $i # Some tests have variants depending on flags @@ -91,13 +99,16 @@ done # These tests do their own thing +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 -i="bank-const.asm" tryDiff bank-const/err.out $outtemp +tryDiff bank-const/out.err $outtemp rc=$(($? || $rc)) 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 @@ -110,27 +121,46 @@ for i in fragment-align/*; do fi done +i="high-low.asm" +startTest $RGBASM -o $otemp high-low/a.asm rgblink -o $gbtemp $otemp $RGBASM -o $otemp high-low/b.asm rgblink -o $gbtemp2 $otemp -i="high-low.asm" tryCmp $gbtemp $gbtemp2 +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 +# 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)) +tryCmp overlay/out.gb $gbtemp +rc=$(($? || $rc)) + +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 dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/good/ref.out.bin)) > $otemp 2>/dev/null -i="section-union/good.asm" tryCmp section-union/good/ref.out.bin $otemp +tryCmp section-union/good/ref.out.bin $otemp rc=$(($? || $rc)) + +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 dd if=$gbtemp count=1 bs=$(printf %s $(wc -c < section-union/fragments/ref.out.bin)) > $otemp 2>/dev/null -i="section-union/fragments.asm" tryCmp section-union/fragments/ref.out.bin $otemp +tryCmp section-union/fragments/ref.out.bin $otemp rc=$(($? || $rc)) + for i in section-union/*.asm; do - $RGBASM -o $otemp $i + startTest + $RGBASM -o $otemp $i $RGBASM -o $gbtemp2 $i -DSECOND if rgblink $otemp $gbtemp2 2>$outtemp; then echo -e "${bold}${red}$i didn't fail to link!${rescolors}${resbold}"