feat(gotter)!: add DT_HASH fallback, relocation type guard, and hook_symbol - #2297
feat(gotter)!: add DT_HASH fallback, relocation type guard, and hook_symbol#2297gyuheon0h wants to merge 10 commits into
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: e9568ee | 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-08-05 21:23:31 Comparing candidate commit e9568ee in PR branch Found 3 performance improvements and 0 performance regressions! Performance is the same for 137 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
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".
| 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 👍 / 👎.
934d93c to
5e08f80
Compare
45c074d to
11a3920
Compare
5e08f80 to
e2110f9
Compare
e2110f9 to
cad573a
Compare
🔒 Cargo Deny Results✅ No issues found! 📦
|
📚 Documentation Check Results📦
|
8674202 to
a4a6346
Compare
…ibdd-got-hook (#2282) Stacked under [feat(got-hook): add DT_HASH fallback, relocation type guard, and hook_symbol](#2297) # What does this PR do? Moves the ELF GOT-patching infrastructure out of libdd-profiling-heap-gotter into a new shared crate libdd-got-hook, so multiple crates can reuse the same machinery for runtime function interposition. This is a pure code move. I tried to have no behavioral changes here. What moves to `libdd-got-hook` - `DynamicInfo::from_phdr`: parse PT_DYNAMIC from a loaded ELF object - `gnu_hash` / `gnu_hash_symbol_count` / `gnu_hash_lookup` / `check_sym`: GNU hash table utilities - `iterate_libraries`: this is a `dl_iterate_phdr` wrapper with panic-safe trampoline - `PageProtGuard` / `read_proc_maps` / `MapEntry`: RELRO-aware page protection management - `lookup_symbol` / `LookupResult`: used for resolving a symbol across all loaded objects - `elf64_r_sym`: relocation info helper - ELF types and constants What stays in `libdd-profiling-heap-gotter` - `SymbolOverrides`: multi-symbol registry - `hooks.rs`: the actual `malloc`/`free`/`calloc`/`realloc` hook functions - lib.rs: `install_heap_overrides` / `update_heap_overrides` public API # Motivation What inspired you to submit this pull request? # Additional Notes Gated on @scottgerring's approval # How to test the change? Describe here in detail how the change can be validated.
…_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.
a4a6346 to
9d4d601
Compare
gyuheon0h
changed the title
9d4d601 to
f19c984
Compare
No. Unfortunately, its hard to see what customers are running for their elf hashing :( but as part of just making this library more robust, I thought it was worth to do haha |
gyuheon0h
changed the title
| /// | ||
| /// `hook_fn` must point to a function with the same calling convention | ||
| /// and signature as the symbol being hooked. The patching is permanent. | ||
| pub unsafe fn hook_symbol(symbol_name: &CStr, hook_fn: usize, orig_out: &mut usize) -> bool { |
There was a problem hiding this comment.
How does this function return errors if any?
There was a problem hiding this comment.
It returns false for any failure (symbol not found, no GOT entries patched). The callers treat hooking as best-effort. If it fails, the feature degrades gracefully ( crash reports just show SIGABRT instead of the assert expression). We could change to Result if we want diagnostics, but none of the current callers would act on specific error variants. Happy to add it if you feel strongly. WDYT?
| (sym_count_fallback(symtab, strtab, sysv_hash), 0) | ||
| }; | ||
|
|
||
| // SAFETY (applies to all `slice::from_raw_parts` calls below): |
There was a problem hiding this comment.
These safety points come from from_raw_parts contract
2d9bca3 to
e9568ee
Compare
4 participants
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
PROF-15629
What does this PR do?
Adds three general improvements to
libdd-got-hookfor 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
Gated on @scottgerring's approval
How to test the change?
Describe here in detail how the change can be validated.