search

hlorenzi / customasm

💻 An assembler for custom, user-defined instruction sets! https://hlorenzi.github.io/customasm/web/
Apache License 2.0
704 stars 55 forks source link

Include order not respected #177

Closed NathanWilliams closed 1 year ago

NathanWilliams commented 1 year ago

I recently upgraded from v0.12.0 to 0.13.2 and my code no longer assembles in the correct order.

I have a file cpu.asm which has the ruledef for my instructions, and at the end of the file I have:

#bank rom
__main:
  jump main

And then in my programs, I always include cpu.asm as the first thing, ensuring __main is the first thing in my output binary, which can then jump to the main label no matter where I put it:

#include "../cpu.asm"
#include "ram.asm"
#include "rom.asm"
#include "const.asm"

#include "timer.asm"
#include "input.asm"

#bank rom
main:
  call      init
...etc...

However now the first symbol comes from the timer.asm file!

   0:0 |    0 |                ; timer_init:
   0:0 |    0 | 01 00          ; load      0
   2:0 |    2 | 15 0e 84       ; store     drop_timer

I tried to force __main to be first with #addr 0x00 but then customasm simply errors out:

error: output overlap
   --> cpu.asm:203:3:
201 | #addr 0x00
202 | __main:
203 |   jump main
    |   ^^^^^^^^^
204 |
    + note: overlaps with:
     --> prg/timer.asm:8:3:
    6 | #bank rom
    7 | timer_init:
    8 |   load      0
      |   ^^^^^^^^^^^
    9 |   store     drop_timer
   10 |   store     cooldown_timer

I attempted to find the issue in the code myself, but I'm not experienced with Rust sorry.

hlorenzi commented 1 year ago

Hmm, this is indeed mystifying. How are your #bankdefs set up? Is timer.asm also using #bank rom?

If nothing else works, you could create a new #bankdef exclusively for your __main "reset vector" (of only one instruction in size), taking care to also offset your rom bank to start one instruction later.

But as it stands, the behavior you're encountering might be a bug. Could you perhaps provide the entire files you're using, or write a minimal reproduction example? Maybe also try writing a minimal example in a single file without using #include, to see if that makes a difference.

NathanWilliams commented 1 year ago

I narrowed down to a smaller test case. It is still a lot of files, but I have included a file called tcases.txt which shows the different inputs & summary of the result. The results are in the out directory, along with the output when I use v0.12.0.

bug.zip

Thanks for the help, let me know if you need anything else.

chrisgbk commented 1 year ago

I believe I narrowed it down further; some combination of #include the same file multiple times, and #once. I changed every file from your sample (except the first instance in the initial assembly file passed to customasm) to NOT include cpu.asm, and ensured every file had #once as the first line, and it seems to be working as I would expect:

TC1:

 outp | addr | data (base 16)

  0:0 |    0 |       ; __main:
  0:0 |    0 | 51 01 ; jump main
  2:0 |    2 |       ; main:
  2:0 |    2 | 01 41 ; load     "A"
...

TC2:

  outp | addr | data (base 16)

  --:- | 8000 |                ; math_locals:
  --:- | 8000 |                ; .loop:
  --:- | 8001 |                ; .tally:
  --:- | 8002 |                ; locals:
  --:- | 8002 |                ; .tally:
  --:- | 8003 |                ; .hundreds:
  --:- | 8004 |                ; .tens:
  --:- | 8005 |                ; .ones:
  --:- | 8006 |                ; .loop_counter:
  --:- | 8007 |                ; telnet_tmpstr:
  --:- | 800b |                ; cmd_buf:
  --:- | 840d |                ; ansi_tmp:
   0:0 |    0 |                ; __main:
   0:0 |    0 | 50 7f 01       ; jump main
   3:0 |    3 |                ; mult_u8:
...

TC3:

  outp | addr | data (base 16)

  --:- | 8000 |                ; math_locals:
  --:- | 8000 |                ; .loop:
  --:- | 8001 |                ; .tally:
  --:- | 8002 |                ; locals:
  --:- | 8002 |                ; .tally:
  --:- | 8003 |                ; .hundreds:
  --:- | 8004 |                ; .tens:
  --:- | 8005 |                ; .ones:
  --:- | 8006 |                ; .loop_counter:
  --:- | 8007 |                ; telnet_tmpstr:
  --:- | 800b |                ; cmd_buf:
  --:- | 840d |                ; ansi_tmp:
   0:0 |    0 |                ; __main:
   0:0 |    0 | 51 01          ; jump main
   2:0 |    2 |                ; main:
   2:0 |    2 | 01 41          ; load       "A"
...

My modified TC2 is identical to the v0.12.0 output supplied, except for minor differences attributable to version differences:

Comparing files tc2.txt and V0.12.0.TXT
***** tc2.txt
  outp | addr | data (base 16)

