Skip to content
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

PPCSymbolDB: Refactor SymbolMap Save/Load #13247

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sepalani
Copy link
Contributor

@sepalani sepalani commented Dec 28, 2024

This PR is an attempt to debug #13195 issues. I'm migrating the change here since it seems to fix the flatpak issue.

This PR refactors parts of the loading/saving process of symbol map. This PR fixes issues I had where some symbols weren't loaded properly (which might get saved later). Here are some examples on how they can alter existing symbol maps.

The alignment wasn't properly detected and 0 was detected as name by sscanf.

This issue was also caused by the namepos/strstr logic which is flawed when the name is small and equals to part of the symbol address, size or vaddress. Moreover, checking for a space doesn't work when alignment is 16. I've seen this alignment value on some *fill* symbols.

-80468370 00000008 80468370 0 TRKPositionFile (entry of .text)  TRK_Hollywood_Revolution.a C:\products\RVL\runtime_libs\debugger\embedded\MetroTRK\Processor\
+80468360 00000008 80468360 0 0468360 00000008 80468360 0 TRKPositionFile (entry of .text)      TRK_Hollywood_Revolution.a C:\products\RVL\runtime_libs\debugger\embedded\MetroTRK\Processor\

savegpr / loadgpr entry wasn't properly detected

This issue also trimmed the object names after the "entry of" part.

-80456e54 000018 80456e54 0 _restgpr_27 (entry of __restore_gpr)        Runtime.PPCEABI.H.a runtime.o
+80456e54 000018 80456e54 0 __restore_gpr::

This PR is ready to be reviewed and tested. I also advise people testing this PR to backup their symbol maps just in case or test this PR in a dedicated portable build.

@sepalani sepalani marked this pull request as ready for review January 4, 2025 13:22
@sepalani sepalani changed the title [DO NOT MERGE] Debug map ranges PR PPCSymbolDB: Refactor SymbolMap Save/Load Jan 4, 2025
@dreamsyntax
Copy link
Member

I'm migrating the change here since it seems to fix the flatpak issue.

I'll verify this, before the mitaclaw requested change it still did not work.

Test pending

@dreamsyntax
Copy link
Member

dreamsyntax commented Jan 7, 2025

Test completed; No longer hangs with the Flatpak 🥳

Looking at the actual diff now with before and after symbols

@dreamsyntax
Copy link
Member

Well that sucks my network failed and I lost my message.
Basically there's an issue still with flatpak (and maybe just linux?) but I can't build natively on linux right now because of fmt and other incompatibilities with arch atm. I can maybe test on mint later.

Map used:
https://raw.githubusercontent.com/ShadowTheHedgehogHacking/shadow-symbols-and-dmw/refs/heads/main/GUPE8P.map

Windows load/save works as expected, with 2.2MB output
Linux (flatpak) load/save results in a different file going to 1.7MB output.
GUPE8P.map.zip

I don't see why the platform would change my .map file in this PR only.

@dreamsyntax
Copy link
Member

dreamsyntax commented Jan 8, 2025

Update on the above.
The loss of symbol data is an issue even on master (I used feature/fix-fmt11) on Linux compared to Windows. Tested native build

Not related to this PR, but would be nice to be fixed.

I also tested with a map file on 2412 (before the 2412-23 symbol format adjustment) and the loss still happens on Linux only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants