vhelin / wla-dx

WLA DX - Yet Another GB-Z80/Z80/Z80N/6502/65C02/65CE02/65816/68000/6800/6801/6809/8008/8080/HUC6280/SPC-700/SuperFX Multi Platform Cross Assembler Package
Other
548 stars 98 forks source link

Address Relative to Label Issues #110

Closed TheMessengerDevelopments closed 3 years ago

TheMessengerDevelopments commented 8 years ago

EDIT: I've found this problem in version 9.7 as well, which I just cloned and compiled today. I've recently started playing with wla-65816 v9.5, and I just noticed some unexpected behavior with the PER operator, specifically when adding and subtracting from it's operand label:

PER break-1
BRA sampleRoutine
break:
...
sampleRoutine:
...
RTS

This simple bit of code was intended to push the address of the byte preceding break, then jump to sampleRoutine, then jump back the the byte before break to continue on with whatever is in the ... (the program counter gets incremented automatically after the RTS).
I expected the assembler to evaluate PER break-1 to PER $0002 as BRA is 2 bytes in size. However, when I looked at the assemblers output in a debugger (I'm using Geiger's snes9x debugger), I found that it actually output was PER $0003. I ran some more tests, leading to these results: Assembly -> Output PER break-3 -> PER $0001 PER break-2 -> PER $0002 (what I wanted) PER break-1 -> PER $0003 (as above) PER break -> PER $0003 PER break+1 -> PER $0005 I can't be the only one who though that looked weird. It seem to me that the relative address in question is offset by one, but only after arithmetic. This only seemed to happen with PER, as I tried the same experiment with PEA, and got the expected behavior (probably because of relative vs absolute addresses). Anyway, does anyone know what's going on here? Is this supposed to happen, or is it some bug that by some twist of fate was never noticed? And does anyone know any way to make it work in a way that doesn't seem so weird? Your responses are much appreciated.

PS. Thank you developers for making this awesome compiler, as it has provided my many hours of fun. Also, I hope you enjoyed your trip Mr. Helin.

nicklausw commented 8 years ago

I'll look into this in a sec, but note: do not use v9.5, as that is not the most recent version. Use the latest version from here (I can probably give binaries if needed).

TheMessengerDevelopments commented 8 years ago

I see. I didn't realize this was still being so actively developed and getting new versions. Thanks!

TheMessengerDevelopments commented 8 years ago

Also a binary would be nice. Or even just some help compiling it. I can't figure out how to compile c to save my life.

nicklausw commented 8 years ago

Well, without specs so I'm assuming 64-bit windows is okay: http://filebin.ca/2izdaORudNNw/wla-dx-5-30-2016.zip

TheMessengerDevelopments commented 8 years ago

yes, that is exactly what I was looking for

nicklausw commented 8 years ago

Crap, those are from my fork, which has a different argument parser. Hope you can figure out the difference...

TheMessengerDevelopments commented 8 years ago

I actually just figured out how to compile it from source instead :) . Thanks for the help though.

