Skip to content

Fix Thumb platform preservation for ELF mini debug symbols#8143

Draft
plafosse wants to merge 1 commit intodevfrom
test_thumb_gnu_debugdata_fix
Draft

Fix Thumb platform preservation for ELF mini debug symbols#8143
plafosse wants to merge 1 commit intodevfrom
test_thumb_gnu_debugdata_fix

Conversation

@plafosse
Copy link
Copy Markdown
Member

@plafosse plafosse commented May 1, 2026

This fixes ARM/Thumb .gnu_debugdata loading by preserving the original mini-debug symbol address before ELF symbol normalization clears the Thumb bit.

For ARM/Thumb binaries, the parent ELF load passes an internal load option to the embedded mini-debug ELF load. The child ELF records metadata only for function symbols whose normalized address differs from the original symbol address. The parent then reads that metadata from the returned mini-debug BinaryView and uses the original address to recover the associated platform, while still defining the symbol at the normalized address.

This keeps the extra metadata work limited to ARM/Thumb binaries with .gnu_debugdata, avoids re-parsing the embedded ELF, and does not expose the temporary metadata on the final parent view.

@plafosse plafosse requested review from bpotchik and zznop May 1, 2026 17:27
Comment thread view/elf/elfview.h
std::map<uint64_t, uint64_t> m_localGotEntries;
std::set<uint64_t> m_gotEntryLocations;
std::vector<BNRelocationInfo> m_relocationInfo;
std::vector<Ref<Metadata>> m_miniDebugSymbolAddresses;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommend naming this m_miniDebugSymbolOriginalAddresses or something that indicates these addresses aren't the symbol addresses, but rather the raw addresses that preserve the lsb.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they aren't though its a 3-tuple of name, address, original_address

@zznop
Copy link
Copy Markdown
Member

zznop commented May 1, 2026

Verified that this fixes the issue for neon crystal gates sequentially. Some minor feedback, but other than that looks good to me. I do wish we didn't parse that section in a separate view though so we didn't need to use metadata to shuttle over info.

@plafosse
Copy link
Copy Markdown
Member Author

plafosse commented May 1, 2026

My original implementation used a custom parser to pull that data out. I ended up discarding that in favor of this approach so that we specifically did not have to maintain two ELF parsers. IMO this is a better solution than duplicating all the parse logic.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants