feat(got-hook): add DT_HASH fallback, relocation type guard, and hook_symbol - #2297
feat(got-hook): add DT_HASH fallback, relocation type guard, and hook_symbol#2297gyuheon0h wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📚 Documentation Check Results📦
|
🔒 Cargo Deny Results✅ No issues found! 📦
|
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 934d93c | Docs | Datadog PR Page | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
BenchmarksComparisonBenchmark execution time: 2026-07-29 19:53:48 Comparing candidate commit 934d93c in PR branch Found 1 performance improvements and 2 performance regressions! Performance is the same for 139 metrics, 0 unstable metrics.
|
1c3157f to
5f00ce2
Compare
e9b992a to
18d4e84
Compare
1c3157f to
5f00ce2
Compare
18d4e84 to
00baa22
Compare
5f00ce2 to
4618f83
Compare
00baa22 to
d4b9d82
Compare
4618f83 to
8713e2e
Compare
d4b9d82 to
560d0a6
Compare
8713e2e to
0f700c2
Compare
…_symbol - DT_HASH (sysv) fallback: from_phdr no longer requires DT_GNU_HASH. Objects linked with --hash-style=sysv are now parsed by reading nchain from the sysv hash header. - elf64_r_type + is_got_pointer_reloc: only patch GLOB_DAT and JUMP_SLOT relocations. Skips non-pointer relocation types that would corrupt adjacent code/data if overwritten as 8-byte pointer slots. - hook_symbol: single-symbol convenience wrapper that composes dlsym, iterate_libraries, and patch_got_entries into one call.
0f700c2 to
934d93c
Compare
560d0a6 to
45c074d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 934d93c530
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } else { | ||
| (sym_count_fallback(symtab, strtab, sysv_hash), 0) |
There was a problem hiding this comment.
When this new branch admits a DSO that only has DT_HASH, the resulting DynamicInfo still has no usable GNU hash, but lookup_symbol resolves definitions only through gnu_hash_lookup. If the real allocator or hook target is provided by a SysV-hash-only DSO, lookup_symbol returns None, SymbolOverrides::apply_overrides leaves the ref slot at 0, and the install path skips patching that symbol even though the object is now considered parseable. Add a SysV hash lookup or a bounded dynsym scan using nchain for this branch.
Useful? React with 👍 / 👎.
| matches!( | ||
| r_type, | ||
| R_X86_64_GLOB_DAT | R_X86_64_JUMP_SLOT | R_AARCH64_GLOB_DAT | R_AARCH64_JUMP_SLOT |
There was a problem hiding this comment.
On 64-bit Linux targets other than x86_64/aarch64, this crate still compiles and the GOT-patching APIs are active, but this shared predicate now admits only the x86_64/aarch64 relocation numbers. Architectures such as s390x, ppc64, or riscv64 use different GLOB_DAT/JUMP_SLOT values, so their import relocations are all skipped and installation can report success after resolving originals without actually patching any GOT entries. Either gate the Linux implementation to the supported architectures or add the architecture-specific constants.
Useful? React with 👍 / 👎.
| let real = libc::dlsym(libc::RTLD_DEFAULT, symbol_name.as_ptr()); | ||
| if real.is_null() { | ||
| return false; | ||
| } | ||
| let real_addr = real as usize; | ||
| if real_addr == hook_fn { | ||
| return false; |
There was a problem hiding this comment.
If the replacement function is exported under the same symbol_name being hooked (a common preload/interposer pattern for crash-tracking wrappers), dlsym(RTLD_DEFAULT, ...) can resolve this library's hook first; the equality check below then returns false and no GOT entries are patched. Use a lookup that excludes hook_fn (as SymbolOverrides already does) or an RTLD_NEXT-style resolution so same-name wrappers can still install.
Useful? React with 👍 / 👎.
1 participant
Stacked above refactor(got-patching): extract shared GOT-patching primitives into libdd-got-hook
Stacked under feat(crashtracking): retrieve c assert message for linux when __assert_fail is dynamically loaded
What does this PR do?
Adds three improvements to
libdd-got-hookneeded for crash tracking (and beneficial to heap profiling):DT_HASH(SysV) fallback:DynamicInfo::from_phdrno longer requiresDT_GNU_HASH. Objects linked with--hash-style=sysvare now parsed by readingnchainfrom the SysV hash header. We jjust skipped these objects previously.elf64_r_type+is_got_pointer_relocnow only patchesGLOB_DATandJUMP_SLOTrelocations (pointer-sized GOT slots) on bothx86_64andaarch64. Non-pointer relocation types are now skipped, preventing potential corruption of adjacent code/data from writing 8 bytes into a 4-byte relocation field. Applied to bothlibdd-got-hook'spatch_got_entriesandlibdd-profiling-heap-gotter'sprocess_relocation. (this was a codex recommendation)hook_symbol: Single-symbol convenience wrapper that combinesdlsym+iterate_libraries+patch_got_entriesinto one call.Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.