Repository navigation
Conversation
Symbol-table addresses come from untrusted sh_offset / DT_SYMTAB values with no alignment guarantee; casting them to ElfSymbol* and dereferencing is undefined behavior and aborts the ASan+UBSan gtest build at symbols_linux.cpp:974. Copy entries (or just st_name) into aligned locals instead. Adds an elfparser_ut regression that places a .symtab at a misaligned file offset and asserts both symbols and the PLT relocation name resolve.
Scan-Build Report
Bug Summary
Filter Results by FileReports
|
|||||||||||||||||||||||||||||||||||||||||||||||||
kaahos
added
test:release
CI Test ResultsRun: #43340155198 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Failed Jobs
Summary: Total: 108 | Passed: 107 | Failed: 1 Updated: 2026-11-13 01:15:25 UTC |
|
🔁 Datadog re-ran 🔗 Commit SHA: 7237a46 | Docs | View more details | Give us feedback! |
State that the misaligned member access is unconditionally undefined behavior (the sanitizer flag only detects it), rename readSymbol to readSymbolEntry to avoid the VMStructs::readSymbol grep collision, and use reinterpret_cast instead of a C-style cast in hashSymbolRelocations.
Resource-dependent tests called exit(0)/exit(1) when the cwd-relative native-libs paths did not resolve, so the whole binary exited before any test ran from the project-root cwd CI uses; replace the early exits with GTEST_SKIP()/FAIL() so the resource-independent tests always execute. Free the CodeCaches that parseLibraries publishes into the tests' stack CodeCacheArrays (and the ElfReladyn fixture's process-wide array via new #ifdef UNIT_TEST freeForTest/freeNativeLibsForTest helpers), so the binary exits LeakSanitizer-clean for the first time. Teardown in the DelayedUnmap tests happens after the unmapper thread joins.
kaahos
changed the title
Reliability & Chaos Results✅ All reliability & chaos checks passed Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/152832102 |
…551) The clause named the detection tooling, not a constraint on the code.
kaahos
changed the title
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 07b533db91
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
The alignment-safe copies preserve conditional field reads, including skipping zero-name symbols before accessing bytes beyond a readable mapping. Runtime sanitizer behavior remains unverified.
🤖 Bits Code Review · Commit 07b533d
There was a problem hiding this comment.
The PR fixes misaligned ELF-symbol reads and test cleanup; the latest update makes the test mapping guard non-copyable. One issue remains: ten generated core dumps add 440 MiB to checkouts and should be removed; runtime sanitizer behavior remains unverified.
The new commit adds ten generated qemu_lineNumberTableCopy_ut_20261110-*.core dumps, each 44 MiB, increasing every checkout and CI workspace by 440 MiB without providing test fixtures. Remove all ten crash artifacts from the PR.
Was this helpful? React 👍 or 👎
🤖 Bits Code Review · Commit 08846b8
08846b8 to
e4e4826
Compare
There was a problem hiding this comment.
The new global-library cleanup breaks repeated full-suite runs: stale inode caches prevent libreladyn from repopulating the emptied library array on the next iteration.
🤖 Bits Code Review · Commit e4e4826
8152ae5 to
d6e5c79
Compare
There was a problem hiding this comment.
More details
The alignment-safe copies preserve conditional field reads, allowing zero-name dynamic symbol entries to be skipped before reading across an unreadable-memory boundary.
🤖 Bits Code Review · Commit d6e5c79
kaahos
changed the title
…lTable (PROF-16551)
kaahos
changed the title
… helpers (PROF-16551)
…ed symtab test (PROF-16551)
kaahos
changed the title
…und relocation walks (PROF-16551)
57032ed to
c3b5d80
Compare
There was a problem hiding this comment.
More details
Source review supports that the alignment-safe copies preserve ELF fields used by import parsing and fingerprinting, while the revised bounds consistently exclude incomplete relocation entries. Runtime sanitizer behavior was not independently verified.
🤖 Bits Code Review · Commit c3b5d80
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Tests failed on this commit bb10fc2: What to do next?
|
What does this PR do?:
Finishes alignment-safe ELF symbol and relocation reads in
symbols_linux.cpp, on top of #974'sloadSymbolTablefix:symbolNameAt()andrelocationAt()copyst_nameand theElf64_Relprefix out withmemcpyat the four sites that dereferenced castElfSymbol*/ElfRelocation*: bothparseDynamicSectionimport paths,addRelocationSymbolsandhashSymbolRelocations.DT_RELAENTis smaller than anElf64_Rel, and advance only while a whole entry fits. That stops reads past a table that ends mid-entry, and thehashSymbolRelocationsoffset no longer wraps back before the table for a hugeDT_RELAENT.Motivation:
PROF-16551: the asan gtest build (
-fsanitize=alignment -fno-sanitize-recover=all) aborts withmember access within misaligned addresson these casts. Nothing checks the alignment ofsh_offset,DT_SYMTAB,DT_JMPRELorDT_RELA, so a crafted or corrupt image can misalign the tables.Additional Notes:
memcpyper entry on cold paths. Well-formed libraries parse and fingerprint exactly as before.freeNativeLibsForTest()is only safe in a binary that never patches or updates build ids, since those tables match caches by address;elfparser_utmeets that.ElfSection/ElfProgramHeader/ElfNote/ElfDyncasts and theDT_HASH/DT_GNU_HASHreads still lack alignment handling;parseDynamicSectiondoes not bound the relocation symbol index, skips.dynsymwhenDT_RELAENTis absent, and takes the.rela.pltstride fromDT_RELAENTrather thanDT_PLTREL.How to test the change?:
For Datadog employees: