search

RPGHacker / asar

(Now) official repository of the SNES assembler Asar, originally created by Alcaro
Other
195 stars 42 forks source link

asar resolving paths depending on working directory in some circumstances #253

Open Underrout opened 2 years ago

Underrout commented 2 years ago

I've already talked about this to Atari and I believe he's identified the source of this bug, I'm mostly posting this issue for documentation reasons so I can link to it in the future.

Given a folder structure like this:

.
└── root/
    ├── a.asm
    └── nested_1/
        ├── b.asm
        └── nested_2/
            └── c.asm

with the files a.asm

; empty

b.asm

incsrc "nested_2/c.asm"

c.asm

incsrc "../a.asm"

if we run asar ./root/nested_1/b.asm, we would expect assembly to always fail, since the incsrc "../a.asm" in c.asm would be referring to ./root/nested_1/a.asm, which does not exist.

And, as expected, assembly does fail in most cases, expect when our current working directory is ./root/nested_1. In this case asar will first correctly look for ./root/nested_1/a.asm, fail to find it and then accidentally check for ../a.asm again, but relative to ./root/nested_1/b.asm this time. This will result in it checking for ./root/a.asm, which does exist, but is only found when we're in this specific working directory.

I believe this bug is the reason something like this:

.
└── root/
    ├── shared_code/
    │   └── a.asm
    └── gps/
        ├── block_boilerplate.asm (generated file)
        └── blocks/
            └── block.asm

with a.asm

; empty

block_boilerplate.asm (generated file)

; ... block insertion stuff ...
incsrc "blocks/block.asm"
; ... more block insertion stuff ...

block.asm

incsrc "../shared_code/a.asm"

will actually cause GPS to insert the block without errors, despite the relative path from the incsrc incorrectly pointing to ./root/gps/shared_code/a.asm, since the include is also resolved relative to ./root/gps/block_boilerplate.asm, which then points to ./root/shared_code/a.asm, the same way as the first example. I believe that this again only works out because GPS's working directory is gps during insertion in this case.

I can't really tell how many people are relying on this bug by accident, but I have seen it happen more than once specifically with this patch which uses a incsrc "../SSPDef/Defines.asm" include in multiple places which often works out even if you don't follow the exact instructions for the patch and just place the SSPDef folder "close enough" for the asar bug to still find it.

Atari2 commented 1 year ago

I'm reposting the message that I wrote on discord when I found the cause of the issue (the screenshot is there as a reference):

asar tries to assemble the absolute path of "incsrc ../a.asm", it correctly assembles c:/users/aless/downloads/root/inside/a.asm (because it creates it from the path of the currently being assembled file, which is c:/users/aless/downloads/root/inside/inside/c.asm in this case) and checks if the file exists. in this case, the file doesn't exist, so it gives up on trying to assemble the absolute path and returns to using "../a.asm". so far it's all fine. however, down the line it checks again for the file's existence to open and read from it, and to do that it does

return (GetFileAttributes(filename) != INVALID_FILE_ATTRIBUTES);

which does use the current working directory. This makes it so asar ends up finding ../a.asm in one case and not the other depending on where you're executing the command from. image