New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updated z88dk-dis to produce duplicate output symbols much less often #2104
updated z88dk-dis to produce duplicate output symbols much less often #2104
Conversation
61cbe3e
to
068f1be
Compare
b3ca260
to
c6f6cb6
Compare
c6f6cb6
to
2d4c935
Compare
Could you please provide a minimum example of a project that reproduces the issue? So I can verify the change is working and merge it. |
Sure, but this is more of an added feature rather than a bugfix. I compile test.c with:
then disassemble it with:
Although the map file contains multiple symbols named 'i_2', the output.asm has these duplicates appended with the module name, or uses a different symbol if there's one that maps to the same address. Here's the content of test.c:
|
Thanks, was able to repro and it seems your change indeed fixes the issue. I am conflicted about this. @suborb what do you think? |
My thinking is that the disassembler output cannot be fed back into z80asm without the end user having to manually edit it anyhow. The disassembler doesn't output symbol names within ld instructions for example. The map files do and I think probably should have the original symbols that were compiled in them. I only intend my changes to the disassembled output to be as an aid to understanding by disambiguating duplicate symbols. I don't expect these changes will fix every problem that might occur with the disassembled output, just make the problems far less common. In the example I gave, there were also duplicate symbols that were manually written by the authors of the library, not by generated code ('loop', 'asm' for two examples). So generating unique i_* symbols wouldn't help with those examples. |
Sure, why not. I'll give a few days for @suborb to respond and then I'll merge. |
Yes of course. I just thought I'd try and explain my thinking better to yourself and suborb. |
@pjshumphreys I've pushed an extra commit to your branch to do some syntax style fixes. If you're okay with those changes -- I'll merge. |
Yeah those syntax changes are fine with me. Please feel free to merge. Just out of interest, is there an automatic linter this project is using? What settings does it have? |
I am not aware of one, just trying to follow existing syntax. Thanks. |
(fixes a bug introduced by #2104)
FYI
That doesn't fit into the 300 or so bytes allocated on stack. So I slapped a quick fix in 23ac97c since cutting the disassembly short is not an option either. |
No description provided.