asar
asar copied to clipboard
RPGHacker
Don't removerats twice when two autocleans point to the same freespace
Fixes #216
No test added. Since this bug requires existing RATS tags to repro, I don't think the current testing framework would support a test for this.
From a cursory glance, this looks like it'll just trade one bug for another, and leak freespace in cases like
;patch1.asm
org $008000
autoclean jsl hijack1
autoclean jsl hijack2
freecode
hijack1:
rtl
freecode
hijack2:
rtl
;patch2.asm
org $008000
autoclean jsl hijack1
autoclean jsl hijack2
freecode
hijack1:
rtl
hijack2:
rtl
or, of course, things like autoclean dl or autoclean read3($00f606) (this patch only touches the autoclean jml/jsl handler).
If I remember correctly, autoclean originally ran in the first pass, and the freespace finder was second; can't delete something that's not there yet. Did we leave that model? If yes, was it due to bugs involving lda ForwardLabel : autoclean jsl AnotherLabel?
Proposed proper patch: For each autoclean, loop the list of freespaces created in this patch prior to this point. If that list contains this address, the autoclean target was already deleted and reused, and should not be deleted again.
No test added. Since this bug requires existing RATS tags to repro, I don't think the current testing framework would support a test for this.
Actually, I think there is a way to tell the test suite to a apply a patch twice to the same ROM. We just probably forgot to document it in README.me.
Taking a peak at prot.asm, the syntax seems to be:
;`+ #2
Alcaro is right, that testcase shows a bug introduced. And I forgot to handle autoclean dl. I'll rework this.
Looks good to me, other than the nitpick. Good job wrangling this thing, Asar is a messy tool and this isn't its easiest part.
i deleted this entire fix in asar2.0, since there rats tags are cleaned during pass 1 and freespaces are allocated at the end of pass 1. which i'm pretty sure avoids the root cause of this bug