Anyway, I gave it a go, and sadly the bug mentioned earlier is still in the current git release(9.7) :( . Is there anywhere that I should report this?

nicklausw commented 8 years ago

You've actually already found the right place, this is the correct area for reporting issues.

Question, are you using the .BASE directive to set the 24-bit address to $80 or $C0?

nicklausw commented 8 years ago

Wait, no, that's irrelevant. Anyway, the opcode tables in WLA say bra is 2 bytes. One for the cpu to know what to do, one for where to branch to.

TheMessengerDevelopments commented 8 years ago

Oh yeah, so it is. I guess I must have been thinking of JMP, which is 3 byes. I'll edit that. Either way though, in the example above, I expected PER break-1 to evaluate to PER $0002, and that was where the problem arose. If I forgo the label and just write PER $0002 the program works exactly as expected, so I assumed the problem was to do with how the labels were being interpreted.

TheMessengerDevelopments commented 8 years ago

Also, not using .BASE, if you still wanted to know

TheMessengerDevelopments commented 8 years ago

I also should point out, The example above was adapted from this book: http://www.cs.bu.edu/~jappavoo/Resources/210/wdc_65816_manual.pdf, pg. 179. It's meant to replicate a branch to subroutine operation, if you hadn't figured that out yet. I was mostly just using the example to figure out how PER is supposed to work.

vhelin commented 8 years ago

Ouch, sounds like this one needs a high priority...

On Tue, May 31, 2016 at 10:44 AM, TheMessengerDevelopments < notifications@github.com> wrote:

I also should point out, The example above was adapted from this book: http://wiki.nesdev.com/w/images/7/76/Programmanual.pdf http://url, pg.

  1. It's meant to replicate a branch to subroutine operation, if you hadn't figured that out yet. I was mostly just using the example to figure out how PER is supposed to work.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vhelin/wla-dx/issues/110#issuecomment-222586258, or mute the thread https://github.com/notifications/unsubscribe/AGHtUhSB8y6vM41-fjtDPlKx3CM0Ejntks5qG66bgaJpZM4IqFml .

TheMessengerDevelopments commented 8 years ago

I hope so. It seems like a really big issue to me, I'm not sure how nobody noticed it before. I mean, what if somebody wanted to return to the byte immediately following a label?

mrehkopf commented 5 years ago

I just encountered the same issue with current HEAD 8a5a894. Seems to happen on stack calculations where wla doesn't take into account that the displacement value consumes 2 bytes instead of one. It appears that only opcodes of type 9 (PER, BRL) are affected. Relative addressing with 8-bit displacement is ok (type 11).

mrehkopf commented 5 years ago

The reason seems to be that there is no way to provide a hint to wlalink whether to calculate the displacement (relative offset) based on a 1-byte or a 2-byte operand, it always assumes 1 byte.

wla-dx sets a "relative" flag in the 'type' field of the compute stack output and pushes the operations of the calculation expression. e.g.

per @checkButton2End-1

will result in a stack element containing three elements in the object file:

1. String: @checkButton2End
2. Number: 1
3. Operation: -
(Metadata Byte: Stack Type = STACKS_TYPE_16BIT; Relative Flag (bit 7): 1)

WLALink proceeds to interpret these elements and first resolves the absolute memory address, then assigns the relative offset to the String value if it's supposed to be relative.

wlalink/analyze.c:69

  /* parse the type */
  if ((sta->type & (1 << 7)) != 0)
    sta->relative_references = YES;
  else
    sta->relative_references = NO;

wlalink/write.c:2078

      /* is the reference relative? */
      if (sta->relative_references == YES)
        k = k - sta->memory_address - 1;

The "- 1" here is a problem, it assumes that a relative offset that is generated from a math expression is always 8 bits (or the total instruction length is always 2 bytes).

I see two potential resolutions for this problem:

  1. reserve another bit in the stack type field (e.g. bit 6) to toggle between a relative operand size of 1 or 2 bytes (0 = 1 byte as before; 1 = 2 bytes). Set the extra bit in WLA-DX when outputting the compute stacks to the object file, and have WLALink act accordingly. Potential risk: incompatible object files

or

  1. Move the hard-coded adjustment "memory_address - 1" from WLALink to WLA-DX and have it adjust the value in the compute stack already. Remove the "- 1" from WLALink code. When WLA-DX encounters a relative operand and the compute stack is output, push 2 more elements to the stack (numbering based on example given above):
  2. Displacement adjustment value (1 or 2) according to the operand size
  3. Operand: -
mrehkopf commented 5 years ago

... or

  1. use the STACKSTYPE*BIT type to determine the actual operand size when parsing the stacks for output in WLALink. This seems to be the least intrusive and cleanest solution.
mrehkopf commented 5 years ago

259

vhelin commented 3 years ago

Ok, it seems that this issue is solved by @mrehkopf 's PR, no-one just never closed this issue. :)