Kingcom / armips

An assembler for various ARM and MIPS platforms. Builds available at http://buildbot.orphis.net/armips/
MIT License
359 stars 77 forks source link

Preserve case when outputting symbols in sym format #155

Open Zeturic opened 5 years ago

Zeturic commented 5 years ago

Sorry for not opening an issue first. But basically:

The sym format normally lowercases all of the symbols when writing the sym file. This can be irritating when the "canonical" version of the name isn't the lowercased one - e.g. in C-land it's Foo but ARMIPS ultimately outputs foo in the symbol file, because No$GBA doesn't treat them case-insensitively and thus doesn't recognize that Foo exists.

This PR makes both sym case-preserving (sym2 already was), avoiding that issue.

If you'd rather this be combined into a single commit, just let me know.

Edit: I went ahead and squashed this into one commit.

Zeturic commented 5 years ago

Oh, whoops. I just realized that e595501 is buggy. If the capitalization doesn't match between the -definelabel on the command line and the usage in the file, ARMIPS doesn't recognize them as being the same label.

I've reverted it for now, while I work on having -definelabel labels properly be case-preserving.

Zeturic commented 5 years ago

I rebased against the most recent master (though admittedly there were no conflicts).

Also, I forgot to mention when I initially posted this, but if there's some compelling technical reason why -sym has to stay as it is (all lowercase), I could modify the case-preserving version to be -sym3 instead.

Kingcom commented 5 years ago

Sorry, I forgot about this one. The -sym option was meant to be compatible with the no$gba assembler, which is why it also lowercases everything. -sym2 was an extension made to support function sizes in PPSSPP, which does not lowercase the labels. If you do not use .function, the -sym2 output should be identical to the -sym output except for the label cases.

It is probably worth discussing if keeping the no$gba behavior is worth it. Its assembler was never particularly good, and I doubt there are many people who depend on this behavior.

Zeturic commented 4 years ago

I meant to respond to this much earlier, sorry.

The reason I couldn't personally use -sym2 was because mGBA doesn't understand the format including function sizes, and I couldn't use -sym because mGBA treats the labels case-sensitively so if you (e.g.) try to set a breakpoint for Foo it doesn't recognize it unless you change it to foo.

As a small aside, one of the changes I made in the PR was to preserve the originalName of a -definelabel label should probably be integrated regardless, because -definelabels were (as far as I know) the only time -sym2 doesn't properly preserve case as its supposed to. I can extract just that change into its own PR if preferred.

I'll also be honest, I'm not actually familiar with a standalone No$GBA assembler. I've only ever used these sorts of files as an input to the No$GBA debugging emulator (which yes, does allow you to assemble code at particular locations in a ROM while it's running, but I don't think that's what you were referring to), or more recently, doing the same thing in mGBA.

sp1187 commented 4 years ago

Do you think you could send the -definelabel case fix as a separate PR, as you suggested before?

Zeturic commented 4 years ago

Because it is at least possible that changing -sym's behavior would break someone's workflow, I implemented a -sym3 that behaves as a case-preserving -sym, as I had suggested before. You can look at the commit here (I didn't push it to this PR, yet).

It's small, doesn't affect -sym's behavior, and still solves my problem.

I can force push that commit into this PR branch if desired.