You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Update symbol lookup in CLR's embedded ELFIO copy, which is maintained separately from upstream.
Technical Details
Only pick SHT_HASH sections as the symbol hash table. Previously any section whose sh_link pointed at the symbol table was used, so a .rela* section could be read as bucket/chain words.
Validate the SysV hash header (nbucket, nchain matching the symbol count, section bounds) and bound chain traversal. A miss on a well-formed table rules out every non-local symbol; local symbols are still searched, since GNU ld can add named local symbols to a SysV-hashed table without putting them on a chain. The full linear scan is only used when the table is unusable (bad header, out-of-range chain index, unreadable symbol, or a cycle) or the name is empty, since unnamed symbols are never reachable through the hash table.
Check symbol records, string-table links, names, and section indices before use. Section-scoped lookups treat SHN_UNDEF and reserved st_shndx values (SHN_ABS, SHN_COMMON, SHN_XINDEX, ...) as matching no section, instead of dereferencing a null section or matching an unrelated one.
Share lookup logic and use alignment-safe, endian-aware reads.
Section loading (section_impl::load()) only loads sections that lie entirely within the image and treats a short read as missing data. Previously a section running past the end of a truncated image was reported as loaded with an uninitialized tail, and the failed read left the stream in an error state so later sections failed to load. This matches upstream ELFIO's load_data().
get_symbols_num() reports no symbols for a section whose data was not loaded (e.g. sh_size beyond the file), so a header claiming billions of entries no longer makes name lookups scan them all.
Lookup by value continues past a matching symbol whose name cannot be resolved instead of failing.
Guard Elf::getSymbolNum() against underflow when the symbol table is empty or has an invalid entry size.
Elf::getSymbol() and Elf::getSymbolInfo() validate a symbol's data before turning st_value into a pointer: symbols in SHN_UNDEF, reserved, or SHT_NOBITS sections, or whose value and size do not fit inside the section, are rejected. Zero-size symbols at the section end and symbols in empty sections are still accepted.
Fix elf.cpp log formats: 64-bit values use PRIu64 and unsigned values use %u instead of %lu (32-bit on Windows) or %d, and the thread id is printed as its hash instead of passing a std::thread::id object to %zx. The error-log macros are compiled into every build (CL_LOG), so the thread-id fix applies to release builds; the other messages only exist with DEBUG_DETAIL.
Behaviour changes
get_symbol(index, ...) now returns false when the linked string table is missing, is not SHT_STRTAB, or the name is not NUL-terminated within it. Previously it returned true and left name unchanged (stale).
Lookups by name write value, size, bind, type, section_index and other only on a match. Previously a miss left them holding the last symbol examined.
get_symbols_num() returns 0 when sh_entsize is smaller than ElfN_Sym. CLR sets the entry size itself (Elf::setupShdr), so CLR-produced binaries are unaffected.
Related: upstream ELFIO #174 and TheRock backport #7772. The upstream PR carries the same hash validation, authoritative misses, output handling, and alignment-safe reads. Differences: upstream also handles GNU hash tables (not present in this copy); this copy additionally has section-scoped lookups (with SHN_UNDEF/reserved-index handling), 32-bit size_t guards, and returns false from get_symbol(index, ...) for an unresolvable name where upstream returns an empty name so that elfio_dump keeps printing the other fields.
Issue Tracking
JIRA ID: SWSPLAT-24296
Test Plan
Run local ELF32/ELF64 checks in both byte orders, covering hash traversal, authoritative misses on a valid table, local symbols missing from the hash chains, same-value symbols with unresolvable names, unnamed-symbol lookups with a hash table present, fallback on malformed tables (bad nbucket, nchain mismatch, out-of-range chain index, cycles), relocation sections linked to the symbol table, undefined and reserved section indices, unchanged outputs on a miss, symbol/name bounds, and CLR-style symbol writing. Also build for 32-bit hosts, look up every symbol of every symbol table in upstream ELFIO's tests/elf_examples by name, and exercise Elf::getSymbol()/getSymbolInfo() through amd::Elf on a written-then-read image with patched out-of-range, oversized, and end-of-section symbols. Compile elf.cpp with DEBUG_DETAIL/CL_LOG and a printf format attribute on log_printf to check the log formats.
Test Result
All local scenarios passed with GCC ASan/UBSan (x86_64) and a 32-bit (-m32) GCC build; all 10,606 example-file lookups (30 through SysV hash tables) succeed; the amd::Elf checks reject every corrupted symbol that the previous code returned an out-of-bounds pointer for; the logging build has no format warnings, including with -Wformat-signedness; MSVC ASan passed on the original revision.
Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*). Current: code file(s) changed: projects/clr/rocclr/elf/elf.cpp, projects/clr/rocclr/elf/elfio/elfio_section.hpp, projects/clr/rocclr/elf/elfio/elfio_symbols.hpp; no test file found
🚫 Draft PR
🔜 To Be Enabled
—
🚩 Feature Flag
🔜 To Be Enabled
—
📊 Code Coverage
🔜 To Be Enabled
—
🎉 All policy checks passed!
📖 Need help? See the Policy FAQ for details on every check and how to fix failures.
Check symbol record sizes and string-table references, read symbols through aligned copies, and share the name and section lookup path.
Refs: SWSPLAT-24296
Fall back to the linear symbol scan only when the SysV hash table is
unusable (bad header, nchain not matching the symbol count, out-of-range
chain index, unreadable symbol, or a cycle). A miss on a well-formed
table now returns false without scanning every symbol.
Make the 32-bit-only size_t guards explicit and macro-safe, and guard
Elf::getSymbolNum() against underflow when the symbol table is empty or
has an invalid entry size.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Match the symbol and string-table paths so hash word reads also reject
section sizes that do not fit in size_t on 32-bit hosts.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Section-scoped symbol lookups no longer treat reserved st_shndx values
(SHN_ABS, SHN_COMMON, SHN_XINDEX, ...) as real section numbers, which
could match an unrelated section in files with that many sections.
Name lookups now search into locals and write the caller's outputs only
when a symbol matches, so a miss leaves them unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SHN_UNDEF is also a special index and does not identify a section. Because ELFIO always creates section 0 with an empty name (elfio.hpp:335-342), an undefined symbol currently matches a section-scoped lookup whose section_name is empty. Reject index 0 along with the reserved high range so undefined symbols cannot be reported as section matches.
SHN_UNDEF does not identify a section. Because section 0 always exists
with an empty name, an undefined symbol could match a lookup for an
empty section name.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thanks, good catch. Fixed in 631675f: matches_symbol now rejects SHN_UNDEF as well as the reserved range (>= SHN_LORESERVE), so an undefined symbol no longer matches a lookup for section 0's empty name. Lookups by name only still return undefined symbols.
There was no practical impact on CLR, because Elf::getSymbol always passes a non-empty section name. Verified locally with an ELF64 case for an undefined symbol and an empty section name; it fails on the previous commit and passes now.
Unnamed symbols are never reachable through a SysV hash table, so an
authoritative miss made get_symbol("") fail when a .hash section was
present but succeed without one. Skip the hash table for empty names.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Elf::getSymbol() and Elf::getSymbolInfo() turned a symbol's st_value
into a pointer into its section without checking it. Reject symbols in
SHN_UNDEF, reserved, or SHT_NOBITS sections, or whose value and size do
not fit inside the section. Zero-size symbols at the section end and
symbols in empty sections are still accepted.
Print 64-bit values with PRIu64 and unsigned int with %u instead of %lu,
which is 32-bit on Windows, and print the thread id as its hash rather
than passing a std::thread::id object to %zx.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GNU ld can place named local symbols in a SysV-hashed symbol table
without putting them on a hash chain, so a hash miss now only rules out
non-local symbols; local ones are still searched.
Lookup by value no longer stops at the first symbol with that value
whose name cannot be resolved; it continues to later matches.
get_symbols_num() reports no symbols for a section without loaded data,
so a header claiming billions of entries (e.g. sh_size beyond the file)
no longer makes name lookups scan them all.
Use %u for the remaining unsigned log arguments in elf.cpp.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A non-null data pointer does not prove that the complete section was loaded. section_impl::load() (elfio_section.hpp:193-208) allocates whenever sh_size < stream_size, does not validate sh_offset + sh_size or the result of stream.read(), and leaves the pointer non-null after a short read. A truncated ELF with a small sh_size near EOF therefore still reports symbols here and lets the new symbol/hash/string checks consume the uninitialized tail; the same assumption also lets isSymbolInSection() return a pointer into unread data. Please make section loading reject/track short reads and base these checks on the actual loaded extent.
section_impl::load() allocated a section buffer whenever sh_size was
smaller than the stream, without checking sh_offset or the result of the
read. A section running past the end of a truncated image was therefore
reported as loaded with an uninitialized tail, and the failed read left
the stream in an error state so later sections failed to load.
Only load sections that lie entirely within the stream, and treat a
short read as missing data and clear the stream error, matching
upstream ELFIO's load_data().
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thanks, good catch. Fixed in 92f7df8: section_impl::load() now only loads sections that lie entirely within the image, and treats a short read as missing data (the stream error is cleared so later sections still load). This matches upstream ELFIO's load_data(), so the get_data() checks now reflect the actual loaded extent.
This strict size check still rejects a section that exactly spans the stream (sh_offset == 0 and sh_size == stream_size), even though the new bounds condition and comment define such a section as fully contained. Accept equality here; the subtraction remains safe because the size check runs first.
The bounds check rejected a section with sh_offset 0 and sh_size equal to
the stream size, although such a section lies within the stream. Accept
equality, guard size + 1 against overflow when the stream size is
unknown, and cast the checked offset for seekg() to avoid a sign
conversion.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Thanks, fixed in d31d2da: a section that exactly spans the stream is now accepted, with an explicit guard so size + 1 can't overflow when the stream size is unknown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Update symbol lookup in CLR's embedded ELFIO copy, which is maintained separately from upstream.
Technical Details
SHT_HASHsections as the symbol hash table. Previously any section whosesh_linkpointed at the symbol table was used, so a.rela*section could be read as bucket/chain words.nbucket,nchainmatching the symbol count, section bounds) and bound chain traversal. A miss on a well-formed table rules out every non-local symbol; local symbols are still searched, since GNU ld can add named local symbols to a SysV-hashed table without putting them on a chain. The full linear scan is only used when the table is unusable (bad header, out-of-range chain index, unreadable symbol, or a cycle) or the name is empty, since unnamed symbols are never reachable through the hash table.SHN_UNDEFand reservedst_shndxvalues (SHN_ABS,SHN_COMMON,SHN_XINDEX, ...) as matching no section, instead of dereferencing a null section or matching an unrelated one.section_impl::load()) only loads sections that lie entirely within the image and treats a short read as missing data. Previously a section running past the end of a truncated image was reported as loaded with an uninitialized tail, and the failed read left the stream in an error state so later sections failed to load. This matches upstream ELFIO'sload_data().get_symbols_num()reports no symbols for a section whose data was not loaded (e.g.sh_sizebeyond the file), so a header claiming billions of entries no longer makes name lookups scan them all.Elf::getSymbolNum()against underflow when the symbol table is empty or has an invalid entry size.Elf::getSymbol()andElf::getSymbolInfo()validate a symbol's data before turningst_valueinto a pointer: symbols inSHN_UNDEF, reserved, orSHT_NOBITSsections, or whose value and size do not fit inside the section, are rejected. Zero-size symbols at the section end and symbols in empty sections are still accepted.elf.cpplog formats: 64-bit values usePRIu64and unsigned values use%uinstead of%lu(32-bit on Windows) or%d, and the thread id is printed as its hash instead of passing astd::thread::idobject to%zx. The error-log macros are compiled into every build (CL_LOG), so the thread-id fix applies to release builds; the other messages only exist withDEBUG_DETAIL.Behaviour changes
get_symbol(index, ...)now returnsfalsewhen the linked string table is missing, is notSHT_STRTAB, or the name is not NUL-terminated within it. Previously it returnedtrueand leftnameunchanged (stale).value,size,bind,type,section_indexandotheronly on a match. Previously a miss left them holding the last symbol examined.get_symbols_num()returns 0 whensh_entsizeis smaller thanElfN_Sym. CLR sets the entry size itself (Elf::setupShdr), so CLR-produced binaries are unaffected.Related: upstream ELFIO #174 and TheRock backport #7772. The upstream PR carries the same hash validation, authoritative misses, output handling, and alignment-safe reads. Differences: upstream also handles GNU hash tables (not present in this copy); this copy additionally has section-scoped lookups (with
SHN_UNDEF/reserved-index handling), 32-bitsize_tguards, and returnsfalsefromget_symbol(index, ...)for an unresolvable name where upstream returns an empty name so thatelfio_dumpkeeps printing the other fields.Issue Tracking
JIRA ID: SWSPLAT-24296
Test Plan
Run local ELF32/ELF64 checks in both byte orders, covering hash traversal, authoritative misses on a valid table, local symbols missing from the hash chains, same-value symbols with unresolvable names, unnamed-symbol lookups with a hash table present, fallback on malformed tables (bad
nbucket,nchainmismatch, out-of-range chain index, cycles), relocation sections linked to the symbol table, undefined and reserved section indices, unchanged outputs on a miss, symbol/name bounds, and CLR-style symbol writing. Also build for 32-bit hosts, look up every symbol of every symbol table in upstream ELFIO'stests/elf_examplesby name, and exerciseElf::getSymbol()/getSymbolInfo()throughamd::Elfon a written-then-read image with patched out-of-range, oversized, and end-of-section symbols. Compileelf.cppwithDEBUG_DETAIL/CL_LOGand a printf format attribute onlog_printfto check the log formats.Test Result
All local scenarios passed with GCC ASan/UBSan (x86_64) and a 32-bit (
-m32) GCC build; all 10,606 example-file lookups (30 through SysV hash tables) succeed; theamd::Elfchecks reject every corrupted symbol that the previous code returned an out-of-bounds pointer for; the logging build has no format warnings, including with-Wformat-signedness; MSVC ASan passed on the original revision.Submission Checklist
Prepared with AI assistance.
🤖 Generated with Claude Code