***** V0.12.0.TXT
customasm v0.12.0 (x86_64-apple-darwin)
assembling `bug/test.asm`...
success after 2 iterations

  outp | addr | data

*****

***** tc2.txt
  86:0 |   86 |                ; newline:
  86:0 |   86 | 0d 0a          ; ascii("\r\n")
  88:0 |   88 | 00             ; NULL
***** V0.12.0.TXT
  86:0 |   86 |                ; newline:
  86:0 |   86 | 0d 0a          ; #d ascii("\r\n")
  88:0 |   88 | 00             ; NULL
*****

***** tc2.txt
  dd:0 |   dd |                ; cls:
  dd:0 |   dd | 1b 5b 32 4a 1b 5b 48 ; ascii("\x1b[2J\x1b[H")
  e4:0 |   e4 | 00             ; 0x00
***** V0.12.0.TXT
  dd:0 |   dd |                ; cls:
  dd:0 |   dd | 1b 5b 32 4a 1b 5b 48 ; #d ascii("\x1b[2J\x1b[H")
  e4:0 |   e4 | 00             ; 0x00
*****

***** tc2.txt
  e5:0 |   e5 |                ; hide_cursor:
  e5:0 |   e5 | 1b 5b 3f 32 35 6c ; ascii("\x1b[?25l")
  eb:0 |   eb | 00             ; 0x00
***** V0.12.0.TXT
  e5:0 |   e5 |                ; hide_cursor:
  e5:0 |   e5 | 1b 5b 3f 32 35 6c ; #d ascii("\x1b[?25l")
  eb:0 |   eb | 00             ; 0x00
*****

***** tc2.txt
  ec:0 |   ec |                ; csi:
  ec:0 |   ec | 1b 5b          ; ascii("\x1b[")
  ee:0 |   ee | 00             ; 0x00
***** V0.12.0.TXT
  ec:0 |   ec |                ; csi:
  ec:0 |   ec | 1b 5b          ; #d ascii("\x1b[")
  ee:0 |   ee | 00             ; 0x00
*****

***** tc2.txt
  ef:0 |   ef |                ; colour_prefix:
  ef:0 |   ef | 34 38 3b 35 3b ; ascii("48;5;")
  f4:0 |   f4 | 00             ; 0x00
***** V0.12.0.TXT
  ef:0 |   ef |                ; colour_prefix:
  ef:0 |   ef | 34 38 3b 35 3b ; #d ascii("48;5;")
  f4:0 |   f4 | 00             ; 0x00
*****

***** tc2.txt
***** V0.12.0.TXT

*****

Following is a much simplified test case; in ansi.asm comment and uncomment the noted line to change how it outputs. simplified bug.zip

My feeling is that #once (or #include'ing files with #once more than once) might be broken in some way.

NathanWilliams commented 1 year ago

That is good work narrowing it down, thank you!

I compared how once is parsed:

v0.13.2 https://github.com/hlorenzi/customasm/blob/f4e04293ad3de2d2c24624e48ec9314a157f17a1/src/asm/parser/mod.rs#L171

v0.12.0 https://github.com/hlorenzi/customasm/blob/2e7e3373a310e881c0163bd131227980dac7dca2/src/asm/parser/file.rs#L14-L16

I don't know Rust, and I can't seem to track down what a TFilename is, or what let filename = filename.into(); does in the old version, but it seems to be missing in newer one. It might be nothing, but the only difference I can see

chrisgbk commented 1 year ago

I think it might be happening in the handling for include; the behavior seems to be that when a file is #include'd that has already been #included'd once, it doesn't get reparsed, so #once is working, but the second #include moves the contents of the file to the subsequent location instead of leaving it at the first location. So when you are expecting a second or any further #includes to simply be a no-operation include, since it's already been included, instead it moves everything in that file to the last location where #include exists, as if it were the only place #include existed.

I've made a second even simpler test case that does away with banks entirely to show this.

simplified bug2.zip

Again, uncommenting and commenting the noted #include line in inc2.asm changes the output from 0x0000 0x0001 0x0002 0x0003 to 0x0000 0x0002 0x0001 0x0003 and back.

So essentially, because customasm supports out-of-order declarations, this doesn't normally cause any issues. However, because your includes actually generate code when they are included instead of simply constants and definitions, the code generation ends up out-of-order with multiple includes, which mangles your entry point and redirect to main.

hlorenzi commented 1 year ago

I noticed this to-do in the same file, and I can't help but think this has something to do with the issue. I haven't investigated it further yet, but it might be that I've underestimated this to-do's implications. 😅 https://github.com/hlorenzi/customasm/blob/f4e04293ad3de2d2c24624e48ec9314a157f17a1/src/asm/parser/mod.rs#L201-L202

hlorenzi commented 1 year ago

I hope v0.13.3 should have fixed this issue! Let me know if there are any new problems.