Arakula / dasmfw

The DisASseMbler FrameWork
GNU General Public License v2.0
13 stars 4 forks source link

Disassembling 6502 code, option "forceaddr off" in the nfo file appears to not work #6

Closed phillipeaton closed 2 years ago

phillipeaton commented 2 years ago

image

I also tried switching to "forceaddr on", but no difference.

Arakula commented 2 years ago

Could you show me an info file snippet, please? Because ... staring at the code in Dasm6500.cpp:

adr_t Dasm650X::DisassembleCode(...) {
...
  case _abs:                            /* absolute                          */
...
    if (forceExtendedAddr && (W & (uint16_t)0xff00) == (uint16_t)dp)
      sparm = ">" + slbl;
    else
      sparm = slbl;

something like

option forceaddr off

should work, in theory. Also, is "jetpac" something you might give to me for testing or something proprietary you'd rather not see floating around?

phillipeaton commented 2 years ago

I will send it to you.

phillipeaton commented 2 years ago

Sent to your listed address on GitHub, let me know if you didn't receive it.

Arakula commented 2 years ago

I received it, am going to look at it later.

Arakula commented 2 years ago

OK, that was a "collateral damage" ... you had some lines of the format option xxx value * comment in your info file. Unfortunately, dasmfw didn't live up to its own documentation in this case and did not interpret everything including and after the asterisk as a comment, but rather tried to interpret value * comment as the option's new value - and "off * gotcha" is not one of "off" or "on". Oh well ... should be fixed now.

phillipeaton commented 2 years ago

I originally copied the 6502 info file from my 6809 project, then hacked it. I assume this fix would affect 6809 disassemblies also? It could actually break things that previously worked if the now-correctly-parsed switches get reversed. Not a problem if you know about it, but if you don't and then you upgrade, you might wonder what happened. Perhaps worth noting in the version history.

Arakula commented 2 years ago

Yes, this affects all disassemblers in dasmfw, so it might be a good idea to double-check. It's already noted in the version history; I'll detail that in dasmfw.htm, too. In the next version.

Arakula commented 2 years ago

And I might revise that feature a bit ...

[Edit:] or not, see 2nd comment below ...

In some info file instructions, asterisks are more meaningful than in others. Imagine a comment line consisting of a row of asterisks with some text in the middle. Something like comment 0815 ********* Here be dragons! ***** for example.

In previous versions - no problem, it was just copied 1:1. In the current version - out comes an empty line, as the same is interpreted like comment 0815 until you modify it to the rather impracticable comment 0815 \*\*\*\*\*\*\*\*\* Here be dragons! \*\*\*\*\*

Not a satisfying solution. I'll think about it and will presumably replace it with something else. Like "if the instruction has a value part that contains free-form text, just copy the line as is; otherwise, cut it at the asterisk." I think that might work better and reduce the possibility of breaking existing info files. On the other hand, this would lead to an inconsistent syntax. Hmmm, I'm really not sure what's best here.

phillipeaton commented 2 years ago

I have another comment-related scenario that produces non-optimal output, I'll document it tonight. (It's possible I can work around it with \, which I didn't know about until now.)

Arakula commented 2 years ago

Hmmm, I played with it a bit ... in previous versions, the "an asterisk is a comment start" was already implemented anyway ... but only in lines containing free-form text. Hehe, gotcha. This definitely needs more thinking. But at least it should not break info files that worked with earlier versions.

Arakula commented 2 years ago

If your scenario contains leading spaces, like in

comment 0815 This is a comment
comment 0815     with an indented second line

modify it to

comment 0815 This is a comment
comment 0815 \    with an indented second line

to get what you want.

phillipeaton commented 2 years ago

Actually, if you look in my nfo file, you'll see that the cchar switch line has no "* text description" part, unlike all my other switch lines. I recall this is because it broke the setting when I commented it previously. I assumed that it was a special case, I didn't consider it a bug!

phillipeaton commented 2 years ago

I just had to update a comment above, because it contained a naked "\", which then escaped the following character 🤦‍♂️

Arakula commented 2 years ago

Hehe, and that's exactly the fringe case I knew would appear sooner or later :-) Now, neither option cchar * nor option cchar \* works correctly. OK, I definitely need to modify the logic.

phillipeaton commented 2 years ago

By adding data hints to the info file, I managed to get as65 to assemble the disassembled file, but there are differences between the original binary and the recreation. This is not really a surprise for a first run! I will investigate further.

image

Arakula commented 2 years ago

Thanks for telling me ... as it's still written in dasmfw.htm, the 650X stuff is largely untested, there might still be errors in the disassembly. If you can pinpoint an instruction that generates a different code when re-assembled, please keep me informed.

I've uploaded v0.25 now, see https://github.com/Arakula/dasmfw/releases/tag/v0.25 - it deals with the issue here in a better way, allowing info file lines like option cchar \*, so I'm closing this issue again.

phillipeaton commented 2 years ago

I have another comment-related scenario that produces non-optimal output, I'll document it tonight. (It's possible I can work around it with \, which I didn't know about until now.)

I recall what it was that I couldn't produce, it's a line of asterisks from the start of a line, i.e. like this:

image

The best I could do was: (I could have probably used asterisks instead of dashes) image

It's non-optimal because there is a space between the first column ; and the first -, not the end of the world...