Skip to content

fix(nid): rebuild SysV hashes after renaming ELF symbols - #6904

Merged
adriadam10 merged 1 commit into
boykopovar:mainfrom
Zinebyte:fix/elf-sysv-nid-hash
Oct 19, 2026
Merged

adriadam10 merged 1 commit into
boykopovar:mainfrom
Zinebyte:fix/elf-sysv-nid-hash

Conversation

@Zinebyte

@Zinebyte Zinebyte commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What

A normal GCC shared library linked with --hash-style=sysv patches successfully but then loses lookup access to its renamed exports: the actual tool prints OK, while a fresh Linux process resolves only 1 of 18 NIDs. Export names changed without rebuilding the SysV hash table.

Rebuild the existing SysV buckets and chains in place using the final dynamic-symbol names, after any GNU hash symbol reorder. Validate table geometry and its symbol-table link before mutation. SysV-only files retain their symbol indices; files with both hash formats keep both tables consistent. The layout and algorithm follow ELF ABI section 8.5.

Tested

Linux x86-64, GCC/G++ 13.3, glibc 2.39, main d9b696ddbb. Built the actual nid_patcher from all nine sources listed by its CMake target, with C++20 and O2.

A real PIC shared library provides 18 functions returning 100 through 117 plus a retained no-patch export returning 901. Unchanged main resolves 1/18 renamed exports; the fix resolves 18/18 with all expected values. Retained, missing and old-name controls pass. GNU+SysV output passes direct table lookup after actual symbol reordering; a separate copy with the GNU dynamic tag disabled also resolves every expected value through the native SysV loader path. Independent review reproduced both native outcomes.

All eight hash regression methods pass, including SysV-only and dual-hash layouts with 1, 3 and 7 buckets, collision chains, preserved symbols and malformed geometry rejection without writing. All eight applicable NID Python test groups pass. Conventions and whitespace checks pass. The full runtime build and retail titles were not exercised. Checked the 829-open-PR inventory and the actual #6901 shared source; no competing implementation was found.

Combined validation with #6906 also passes: both commits merge cleanly, the rebuilt actual tool passes all eight applicable NID groups, native SysV/dual-hash export lookup controls and zero/partial-write error controls.

Checklist

  • Based on current main; no other open PR implements the same fix
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw; no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

@Zinebyte Zinebyte mentioned this pull request Oct 16, 2026
11 tasks done
@adriadam10 adriadam10 self-assigned this Oct 19, 2026
@adriadam10
adriadam10 merged commit 27cbe9c into boykopovar:main Oct 19, 2026
9 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked the SysV hash rebuild against ELF ABI 8.5: names come from the final symbol list (after the GNU reorder, retained exports included), geometry and sh_link are validated before any write, and the test pins lookups by new name for SysV-only and dual-hash layouts (1/3/7 buckets) plus rejection of bad geometry without writing; merged.

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