* [RFC PATCH 0/3] objtool/klp: DWARF-based cross-build symbol correlation
@ 2026-10-26 10:19 Longjun Luo
2026-10-26 10:19 ` [RFC PATCH 1/3] objtool/klp: Correlate symbols using DWARF identity Longjun Luo
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Longjun Luo @ 2026-10-26 10:19 UTC (permalink / raw)
To: jpoimboe, peterz
Cc: mbenes, pmladek, joe.lawrence, song, puranjay, nathan,
ndesaulniers, morbo, justinstitt, live-patching, llvm,
linux-kernel, Longjun Luo
Motivation
----------
I have been working on a general-purpose livepatching mechanism,
non-intrusive (transparent to build systems) and mainly for C/C++
userspace programs.
During this work, I found that LTO optimizations make cross-build
symbol correlation particularly challenging, as the ELF symbol table
(`STT_FILE` and symbol names) lacks sufficient context to reliably
distinguish symbols across builds.
In traditional per-object diffing (`kpatch-build`), comparing
individual `.o` files naturally isolates each translation unit. LTO
crosses these boundaries and can change symbol binding through
promotion or internalization. This series continues object-level
diffing by using the DWARF DIE tree to disambiguate symbols.
To support LTO on `vmlinux.o`, `klp-build` precomputes per-symbol
checksums in `klp-checksum` and then correlates symbols in `klp-diff`
through a multi-stage heuristic pipeline:
1. Lockstep pairing of all `STT_FILE` symbols across both objects.
2. An iterative loop alternating between a 4-level filtering funnel
in `find_twin()` (demangled name -> `is_tu_local_sym()` ->
`same_file()` -> size/checksum) and `.llvm.<hash>` suffix
propagation via `suffix_map` and `find_twin_suffixed()`.
3. A last-resort relative-position match among remaining same-named
candidates in `find_twin_positional()`.
This approach is effective for most kernel patches and matches all
comparable twins in my baseline `vmlinux.o` audits (see Evaluation).
In some scenarios, however, LTO optimizations can still break its
assumptions, leading to missed correlations or silently pairing
unrelated symbols.
This RFC proposes a DWARF-derived `(source, scope, name)` identity to
complete the symbol representation layer in `klp-build`, strengthening
symbol correlation and laying a foundation for subsequent stages.
The series contains three patches:
1/3: Symbol identity and correlation.
2/3: Reference checksums using symbol identity.
3/3: Regression tests.
Limitations of ELF-only Correlation
-----------------------------------
The following minimized reproducers illustrate correlation failures
I found while analyzing kernel builds and common kernel patterns.
1. Missing lexical scope (`scope` is needed) [GCC, non-LTO]
In `fs/jfs/inode.c`, `jfs_commit_inode()` and `jfs_dirty_inode()`
each define `static int noisy = 5` to limit error messages.
A small error-reporting patch adds the inode number to the existing
`jfs_commit_inode()` diagnostic, and limits repeated writeback errors:
int jfs_commit_inode(struct inode *inode, int wait)
{
static int noisy = 5;
...
- jfs_err("jfs_commit_inode(0x%p) called on read-only volume",
- inode);
+ jfs_err("jfs_commit_inode(0x%p), ino=%llu on read-only volume",
+ inode, inode->i_ino);
...
}
int jfs_write_inode(struct inode *inode, struct writeback_control *wbc)
{
+ static atomic_t noisy = ATOMIC_INIT(5);
...
if (jfs_commit_inode(inode, wait)) {
- jfs_err("jfs_write_inode: jfs_commit_inode failed!");
+ if (atomic_dec_if_positive(&noisy) >= 0)
+ jfs_err("jfs_write_inode: commit failed for inode %llu",
+ inode->i_ino);
return -EIO;
}
...
}
Building the real `fs/jfs/inode.o` with GCC 16.2.1 and
`CONFIG_JFS_DEBUG=y` gives:
Owning function Original name Patched name
-------------------- ------------- ------------
jfs_commit_inode noisy.1 noisy.2
jfs_dirty_inode noisy.0 noisy.0
jfs_write_inode absent noisy.1
Expected: preserve the existing counters; allocate only the new
`jfs_write_inode()` counter.
Actual: Two `no correlation` warnings, but the livepatch builds
and loads successfully. The replaced `jfs_commit_inode()`
references a fresh `noisy.2` and emits the diagnostic
five more times after the original warning budget is
exhausted.
This series: preserves the original counter through its function
scope; no further diagnostics are emitted after applying
the livepatch.
Cause: Under GCC, all three 4-byte objects (initialized to 5)
strip to `demangled_name = "noisy"` under the same
`STT_FILE` without function scope; when the candidate
count changes from two to three, positional matching
gives up. The same naming pattern also occurs with the
two function-local `DEFINE_MUTEX(mutex)` instances in
`kernel/sched/rt.c`.
2. ThinLTO promotion shift swaps cross-TU state (`source` is needed)
[Clang/LLD 23.1.0 ThinLTO]
With the kernel's default ThinLTO flags
(`-always-rename-promoted-locals=false` and `-import-instr-limit=5`)
and unchanged link inputs and order:
Before (orig):
/* a.c */ static volatile int state[4];
int a_get(unsigned int i) { return state[i]; }
/* b.c */ int target(unsigned int i) { return a_get(i) + d_get(i); }
/* d.c */ static volatile int state[4];
int d_get(unsigned int i) { return state[i]; }
/* Both a_get() and d_get() are imported into b.c:
* a.c state -> STB_GLOBAL: "state"
* d.c state -> STB_GLOBAL: "state.llvm.<H>" */
After (patched, adding a bounds check to `a_get()`):
/* a.c */ static volatile int state[4];
int a_get(unsigned int i) {
+ if (i >= 4) return 0;
return state[i];
}
/* a_get() is no longer imported; d.c's state claims "state":
* a.c state -> STB_LOCAL: "state"
* d.c state -> STB_GLOBAL: "state" */
Expected: patched `a_get()` continues referencing `a.c`'s `state`.
Actual: `klp-diff` exits 0 with no warning and swaps the two
state identities: with `a.c:state[0] = 64` and
`d.c:state[0] = 32`, `a_get(0)` returns 32 instead of 64,
and the inlined `d_get()` in untouched `b.c` binds to
`a.c`'s array.
This series: `a_get(0)` returns the expected 64.
Cause: Under `-always-rename-promoted-locals=false`, ThinLTO
keeps the unsuffixed `STB_GLOBAL` name for the first
promoted static of each name in link order and gives
the others `.llvm.<H>` suffixes. When import decisions
shift, the unsuffixed name moves to another TU's
symbol, and `is_tu_local_sym()` treats both as
non-TU-local.
In a real ThinLTO kernel build, this same shift moves the unsuffixed
`m_start/m_next/m_stop/m_show` `seq_operations` callbacks from
`fs/namespace.c` to `kernel/module/procfs.c` when a 30-line patch under
`kernel/module/` reuses the `procfs.c` iterator. `klp-diff` marks all
eight callbacks as changed (including four in untouched
`fs/namespace.c`) and exits successfully without warnings. Loading
the generated module fails on duplicate `m_start,1` sysfs registration.
This series includes only the intended changes
(`read_file_mod_stats()` and `modules_seq_ops()`) and loads and rolls
back cleanly.
3. Loss of `STT_FILE` boundaries [Clang Full LTO]
Before (orig):
/* left.c */ static volatile int state;
int left_fn(void) { return ++state; }
/* right.c */ static volatile int state;
__attribute__((noinline)) static int helper(void) {
return ++state;
}
int right_fn(void) { return helper(); }
/* Full LTO ELF: STT_FILE is "ld-temp.o", 1 local "helper" */
After (patched):
/* left.c extracts ++state into a same-named noinline helper(): */
/* left.c */ static volatile int state;
+ __attribute__((noinline)) static int helper(void) {
+ return ++state;
+ }
+ int left_fn(void) { return helper(); }
/* right.c is unchanged */
/* Full LTO ELF: STT_FILE is "ld-temp.o", 2 local "helper" symbols
* with identical size and checksum */
Expected: `right.c:helper` correlates with orig;
`left.c:helper` is new.
Actual: Warns `no correlation: helper`; unchanged `right.c:helper`
loses correlation and both helpers are cloned as new
functions.
This series: correlates `right.c:helper` with orig and avoids copying
the unchanged helper (both livepatches preserve the
counters at runtime).
Cause: Full LTO merges all IR modules before codegen, collapsing
`STT_FILE` into a single `ld-temp.o` without per-TU
`.llvm.<hash>` suffixes, so ELF symbols lose their
defining TU boundaries.
Proposed Design: (source, scope, name) Identity
-----------------------------------------------
DWARF entries are matched to ELF symbols directly by relocated
`(section, offset)` address. From an in-memory perspective, each ELF
symbol is identified by `(source, scope, name)`:
- `source`: The physical translation unit (CU) that emits the instance
into memory (for `DW_AT_external` symbols, as opposed to unstable
ELF `st_bind`, `source` is set to `""`; `STT_FILE` is used only as
a fallback when DWARF is unavailable).
- `scope`: The slash-delimited chain of named enclosing scopes recorded
by DWARF for the in-memory instance (e.g., `"/"` or `"/func/"`).
- `name`:
a. `demangled_name`: objtool's normalized name with compiler
suffixes stripped.
b. `symbol name`: the raw symbol name from the ELF symbol table.
Symbol correlation proceeds in two steps (see Patch 1 for details):
1. Group symbols into `(source, scope, demangled_name)` hash buckets
and apply centralized ELF suffix compatibility rules within each
bucket.
2. Accept only unique, deterministic twins within the bucket (first by
compatible name, then using size and checksum for remaining
unchanged candidates), verified bidirectionally (`orig <-> patched`)
without positional fallback.
This series requires libdw, adds `-g` to the klp-build compilation, and
currently rejects split DWARF.
Example (`source` and `scope` reflect post-optimization DWARF):
/* include/header.h */
static inline int record(int x) {
if (x > 0) {
static int state;
return ++state;
}
return 0;
}
/* net/a.c */
int tx(int x) { return record(x); }
From a static source-text view, `state` is written in `include/header.h`
inside an `if` lexical block, and `record()` is inlined into `tx()`.
From the in-memory DWARF perspective:
- `source` is `"net/a.c"` (the physical CU that allocates `state` in
memory, not `"include/header.h"`).
- `scope` is `"/record/"`: the scope construction follows the owning
subprogram and skips unnamed lexical blocks. Inlining into `tx()`
does not change the variable's owning scope.
Symbol Correlation Evaluation
-----------------------------
This series accepts unique, bidirectional 1:1 matches without
positional guessing. In a cross-audit of x86-64 `vmlinux.o` builds,
I compared klp-build's twins with those independently derived using
my standalone DWARF-based symbol identity tool. The results agree on
all comparable twin pairs:
Build pair Comparable pairs Discrepancies
--------------------------------- ---------------- -------------
Clang ThinLTO: getpid 91,300 0
Clang ThinLTO: ACPICA 91,300 0
Clang Full LTO: version_show 91,171 0
GCC non-LTO: version_show 95,337 0
The builds use DWARF 5: ThinLTO uses Clang/LLD 22.1.8
on Linux 7.3-rc7 `24a19b953e2a`; GCC 16.2.1 / GNU ld 2.46.1 and
Clang/LLD 22.1.8 Full LTO use mainline `4403278e4ea2`.
Within this set, the twins retained by this series agree with those
derived from my tool. In the ThinLTO builds, this series leaves 363
data symbols per side uncorrelated that klp-build pairs heuristically:
DWARF information Count Follow-up
------------------------------------- ----- -------------------------
Available (pieces/value expressions) 89 Extend reader support.
Incomplete (anonymous lexical scopes) 154 Detect ambiguity and
reject.
Absent (C compound literals) 120 Compare references from
changed functions; reuse
unchanged objects.
These symbols are reported as `no correlation` instead of being paired
by guesswork. Until the follow-ups land, changed functions that
reference them still get cloned copies, as in Case 1 with the existing
`klp-build`. I have tested follow-up patches for the table above and
will post them separately.
The upstream and added tests report 51 pass / 3 fail / 7 skip with GCC
and 57 pass / 3 fail / 1 skip with Clang (the three failures are policy
rejections of linkage flips and duplicate-TU compilations), and the
kernel cases above were verified in QEMU (apply, behavior, and rollback).
The `(source, scope, name)` tuple also strengthens `klp-checksum`'s
representation layer. Compressing references into a `demangled_name`
digest loses resolution and misses switches between same-named targets
of different origin or linkage [1]. Hashing `source/scope/demangled_name`
(Patch 2) resolves this while keeping compiler suffix changes
transparent, confirmed by runtime tests across two GCC cases and one
ThinLTO case.
For `klp checksum --dry-run` on `vmlinux.o`, the time and peak RSS go
from 4.546 s / 3984.8 MiB to 13.752 s / 5825.3 MiB with this series,
or 10.823 s / 4870.0 MiB with the optimized reader.
Compared with the comparison approach used in my userspace
implementation, `klp-checksum`'s single-object digest is faster and
simpler; adding the tuple preserves these advantages while recovering
target-identity precision.
On the pre-checksummed ThinLTO `vmlinux.o` pair (~1.081 GiB per input,
353.8 MiB `.debug_info`), `objtool klp diff` medians from three warm runs
on an Intel Core i7-11850H (30.6 GiB RAM, libdw 0.196) are:
Version Elapsed time CPU time Peak RSS
--------------------------- ------------ -------- ----------
klp-build baseline 3.816 s 3.814 s 3566.3 MiB
This series 22.404 s 22.399 s 5470.3 MiB
Optimized (single-threaded) 16.441 s 16.437 s 3714.0 MiB
This series prioritizes readability and correctness. The single-threaded
optimized prototype produces the same twin tables and patch outputs as
this series. It takes 16.441 s and adds 147.7 MiB (+4.1%) of peak memory
over the baseline. These optimizations will be submitted separately.
DWARF Reliability & Scenarios
-----------------------------
Correlation uses CU paths, named scopes, `DW_AT_external` and fixed
addresses from the DWARF DIE tree. It does not require CFI, line tables
or dynamic location lists. This reduces dependence on compiler-specific
flags and version-dependent naming behavior (as seen in Case 2 between
LLVM 22.1.8 and LLVM 23.1.0).
In my userspace livepatching implementation, I evaluated the identity
model across 17 security-fix cases in 15 C/C++ projects:
- Databases & Caches: Redis, PostgreSQL, MariaDB
- Web & Network: Nginx, Squid, BIND, OpenSSH, Samba, Netatalk
- System & Libraries: glibc, OpenSSL, QEMU, GStreamer, Poppler, libjxl
- Build Configurations: DWARF 4/5;
GCC (non-LTO, single/multi-partition LTO);
Clang/LLD (non-LTO, Full LTO, ThinLTO)
- Distributions: Debian 13, Fedora 44, openEuler 24.03 LTS SP3,
openSUSE Leap 16.0, RHEL UBI 10.2, Ubuntu 24.04
- Toolchains: GCC 12-16 and Clang 17-22
For Nginx CVE-2026-27654, all 30 configurations across six distributions
and five compiler/LTO modes completed patch application, behavior
verification and rollback. The Redis and OpenSSH cases also completed
application and rollback tests across all six distributions.
Strict cross-verification during this work also led to several upstream
toolchain bug reports, fixes and a DWARF proposal [2-9]. All of these
corner cases are either fixed upstream or mitigated in my standalone
tool and this series, confirming that DWARF DIE trees from current
toolchains provide sufficient and reliable information for this
correlation approach.
More broadly, symbol correlation across build stages under LTO is a common
problem across several domains, such as target-symbol resolution in
`klp-build` (`.klp.symid` / `sympos`) and profile correlation in
AutoFDO, where this same identity model can also be applied.
Related Work & References
-------------------------
Related work includes my earlier kpatch-build contributions [10-11]
and my reports and fixes in GCC, LLVM, GNU binutils, GDB, LLDB and
AutoFDO, covering DWARF generation, address interpretation and source
attribution [2-9].
[1] klp-checksum reference target ambiguity report:
https://lore.kernel.org/live-patching/20260910085218.66565-1-luolongjuna@gmail.com/
[2] GCC PR126355 (address-bearing DIEs for IPA-ICF wrappers, merged):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126355
[3] GCC PR126348 (abstract-origin TU provenance across LTO partitions):
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=126348
[4] DWARF 6 proposal (separate definition address from linker bindings):
https://lists.dwarfstd.org/pipermail/dwarf-discuss/2026-September/002859.html
[5] LLDB #233545 (fix overridden weak function lookup, open PR):
https://github.com/llvm/llvm-project/pull/233545
[6] GDB PR34741 (overridden weak definition address resolution):
https://sourceware.org/bugzilla/show_bug.cgi?id=34741
[7] LLVM #225358 (fix misleading inline attribution in store merging):
https://github.com/llvm/llvm-project/pull/225358
[8] AutoFDO #267 (fix absolute DWARF 5 range handling, merged):
https://github.com/google/autofdo/pull/267
[9] GNU AS (fix CU source selection for preprocessed assembly):
https://sourceware.org/pipermail/binutils/2026-October/152209.html
[10] kpatch #1301 (fix missing .rela section handling, merged):
https://github.com/dynup/kpatch/pull/1301
[11] kpatch #1303 (strengthen section-change relocation check, merged):
https://github.com/dynup/kpatch/pull/1303
Longjun Luo (3):
objtool/klp: Correlate symbols using DWARF identity
objtool/klp: Include symbol identities in reference checksums
objtool/klp: Test DWARF identities and reference checksums
scripts/livepatch/klp-build | 5 +-
tools/objtool/Build | 4 +-
tools/objtool/Makefile | 12 +-
tools/objtool/elf.c | 33 +-
tools/objtool/include/objtool/elf.h | 34 +-
tools/objtool/include/objtool/klp-dwarf.h | 21 +
tools/objtool/include/objtool/klp-identity.h | 26 +
tools/objtool/include/objtool/klp.h | 2 +
tools/objtool/klp-checksum.c | 25 +-
tools/objtool/klp-diff.c | 563 ++++---------
tools/objtool/klp-dwarf.c | 778 ++++++++++++++++++
tools/objtool/klp-identity.c | 177 ++++
.../tests/generic/fixtures/identity/common.h | 15 +
.../generic/fixtures/identity/consumer.c | 30 +
.../tests/generic/fixtures/identity/entries.c | 10 +
.../generic/fixtures/identity/full_left.c | 16 +
.../generic/fixtures/identity/full_right.c | 6 +
.../generic/fixtures/identity/move_left.c | 9 +
.../generic/fixtures/identity/move_right.c | 8 +
.../generic/fixtures/identity/new/same.c | 6 +
.../generic/fixtures/identity/new_driver.c | 15 +
.../generic/fixtures/identity/new_provider.c | 4 +
.../generic/fixtures/identity/old/same.c | 6 +
.../fixtures/identity/promotion_bounds_a.c | 12 +
.../fixtures/identity/promotion_bounds_b.c | 12 +
.../fixtures/identity/promotion_bounds_d.c | 8 +
.../generic/fixtures/identity/provider.c | 9 +
.../generic/fixtures/identity/readonly.c | 30 +
.../tests/generic/fixtures/identity/record.h | 8 +
.../tests/generic/fixtures/identity/rx.c | 4 +
.../generic/fixtures/identity/source_driver.c | 11 +
.../generic/fixtures/identity/static_scope.c | 31 +
.../fixtures/identity/static_scope_reorder.c | 31 +
.../tests/generic/fixtures/identity/tx.c | 4 +
.../generic/test-checksum-identity-source.sh | 24 +
.../tests/generic/test-checksum-identity.sh | 31 +
.../tests/generic/test-identity-full-lto.sh | 28 +
.../tests/generic/test-identity-new-tu.sh | 22 +
.../generic/test-identity-readonly-address.sh | 23 +
.../tests/generic/test-identity-source.sh | 26 +
.../generic/test-identity-static-scope.sh | 35 +
.../test-identity-thinlto-name-drift.sh | 65 ++
.../test-identity-thinlto-promotion.sh | 25 +
.../tests/generic/test-thinlto-ambiguity.sh | 2 +-
.../tests/generic/test-thinlto-local.sh | 4 +-
tools/objtool/tests/identity-lib.sh | 65 ++
tools/objtool/tests/lib.sh | 7 +-
47 files changed, 1865 insertions(+), 457 deletions(-)
create mode 100644 tools/objtool/include/objtool/klp-dwarf.h
create mode 100644 tools/objtool/include/objtool/klp-identity.h
create mode 100644 tools/objtool/klp-dwarf.c
create mode 100644 tools/objtool/klp-identity.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/common.h
create mode 100644 tools/objtool/tests/generic/fixtures/identity/consumer.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/entries.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/full_left.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/full_right.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/move_left.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/move_right.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/new/same.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/new_driver.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/new_provider.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/old/same.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_a.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_b.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_d.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/provider.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/readonly.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/record.h
create mode 100644 tools/objtool/tests/generic/fixtures/identity/rx.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/source_driver.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/static_scope.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/static_scope_reorder.c
create mode 100644 tools/objtool/tests/generic/fixtures/identity/tx.c
create mode 100755 tools/objtool/tests/generic/test-checksum-identity-source.sh
create mode 100755 tools/objtool/tests/generic/test-checksum-identity.sh
create mode 100755 tools/objtool/tests/generic/test-identity-full-lto.sh
create mode 100755 tools/objtool/tests/generic/test-identity-new-tu.sh
create mode 100755 tools/objtool/tests/generic/test-identity-readonly-address.sh
create mode 100755 tools/objtool/tests/generic/test-identity-source.sh
create mode 100755 tools/objtool/tests/generic/test-identity-static-scope.sh
create mode 100755 tools/objtool/tests/generic/test-identity-thinlto-name-drift.sh
create mode 100755 tools/objtool/tests/generic/test-identity-thinlto-promotion.sh
create mode 100644 tools/objtool/tests/identity-lib.sh
base-commit: 945537271df1e3680be9b32af7041ac20f1184fb
--
2.53.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [RFC PATCH 1/3] objtool/klp: Correlate symbols using DWARF identity 2026-10-26 10:19 [RFC PATCH 0/3] objtool/klp: DWARF-based cross-build symbol correlation Longjun Luo @ 2026-10-26 10:19 ` Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 2/3] objtool/klp: Include symbol identities in reference checksums Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 3/3] objtool/klp: Test DWARF identities and " Longjun Luo 2 siblings, 0 replies; 4+ messages in thread From: Longjun Luo @ 2026-10-26 10:19 UTC (permalink / raw) To: jpoimboe, peterz Cc: mbenes, pmladek, joe.lawrence, song, puranjay, nathan, ndesaulniers, morbo, justinstitt, live-patching, llvm, linux-kernel, Longjun Luo ELF names and STT_FILE entries do not provide enough context to distinguish symbols across builds. Compiler renumbering and LTO can cause missed matches or silently pair unrelated symbols. Extend symbol correlation to (source, scope, name). Use DWARF to recover the defining translation unit and named scope, with STT_FILE as a fallback. Retain checksums to resolve remaining ambiguity. Accept only unique matches. Report ambiguous symbols and leave them uncorrelated instead of pairing them by position. Building objtool's klp support now requires libdw. klp-build compiles the kernel with -g (KCFLAGS and KAFLAGS) so that DWARF is available regardless of the kernel's debug-info configuration, and the klp test fixtures do the same. Split DWARF is rejected for now. Tested independently on x86-64 with tools/objtool/tests/run-tests.sh: GCC 16.2.1: 45 passed, 3 failed, 4 skipped. Clang 22.1.8: 48 passed, 3 failed, 1 skipped. The three failures reflect deliberate rejections: - local-to-global-flip: the patch changes a symbol's source-level linkage (DW_AT_external, not ELF st_bind, so LTO promotion is not affected). A static symbol's identity carries its defining CU while an external one's does not, so the two are different identities. Pairing them would need a rule outside the identity model; the patch can instead keep the static symbol and add a global wrapper. - sympos, sympos-vmlinux: the same source is compiled into multiple objects, producing instances with identical (source, scope, name). Telling them apart would depend on object or link order, which this series deliberately avoids. This does not occur in the audited vmlinux.o builds. Assisted-by: LLM Signed-off-by: Longjun Luo <luolongjuna@gmail.com> --- scripts/livepatch/klp-build | 5 +- tools/objtool/Build | 4 +- tools/objtool/Makefile | 12 +- tools/objtool/elf.c | 33 +- tools/objtool/include/objtool/elf.h | 34 +- tools/objtool/include/objtool/klp-dwarf.h | 21 + tools/objtool/include/objtool/klp-identity.h | 25 + tools/objtool/klp-checksum.c | 16 +- tools/objtool/klp-diff.c | 561 ++++--------- tools/objtool/klp-dwarf.c | 778 ++++++++++++++++++ tools/objtool/klp-identity.c | 170 ++++ .../tests/generic/test-thinlto-ambiguity.sh | 2 +- .../tests/generic/test-thinlto-local.sh | 4 +- tools/objtool/tests/lib.sh | 7 +- 14 files changed, 1217 insertions(+), 455 deletions(-) create mode 100644 tools/objtool/include/objtool/klp-dwarf.h create mode 100644 tools/objtool/include/objtool/klp-identity.h create mode 100644 tools/objtool/klp-dwarf.c create mode 100644 tools/objtool/klp-identity.c diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build index b52a8489d9f6..0f141dc06fd2 100755 --- a/scripts/livepatch/klp-build +++ b/scripts/livepatch/klp-build @@ -279,7 +279,7 @@ validate_config() { die "Clang assembler version < 20 not supported" [[ -x "$OBJTOOL" ]] && "$OBJTOOL" klp 2>&1 | command grep -q "not implemented" && \ - die "objtool not built with KLP support; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile" + die "objtool not built with KLP support; install xxhash-devel/libxxhash-dev (version >= 0.8) and elfutils-devel/libdw-dev and recompile" return 0 } @@ -566,7 +566,8 @@ build_kernel() { cmd+=("-s") fi cmd+=("-j$JOBS") - cmd+=("KCFLAGS=-ffunction-sections -fdata-sections") + cmd+=("KCFLAGS=-g -ffunction-sections -fdata-sections") + cmd+=("KAFLAGS=-g") cmd+=("vmlinux") cmd+=("modules") diff --git a/tools/objtool/Build b/tools/objtool/Build index 59f948628098..84139709a2dd 100644 --- a/tools/objtool/Build +++ b/tools/objtool/Build @@ -13,8 +13,8 @@ objtool-$(BUILD_DISAS) += disas.o objtool-$(BUILD_DISAS) += trace.o objtool-$(BUILD_ORC) += orc_gen.o orc_dump.o -objtool-$(BUILD_KLP) += builtin-klp.o klp-checksum.o klp-diff.o \ - klp-post-link.o klp-sympos.o +objtool-$(BUILD_KLP) += builtin-klp.o klp-checksum.o klp-diff.o klp-dwarf.o \ + klp-post-link.o klp-sympos.o klp-identity.o objtool-y += libstring.o objtool-y += libctype.o diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile index db99c4759b2c..e2fbd778bb74 100644 --- a/tools/objtool/Makefile +++ b/tools/objtool/Makefile @@ -14,7 +14,12 @@ endif ifeq ($(ARCH_HAS_KLP),y) HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \ $(HOSTCC) $(HOSTCFLAGS) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n) - ifeq ($(HAVE_XXHASH),y) + LIBDW_CFLAGS := $(shell $(HOSTPKG_CONFIG) libdw --cflags 2>/dev/null) + LIBDW_LIBS := $(shell $(HOSTPKG_CONFIG) libdw --libs 2>/dev/null || echo -ldw) + HAVE_LIBDW = $(shell printf "$(pound)include <elfutils/libdwfl.h>\nint main(void) { return !dwfl_begin(0); }" | \ + $(HOSTCC) $(HOSTCFLAGS) $(LIBDW_CFLAGS) -xc - -o /dev/null \ + $(HOSTLDFLAGS) $(LIBDW_LIBS) 2>/dev/null && echo y || echo n) + ifeq ($(HAVE_XXHASH)$(HAVE_LIBDW),yy) BUILD_KLP := y LIBXXHASH_CFLAGS := $(shell $(HOSTPKG_CONFIG) libxxhash --cflags 2>/dev/null) \ -DBUILD_KLP @@ -65,6 +70,11 @@ OBJTOOL_CFLAGS := -std=gnu11 -fomit-frame-pointer -O2 -g $(WARNINGS) \ OBJTOOL_LDFLAGS := $(LIBSUBCMD) $(LIBELF_LIBS) $(LIBXXHASH_LIBS) $(HOSTLDFLAGS) +ifeq ($(BUILD_KLP),y) +OBJTOOL_CFLAGS += $(LIBDW_CFLAGS) +OBJTOOL_LDFLAGS += $(LIBDW_LIBS) +endif + # Allow old libelf to be used: elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(HOSTCC) $(OBJTOOL_CFLAGS) -x c -E - 2>/dev/null | grep elf_getshdr) OBJTOOL_CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED) diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c index a791f4ea6ec1..2602e6956993 100644 --- a/tools/objtool/elf.c +++ b/tools/objtool/elf.c @@ -9,7 +9,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include <sys/mman.h> #include <fcntl.h> #include <stdio.h> #include <stdlib.h> @@ -20,7 +19,6 @@ #include <linux/align.h> #include <linux/kernel.h> #include <linux/interval_tree_generic.h> -#include <linux/log2.h> #include <objtool/builtin.h> #include <objtool/elf.h> #include <objtool/klp.h> @@ -33,13 +31,6 @@ u32 str_hash_demangled(const char *str) return jhash(str, demangled_name_len(str), 0); } -#define elf_hash_add(name, node, key) \ -({ \ - struct elf_hash_node *__node = node; \ - __node->next = __elf_table_entry(elf, name, key); \ - __elf_table_entry(elf, name, key) = __node; \ -}) - static inline void __elf_hash_del(struct elf_hash_node *node, struct elf_hash_node **head) { @@ -61,20 +52,6 @@ static inline void __elf_hash_del(struct elf_hash_node *node, #define elf_hash_del(name, node, key) \ __elf_hash_del(node, &__elf_table_entry(elf, name, key)) -#define elf_alloc_hash(name, size) \ -({ \ - __elf_bits(elf, name) = max(10, ilog2(size)); \ - __elf_table(elf, name) = mmap(NULL, \ - sizeof(struct elf_hash_node *) << __elf_bits(elf, name), \ - PROT_READ|PROT_WRITE, \ - MAP_PRIVATE|MAP_ANON, -1, 0); \ - if (__elf_table(elf, name) == (void *)-1L) { \ - ERROR_GLIBC("mmap fail " #name); \ - __elf_table(elf, name) = NULL; \ - } \ - __elf_table(elf, name); \ -}) - static inline unsigned long __sym_start(struct symbol *s) { return s->offset; @@ -538,13 +515,13 @@ static int elf_add_symbol(struct elf *elf, struct symbol *sym) entry = &sym->sec->symbol_list; list_add(&sym->list, entry); - sym->demangled_name = demangle_name(sym); - if (!sym->demangled_name) + sym->identity.demangled_name = demangle_name(sym); + if (!sym->identity.demangled_name) return -1; list_add_tail(&sym->global_list, &elf->symbols); elf_hash_add(symbol, &sym->hash, sym->idx); - elf_hash_add(symbol_name, &sym->name_hash, str_hash(sym->demangled_name)); + elf_hash_add(symbol_name, &sym->name_hash, str_hash(sym->identity.demangled_name)); if (is_func_sym(sym) && (strstarts(sym->name, "__pfx_") || @@ -660,6 +637,10 @@ static int read_symbols(struct elf *elf) if (elf_add_symbol(elf, sym)) return -1; + /* + * FIXME: Assembly inputs without STT_FILE can inherit the preceding + * input's FILE after partial linking. + */ if (is_file_sym(sym)) file = sym; else if (sym->bind == STB_LOCAL && !is_sec_sym(sym)) diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h index a82517a76a0f..ef5d74751a9e 100644 --- a/tools/objtool/include/objtool/elf.h +++ b/tools/objtool/include/objtool/elf.h @@ -7,15 +7,18 @@ #define _OBJTOOL_ELF_H #include <stdio.h> +#include <sys/mman.h> #include <gelf.h> #include <linux/string.h> #include <linux/list.h> #include <linux/hashtable.h> +#include <linux/log2.h> #include <linux/rbtree.h> #include <linux/jhash.h> #include <objtool/endianness.h> #include <objtool/checksum_types.h> +#include <objtool/klp-identity.h> #include <arch/elf.h> #define SEC_NAME_LEN 1024 @@ -70,9 +73,11 @@ struct symbol { struct rb_node node; struct elf_hash_node hash; struct elf_hash_node name_hash; + struct elf_hash_node identity_hash; GElf_Sym sym; struct section *sec; - const char *name, *demangled_name; + const char *name; + struct identity identity; unsigned int idx, len; unsigned long offset; unsigned long __subtree_last; @@ -125,12 +130,16 @@ struct elf { int symbol_bits; int symbol_name_bits; + int symbol_identity_bits; + int dwarf_address_bits; int section_bits; int section_name_bits; int reloc_bits; struct elf_hash_node **symbol_hash; struct elf_hash_node **symbol_name_hash; + struct elf_hash_node **symbol_identity_hash; + struct elf_hash_node **dwarf_address_hash; struct elf_hash_node **section_hash; struct elf_hash_node **section_name_hash; struct elf_hash_node **reloc_hash; @@ -145,6 +154,27 @@ struct elf { #define __elf_table_entry(elf, name, key) \ __elf_table(elf, name)[hash_min(key, __elf_bits(elf, name))] +#define elf_alloc_hash(name, size) \ +({ \ + __elf_bits(elf, name) = max(10, ilog2(size)); \ + __elf_table(elf, name) = mmap(NULL, \ + sizeof(struct elf_hash_node *) << __elf_bits(elf, name), \ + PROT_READ|PROT_WRITE, \ + MAP_PRIVATE|MAP_ANON, -1, 0); \ + if (__elf_table(elf, name) == (void *)-1L) { \ + ERROR_GLIBC("mmap fail " #name); \ + __elf_table(elf, name) = NULL; \ + } \ + __elf_table(elf, name); \ +}) + +#define elf_hash_add(name, node, key) \ +({ \ + struct elf_hash_node *__node = node; \ + __node->next = __elf_table_entry(elf, name, key); \ + __elf_table_entry(elf, name, key) = __node; \ +}) + #define elf_list_entry(ptr, type, member) \ ({ \ typeof(ptr) __ptr = (ptr); \ @@ -516,7 +546,7 @@ static inline void set_sym_next_reloc(struct reloc *reloc, struct reloc *next) #define for_each_sym_by_demangled_name(elf, name, sym) \ elf_hash_for_each_possible(elf, symbol_name, sym, name_hash, \ str_hash(name)) \ - if (strcmp(sym->demangled_name, name)) {} else + if (strcmp(sym->identity.demangled_name, name)) {} else #define rsec_next_reloc(rsec, reloc) \ reloc_idx(reloc) < sec_num_entries(rsec) - 1 ? reloc + 1 : NULL diff --git a/tools/objtool/include/objtool/klp-dwarf.h b/tools/objtool/include/objtool/klp-dwarf.h new file mode 100644 index 000000000000..566f79f54c63 --- /dev/null +++ b/tools/objtool/include/objtool/klp-dwarf.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> + */ + +#ifndef _OBJTOOL_KLP_DWARF_H +#define _OBJTOOL_KLP_DWARF_H + +struct elf; +struct symbol; + +int init_identity_cache(struct elf *elf); + +/* + * Set source, scope and external in sym->identity from DWARF. + * Return 0 if found, 1 if unavailable, or -1 for invalid/conflicting DWARF. + * Returned strings belong to elf and must not be freed by the caller. + */ +int get_identity_for_symbol(struct elf *elf, struct symbol *sym); + +#endif diff --git a/tools/objtool/include/objtool/klp-identity.h b/tools/objtool/include/objtool/klp-identity.h new file mode 100644 index 000000000000..0c7be8cfd49b --- /dev/null +++ b/tools/objtool/include/objtool/klp-identity.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> + */ + +#ifndef _OBJTOOL_KLP_IDENTITY_H +#define _OBJTOOL_KLP_IDENTITY_H + +#include <linux/types.h> + +struct elf; + +struct identity { + const char *source; /* physical defining TU path, or "" */ + const char *scope; /* slash-delimited enclosing scopes */ + const char *demangled_name; /* suffix-stripped symbol name */ + bool external; +}; + +/* The caller must set symbol->dont_correlate before preparing identities. */ +int prepare_identities(struct elf *elf); +u32 identity_hash(const struct identity *identity); +bool same_identity(const struct identity *a, const struct identity *b); + +#endif diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c index ebe25f9c5260..1f1b5f068168 100644 --- a/tools/objtool/klp-checksum.c +++ b/tools/objtool/klp-checksum.c @@ -98,8 +98,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, __checksum_update_insn(func, insn, buf, len); if (call_dest) { - __checksum_update_insn(func, insn, call_dest->demangled_name, - strlen(call_dest->demangled_name)); + __checksum_update_insn(func, insn, call_dest->identity.demangled_name, + strlen(call_dest->identity.demangled_name)); } else if (jump_dest) { struct symbol *dest_sym; @@ -115,8 +115,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, if (!dest_sym) goto alts; - __checksum_update_insn(func, insn, dest_sym->demangled_name, - strlen(dest_sym->demangled_name)); + __checksum_update_insn(func, insn, dest_sym->identity.demangled_name, + strlen(dest_sym->identity.demangled_name)); offset = jump_dest->offset - dest_sym->offset; __checksum_update_insn(func, insn, &offset, sizeof(offset)); @@ -158,8 +158,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, offset -= sym->offset; } - __checksum_update_insn(func, insn, sym->demangled_name, - strlen(sym->demangled_name)); + __checksum_update_insn(func, insn, sym->identity.demangled_name, + strlen(sym->identity.demangled_name)); __checksum_update_insn(func, insn, &offset, sizeof(offset)); alts: @@ -228,8 +228,8 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym } __checksum_update_object(sym, sym_offset, "reloc name", - target->demangled_name, - strlen(target->demangled_name)); + target->identity.demangled_name, + strlen(target->identity.demangled_name)); __checksum_update_object(sym, sym_offset, "reloc addend", &offset, sizeof(offset)); } diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c index 34da55e45609..957e589b6603 100644 --- a/tools/objtool/klp-diff.c +++ b/tools/objtool/klp-diff.c @@ -3,7 +3,6 @@ #include <subcmd/parse-options.h> #include <stdlib.h> #include <string.h> -#include <libgen.h> #include <stdio.h> #include <ctype.h> @@ -11,6 +10,7 @@ #include <objtool/warn.h> #include <objtool/arch.h> #include <objtool/klp.h> +#include <objtool/klp-identity.h> #include <objtool/util.h> #include <objtool/special.h> @@ -19,7 +19,7 @@ #include <linux/livepatch_external.h> #include <linux/stringify.h> #include <linux/string.h> -#include <linux/jhash.h> +#include <linux/hashtable.h> #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) @@ -260,28 +260,6 @@ static int read_sym_checksums(struct elf *elf) return 0; } -static struct symbol *first_file_symbol(struct elf *elf) -{ - struct symbol *sym; - - for_each_sym(elf, sym) { - if (is_file_sym(sym)) - return sym; - } - - return NULL; -} - -static struct symbol *next_file_symbol(struct elf *elf, struct symbol *sym) -{ - for_each_sym_continue(elf, sym) { - if (is_file_sym(sym)) - return sym; - } - - return NULL; -} - /* * Certain static local variables should never be correlated. They will be * used in place rather than referencing the originals. @@ -438,6 +416,11 @@ static bool is_anonymous_rodata(struct symbol *sym) (!is_object_sym(sym) || strstarts(sym->sec->name, ".rodata.cst")); } +static bool is_metadata_section(struct section *sec) +{ + return !strcmp(sec->name, ".modinfo"); +} + /* * These symbols should never be correlated, so their local patched versions * are used instead of linking to the originals. @@ -455,361 +438,157 @@ static bool dont_correlate(struct symbol *sym) is_anonymous_rodata(sym) || is_initcall_sym(sym) || is_addressable_sym(sym) || + is_metadata_section(sym->sec) || is_special_section(sym->sec) || is_special_section_aux(sym->sec); } -static const char *llvm_suffix(const char *name) +static size_t llvm_name_len(const char *name) { - return strstr(name, ".llvm."); -} + const char *suffix = strstr(name, ".llvm."), *p; -static bool is_llvm_sym(struct symbol *sym) -{ - return llvm_suffix(sym->name); + if (!suffix || !isdigit((unsigned char)suffix[6])) + return strlen(name); + for (p = suffix + 6; isdigit((unsigned char)*p); p++) + ; + return *p ? strlen(name) : (size_t)(suffix - name); } -/* - * Determine if two symbols have compatible source file origins: - * - * - If both symbols are local, only return true if they belong to the same - * ELF file symbol. - * - * - If both symbols are global, always return true, as globals don't have - * file associations. - * - * - If they have different scopes, also return true, as the patch might have - * changed the symbol's scope. - * - * Works for both same-ELF (direct pointer compare) and cross-ELF - * (compare via file->twin) cases. - */ -static bool maybe_same_file(struct symbol *sym1, struct symbol *sym2) +/* Keep object-fragment numbers, e.g. amd_northbridges.0. */ +static bool same_llvm_name(const char *a, const char *b) { - if (!sym1->file || !sym2->file) - return true; - if (sym1->file == sym2->file) - return true; - return sym1->file->twin == sym2->file; + size_t a_len = llvm_name_len(a), b_len = llvm_name_len(b); + + return a_len == b_len && !strncmp(a, b, a_len); } -/* - * Similar to maybe_same_file(), but strict: no scope changes allowed. - * - * Works for both same-ELF (direct pointer compare) and cross-ELF - * (compare via file->twin) cases. - */ -static bool same_file(struct symbol *sym1, struct symbol *sym2) +static const char *skip_lto_priv(const char *name) { - if (llvm_suffix(sym1->name) && llvm_suffix(sym2->name)) - return true; - if (!sym1->file && !sym2->file) - return true; - if (!sym1->file || !sym2->file) - return false; - if (sym1->file == sym2->file) - return true; - return sym1->file->twin == sym2->file; + if (!strstarts(name, ".lto_priv.") || !isdigit((unsigned char)name[10])) + return name; + name += 10; + while (isdigit((unsigned char)*name)) + name++; + return name; } -/* - * Is it a local symbol, or at least was it local in the translation unit - * before LLVM promoted it? - */ -static bool is_tu_local_sym(struct symbol *sym) +static bool is_numeric_suffix(const char *name) { - return is_local_sym(sym) || is_llvm_sym(sym); + if (*name++ != '.' || !isdigit((unsigned char)*name)) + return false; + while (isdigit((unsigned char)*name)) + name++; + return !*name; } -/* - * Try to find sym1's twin in patched using deterministic matching. - * - * Multiple symbols can share a demangled name (e.g., static functions in - * different TUs). This function counts same-named candidates through a - * funnel of progressively tighter filters. Each level is a strict subset - * of the previous one. - * - * The widest level that yields a 1:1 match wins. Narrower levels are only - * needed when the wider level is ambiguous (count > 1). - * - * Candidates are pre-filtered by maybe_same_file(), which narrows most - * local symbols to their own TU. For example, 19 different static - * type_show() functions across vmlinux.o each see only one candidate after - * pre-filtering, so they match immediately at Level 1. - * - * Level 1 (name): Works when the demangled name is unique after - * pre-filtering. Handles most symbols: unique globals like copy_signal(), - * or per-TU locals like pcspkr_probe(). - * - * Level 2 (scope): Filters by local-vs-global (TU-local-vs-not). Example: - * parse_header() exists as both a static and a global function. Level 1 - * sees both (same demangled name), but Level 2 separates them by scope. - * - * Level 3 (file): Strict file matching via same_file(), which rejects scope - * changes. Example: LLVM-promoted foo.llvm.12345 (global, no FILE symbol) - * vs genuine local foo (has FILE symbol). Both are TU-local so Level 2 - * can't distinguish them, but same_file() rejects the pair because one has - * a file association and the other doesn't. - * - * Level 4 (checksum): Distinguishes by function checksum. Example: - * usb_devnode.llvm.AAA and usb_devnode.llvm.BBB are two LLVM-promoted - * functions from different TUs with the same demangled name. After a TU - * change, the .llvm. hashes change but the functions themselves may be - * unchanged. Level 4 matches each to the patched candidate with the - * same checksum. - */ -static struct symbol *find_twin(struct elfs *e, struct symbol *sym1) +/* Allow compiler counters and GCC partition renames to change. */ +static bool same_compiler_name(const char *a, const char *b) { - struct symbol *name_last = NULL, *scope_last = NULL, - *file_last = NULL, *csum_last = NULL; - unsigned int name_orig = 0, name_patched = 0; - unsigned int scope_orig = 0, scope_patched = 0; - unsigned int file_orig = 0, file_patched = 0; - unsigned int csum_orig = 0, csum_patched = 0; - struct symbol *sym2, *match = NULL; - - /* Count orig candidates */ - for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) { - if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate || - (!maybe_same_file(sym1, sym2))) - continue; - - /* Level 1: name match (widest filter) */ - name_orig++; - - /* Level 2: scope (scope changes allowed) */ - if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2)) - continue; - scope_orig++; - - /* Level 3: file (scope changes disallowed) */ - if (!same_file(sym1, sym2)) - continue; - file_orig++; - - /* Level 4: checksum (unchanged symbols) */ - if (sym1->len != sym2->len || !sym1->csum.checksum || - sym1->csum.checksum != sym2->csum.checksum) - continue; - csum_orig++; - } - - /* Count patched candidates */ - for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) { - if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate || - !maybe_same_file(sym1, sym2)) - continue; - - /* Level 1 */ - name_patched++; - name_last = sym2; - - /* Level 2 */ - if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2)) - continue; - scope_patched++; - scope_last = sym2; + for (;;) { + const char *next_a = skip_lto_priv(a), *next_b = skip_lto_priv(b); - /* Level 3 */ - if (!same_file(sym1, sym2)) + if (next_a != a || next_b != b) { + a = next_a; + b = next_b; continue; - file_patched++; - file_last = sym2; - - /* Level 4 */ - if (sym1->len != sym2->len || !sym1->csum.checksum || - sym1->csum.checksum != sym2->csum.checksum) - continue; - csum_patched++; - csum_last = sym2; + } + if (*a != *b) + break; + if (!*a) + return true; + if (*a == '.' && isdigit((unsigned char)a[1])) { + if (!isdigit((unsigned char)b[1])) + return false; + for (a++; isdigit((unsigned char)*a); a++) + ; + for (b++; isdigit((unsigned char)*b); b++) + ; + } else { + a++; + b++; + } } - /* Return the widest level that yields a unique (1:1) match */ - if (name_orig == 1 && name_patched == 1) - match = name_last; - else if (scope_orig == 1 && scope_patched == 1) - match = scope_last; - else if (file_orig == 1 && file_patched == 1) - match = file_last; - else if (csum_orig == 1 && csum_patched == 1) - match = csum_last; - - if (!match) - return NULL; - - if (name_orig != 1 || name_patched != 1) - dbg_correlate("find_twin(): %s%s -> %s%s", - sym1->name, is_func_sym(sym1) ? "()" : "", - match->name, is_func_sym(match) ? "()" : ""); - - return match; + /* A trailing counter may be absent on one side. */ + return (!*a && is_numeric_suffix(b)) || (!*b && is_numeric_suffix(a)); } -struct llvm_suffix_pair { - struct hlist_node hash; - const char *orig; - const char *patched; -}; - -static DECLARE_HASHTABLE(suffix_map, 7); - -/* - * Build a mapping of known orig-to-patched LLVM suffixes based on - * already-correlated symbol pairs. All promoted symbols from the same TU - * share the same .llvm.<hash> suffix, so one correlated pair seeds the map - * for the entire TU. - */ -static int update_suffix_map(struct elf *elf) +/* Keep clone tags such as .cold, .isra and .part distinct. */ +static bool compatible_symbol_name(const char *a, const char *b) { - struct llvm_suffix_pair *entry; - struct symbol *sym; - - for_each_sym(elf, sym) { - const char *s1, *s2; - bool found; - - if (!sym->twin) - continue; + if (same_llvm_name(a, b)) + return true; + if (strchr(a, '@') || strchr(b, '@') || strstr(a, ".str1.")) + return !strcmp(a, b); + return same_compiler_name(a, b); +} - s1 = llvm_suffix(sym->name); - s2 = llvm_suffix(sym->twin->name); +static bool is_twin_candidate(struct symbol *sym1, struct symbol *sym2) +{ + if (sym1->type != sym2->type || !same_identity(&sym1->identity, &sym2->identity)) + return false; + /* Don't match zero-length labels with sized symbols. */ + if (!is_undef_sym(sym1) && !is_undef_sym(sym2) && + (!sym1->len != !sym2->len)) + return false; + return compatible_symbol_name(sym1->name, sym2->name); +} - if (!s1 || !s2) - continue; +static struct symbol *find_twin(struct elf *elf, struct symbol *sym1) +{ + struct symbol *sym2, *match = NULL; + unsigned int candidates = 0; + u32 key = identity_hash(&sym1->identity); - found = false; - hash_for_each_possible(suffix_map, entry, hash, str_hash(s1)) { - if (!strcmp(entry->orig, s1)) { - found = true; - break; - } - } - if (found) + elf_hash_for_each_possible(elf, symbol_identity, sym2, identity_hash, key) { + if (sym2->twin || !is_twin_candidate(sym1, sym2)) continue; - - entry = calloc(1, sizeof(*entry)); - if (!entry) { - ERROR_GLIBC("calloc"); - return -1; - } - - entry->orig = s1; - entry->patched = s2; - hash_add(suffix_map, &entry->hash, str_hash(s1)); + match = sym2; + candidates++; } - return 0; + return candidates == 1 ? match : NULL; } -/* - * Match by translating the symbol's .llvm.<hash> suffix through the suffix - * map to find the corresponding hash suffix for the patched object. - * - * Example: In the original kernel, TU drivers/base/core.c contains - * foo.llvm.12345 and bar.llvm.12345 (same TU, same hash). After patching, - * they become foo.llvm.67890 and bar.llvm.67890. If foo was already - * correlated by find_twin() (e.g., unique by name), the suffix map records - * .llvm.12345 -> .llvm.67890. When processing bar.llvm.12345, this - * function looks up .llvm.12345, gets .llvm.67890, constructs the name - * bar.llvm.67890, and finds the match. - */ -static struct symbol *find_twin_suffixed(struct elf *elf, struct symbol *sym1) +/* Checksums can disambiguate only unchanged symbols. */ +static struct symbol *find_twin_checksum(struct elf *elf, struct symbol *sym1) { - const char *suffix, *patched_suffix = NULL; struct symbol *sym2, *match = NULL; - char name[SYM_NAME_LEN]; - struct llvm_suffix_pair *entry; - int count = 0; + unsigned int candidates = 0; + u32 key = identity_hash(&sym1->identity); - suffix = llvm_suffix(sym1->name); - if (!suffix) + if (!sym1->csum.checksum) return NULL; - hash_for_each_possible(suffix_map, entry, hash, str_hash(suffix)) { - if (!strcmp(entry->orig, suffix)) { - patched_suffix = entry->patched; - break; - } - } - if (!patched_suffix) - return NULL; - - if (snprintf_check(name, SYM_NAME_LEN, "%s%s", - sym1->demangled_name, patched_suffix)) - return NULL; - - for_each_sym_by_name(elf, name, sym2) { - if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate) + elf_hash_for_each_possible(elf, symbol_identity, sym2, identity_hash, key) { + if (!is_twin_candidate(sym1, sym2)) + continue; + if (sym2->twin || sym1->len != sym2->len || + sym1->csum.checksum != sym2->csum.checksum) continue; - count++; match = sym2; + candidates++; } - if (count != 1) - return NULL; - - dbg_correlate("find_suffixed_twin(): %s%s -> %s%s", - sym1->name, is_func_sym(sym1) ? "()" : "", - match->name, is_func_sym(match) ? "()" : ""); - - return match; + return candidates == 1 ? match : NULL; } -/* - * Last-resort positional matching. - * - * Finds a symbol with the same position in the symbol table among - * same-demangled-name candidates, similar to livepatch sympos. Note that - * LLVM-promoted symbols are globals, which come after locals in the symbol - * table, so we have to be careful not to compare different scopes. - * - * Example: arch/x86/events/intel/core.c defines many __quirk variables via - * X86_MATCH_*() macros. In the symbol table they appear as __quirk.90, - * __quirk.97, __quirk.101, etc., all with demangled name __quirk, same - * scope, and same FILE symbol. No deterministic filter can distinguish - * them, so they're matched by position: the 1st __quirk in orig matches the - * 1st in patched, the 2nd matches the 2nd, etc. - * - * This is less deterministic than the other strategies, so it's done last. - */ -static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1) +static void correlate_pass(struct elfs *e, + struct symbol *(*find)(struct elf *, struct symbol *)) { - unsigned int idx_orig = 0, idx_patched = 0; - unsigned int sym1_pos = 0; - struct symbol *sym2, *match = NULL; - - for_each_sym_by_demangled_name(e->orig, sym1->demangled_name, sym2) { - if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate || - !maybe_same_file(sym1, sym2)) - continue; - if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) || - is_llvm_sym(sym1) != is_llvm_sym(sym2)) - continue; - if (sym1 == sym2) - sym1_pos = idx_orig; - idx_orig++; - } + struct symbol *sym, *match; - for_each_sym_by_demangled_name(e->patched, sym1->demangled_name, sym2) { - if (sym2->twin || sym1->type != sym2->type || sym2->dont_correlate || - !maybe_same_file(sym1, sym2)) + for_each_sym(e->orig, sym) { + if (sym->twin || sym->dont_correlate) continue; - if (is_tu_local_sym(sym1) != is_tu_local_sym(sym2) || - is_llvm_sym(sym1) != is_llvm_sym(sym2)) + match = find(e->patched, sym); + /* Two originals can share the same sole patched candidate. */ + if (!match || find(e->orig, match) != sym) continue; - if (idx_patched == sym1_pos) - match = sym2; - idx_patched++; + sym->twin = match; + match->twin = sym; + dbg_correlate("%s(): %s -> %s", __func__, sym->name, match->name); } - - if (idx_orig != idx_patched) - return NULL; - - dbg_correlate("find_twin_positional(): %s%s -> %s%s", - sym1->name, is_func_sym(sym1) ? "()" : "", - match->name, is_func_sym(match) ? "()" : ""); - - return match; } /* @@ -819,96 +598,55 @@ static struct symbol *find_twin_positional(struct elfs *e, struct symbol *sym1) */ static int correlate_symbols(struct elfs *e) { - struct symbol *file1_sym, *file2_sym; - struct symbol *sym1, *sym2; - bool progress; - - for_each_sym(e->orig, sym1) - sym1->dont_correlate = dont_correlate(sym1); - for_each_sym(e->patched, sym2) - sym2->dont_correlate = dont_correlate(sym2); - - /* Correlate FILE symbols */ - file1_sym = first_file_symbol(e->orig); - file2_sym = first_file_symbol(e->patched); + struct symbol *sym; - for (; ; file1_sym = next_file_symbol(e->orig, file1_sym), - file2_sym = next_file_symbol(e->patched, file2_sym)) { + for_each_sym(e->orig, sym) + sym->dont_correlate = dont_correlate(sym); + for_each_sym(e->patched, sym) + sym->dont_correlate = dont_correlate(sym); - if (!file1_sym && file2_sym) { - ERROR("FILE symbol mismatch: NULL != %s", file2_sym->name); - return -1; - } + if (prepare_identities(e->orig) || prepare_identities(e->patched)) + return -1; - if (file1_sym && !file2_sym) { - ERROR("FILE symbol mismatch: %s != NULL", file1_sym->name); - return -1; - } + correlate_pass(e, find_twin); + correlate_pass(e, find_twin_checksum); - if (!file1_sym) - break; + for_each_sym(e->orig, sym) + if (!sym->twin && !sym->dont_correlate) + WARN("no correlation: %s", sym->name); - if (strcmp(file1_sym->name, file2_sym->name)) { - ERROR("FILE symbol mismatch: %s != %s", file1_sym->name, file2_sym->name); - return -1; - } + return 0; +} - file1_sym->twin = file2_sym; - file2_sym->twin = file1_sym; - } +static int check_linkage_changes(struct elfs *e) +{ + struct symbol *orig_sym, *patched_sym; + for_each_sym(e->orig, orig_sym) { + if (orig_sym->twin || orig_sym->dont_correlate || + is_undef_sym(orig_sym) || !orig_sym->len) + continue; - /* - * Correlate in two phases: loop deterministic levels until no more - * progress, then use positional fallback for the rest. This prevents - * the nondeterministic positional matching from stealing symbols that - * have deterministic matches. - */ - hash_init(suffix_map); - do { - progress = false; - for_each_sym(e->orig, sym1) { - if (sym1->twin || sym1->dont_correlate) + /* + * Identities distinguish linkage changes, but later name-based + * resolution can bind references to the wrong state. Reject them. + */ + for_each_sym_by_demangled_name(e->patched, orig_sym->identity.demangled_name, + patched_sym) { + if (patched_sym->twin || patched_sym->dont_correlate || + is_undef_sym(patched_sym) || !patched_sym->len) continue; - sym2 = find_twin(e, sym1); - if (!sym2) + if (orig_sym->type != patched_sym->type || + strcmp(orig_sym->identity.scope, patched_sym->identity.scope) || + orig_sym->identity.external == patched_sym->identity.external) continue; - sym1->twin = sym2; - sym2->twin = sym1; - progress = true; - } - - if (update_suffix_map(e->orig)) + ERROR("unsupported linkage change (%s, %s): original %s symtab[%u] (external=%d), patched %s symtab[%u] (external=%d)", + orig_sym->identity.scope, orig_sym->identity.demangled_name, + orig_sym->name, orig_sym->idx, orig_sym->identity.external, + patched_sym->name, patched_sym->idx, patched_sym->identity.external); return -1; - - for_each_sym(e->orig, sym1) { - if (sym1->twin || sym1->dont_correlate) - continue; - sym2 = find_twin_suffixed(e->patched, sym1); - if (!sym2) - continue; - sym1->twin = sym2; - sym2->twin = sym1; - progress = true; } - } while (progress); - - for_each_sym(e->orig, sym1) { - if (sym1->twin || sym1->dont_correlate) - continue; - sym2 = find_twin_positional(e, sym1); - if (!sym2) - continue; - sym1->twin = sym2; - sym2->twin = sym1; - } - - for_each_sym(e->orig, sym1) { - if (sym1->twin || sym1->dont_correlate) - continue; - WARN("no correlation: %s", sym1->name); } - return 0; } @@ -1079,7 +817,8 @@ static int mark_changed_functions(struct elfs *e) /* Find added functions and print them */ for_each_sym(e->patched, patched_sym) { - if (!is_func_sym(patched_sym) || patched_sym->dont_correlate) + if (!is_func_sym(patched_sym) || patched_sym->dont_correlate || + is_undef_sym(patched_sym)) continue; if (!patched_sym->twin) { @@ -1206,6 +945,9 @@ static bool klp_reloc_needed(struct reloc *patched_reloc) * patch. The symbol could be defined in this .o or in another * .o in the patch module. * + * FIXME: Failed correlation can duplicate existing data, breaking + * shared state or pointer identity. + * * This check needs to be *after* the export check due to the * possibility of the patch adding a new UNDEF reference to an * exported symbol. @@ -2254,6 +1996,9 @@ int cmd_klp_diff(int argc, const char **argv) if (correlate_symbols(&e)) return -1; + if (check_linkage_changes(&e)) + return -1; + ret = mark_changed_functions(&e); if (ret < 0) return -1; diff --git a/tools/objtool/klp-dwarf.c b/tools/objtool/klp-dwarf.c new file mode 100644 index 000000000000..eff054f2864b --- /dev/null +++ b/tools/objtool/klp-dwarf.c @@ -0,0 +1,778 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> + * + * Compute DWARF identity information for ELF symbols by walking the DIE + * tree and following abstract_origin/specification references. + * + * - source: physical translation unit (CU) of the definition/origin + * - scope: slash-delimited chain of named enclosing scopes + */ +#define _GNU_SOURCE +#include <ctype.h> +#include <dwarf.h> +#include <elfutils/libdwfl.h> +#include <stdlib.h> +#include <string.h> + +#include <linux/list.h> + +#include <objtool/elf.h> +#include <objtool/klp-dwarf.h> +#include <objtool/warn.h> + +#define IDENTITY_HASH_BITS 18 +#define IDENTITY_MAX_DEPTH 256 + +struct identity_cu { + struct list_head list; + char *path; +}; + +struct identity_die { + struct hlist_node hash; + Dwarf_Die die; + struct identity_cu *cu; + struct identity_die *parent, *definition; + void *abstract_origin, *specification; + const char *name; + char *scope; + bool declaration; +}; + +struct address_identity { + struct elf_hash_node hash; + unsigned int section; + unsigned long offset; + char *source, *scope, *name; + bool external; +}; + +struct identity_reader { + struct elf *elf; + Dwfl_Module *module; + Dwarf *dwarf; + Dwarf_Addr bias; + struct list_head cus; + DECLARE_HASHTABLE(dies, IDENTITY_HASH_BITS); +}; + +static int die_error(Dwarf_Die *die, const char *message) +{ + ERROR("DWARF identity: DIE %#llx: %s", (unsigned long long)dwarf_dieoffset(die), message); + return -1; +} + +static bool is_unit(int tag) +{ + return tag == DW_TAG_compile_unit || tag == DW_TAG_partial_unit || tag == DW_TAG_type_unit; +} + +static bool is_symbol(int tag) +{ + return tag == DW_TAG_subprogram || tag == DW_TAG_variable || tag == DW_TAG_constant; +} + +static bool is_scope(Dwarf_Die *die) +{ + int tag = dwarf_tag(die); + + return is_unit(tag) || is_symbol(tag) || tag == DW_TAG_lexical_block || + tag == DW_TAG_namespace || tag == DW_TAG_class_type || + tag == DW_TAG_structure_type || tag == DW_TAG_union_type || + tag == DW_TAG_module || + (tag == DW_TAG_member && dwarf_hasattr(die, DW_AT_declaration)); +} + +static struct identity_die *find_die(struct identity_reader *reader, void *address) +{ + struct identity_die *entry; + + hash_for_each_possible(reader->dies, entry, hash, (unsigned long)address) + if (entry->die.addr == address) + return entry; + return NULL; +} + +/* Normalize source paths for identity comparison without filesystem access. */ +static void normalize_source(char *path) +{ + char *root = path + (*path == '/'); + char *read = root, *write = root, *part, *previous; + size_t len; + + while ((part = strsep(&read, "/"))) { + if (!*part || !strcmp(part, ".")) + continue; + if (!strcmp(part, "..")) { + previous = write; + while (previous > root && previous[-1] != '/') + previous--; + if (previous < write && + !(write - previous == 2 && !memcmp(previous, "..", 2))) { + write = previous > root ? previous - 1 : root; + continue; + } + if (root != path) + continue; + } + len = strlen(part); + if (write > root) + *write++ = '/'; + memmove(write, part, len); + write += len; + } + *write = '\0'; +} + +static int read_unit_source(struct identity_cu *cu, Dwarf_Die *die) +{ + Dwarf_Attribute attr; + const char *name = NULL, *dir = NULL; + char *path; + + if (dwarf_tag(die) != DW_TAG_compile_unit) + return 0; + if (dwarf_attr(die, DW_AT_name, &attr)) { + name = dwarf_formstring(&attr); + if (!name) + return die_error(die, "invalid compilation unit name"); + } + if (dwarf_attr(die, DW_AT_comp_dir, &attr)) { + dir = dwarf_formstring(&attr); + if (!dir) + return die_error(die, "invalid compilation directory"); + } + if (!name || !*name || (*name != '/' && !dir)) + return 0; + if (*name == '/') + path = strdup(name); + else if (asprintf(&path, "%s/%s", dir, name) < 0) + return -1; + if (!path) + return -1; + normalize_source(path); + cu->path = path; + return 0; +} + +static int read_reference(struct identity_reader *reader, Dwarf_Die *die, + unsigned int attribute, void **address) +{ + Dwarf_Attribute attr; + Dwarf_Die reference; + + if (!dwarf_attr(die, attribute, &attr)) + return 0; + if (!dwarf_formref_die(&attr, &reference)) + return die_error(die, "invalid declaration reference"); + if (dwarf_cu_getdwarf(reference.cu) != reader->dwarf) + return die_error(die, "references to supplementary DWARF files are unsupported"); + *address = reference.addr; + return 0; +} + +static int read_attributes(struct identity_reader *reader, struct identity_die *entry) +{ + Dwarf_Die *die = &entry->die; + Dwarf_Attribute attr; + int tag = dwarf_tag(die); + + if (dwarf_attr(die, DW_AT_name, &attr)) { + entry->name = dwarf_formstring(&attr); + if (!entry->name || (!is_unit(tag) && strchr(entry->name, '/'))) + return die_error(die, "invalid or unsupported scope name"); + } + if (!is_symbol(tag) && tag != DW_TAG_member) + return 0; + if (dwarf_attr(die, DW_AT_declaration, &attr)) { + if (dwarf_formflag(&attr, &entry->declaration)) + return die_error(die, "invalid declaration flag"); + if (entry->declaration && + (dwarf_hasattr(die, DW_AT_location) || dwarf_hasattr(die, DW_AT_low_pc))) + return die_error(die, "declaration has a concrete address"); + } + if (tag == DW_TAG_member) + return 0; + if (read_reference(reader, die, DW_AT_abstract_origin, &entry->abstract_origin) || + read_reference(reader, die, DW_AT_specification, &entry->specification)) + return -1; + if (entry->abstract_origin && entry->specification) + return die_error(die, "both origin and specification are unsupported"); + return 0; +} + +static int scan_attribute(Dwarf_Attribute *attr, void *unused) +{ + return DWARF_CB_OK; +} + +static int index_dies(struct identity_reader *reader, struct identity_cu *cu, + Dwarf_Die *die, struct identity_die *parent, unsigned int depth) +{ + struct identity_die *entry; + Dwarf_Die child; + int ret; + + if (!dwarf_tag(die) || dwarf_getattrs(die, scan_attribute, NULL, 0) < 0) + return die_error(die, "invalid DIE attributes"); + if (depth > IDENTITY_MAX_DEPTH) + return die_error(die, "DIE nesting exceeds the supported depth"); + if (is_scope(die)) { + entry = calloc(1, sizeof(*entry)); + if (!entry) + return -1; + entry->die = *die; + entry->cu = cu; + entry->parent = parent; + hash_add(reader->dies, &entry->hash, (unsigned long)die->addr); + if (read_attributes(reader, entry)) + return -1; + if (dwarf_tag(die) != DW_TAG_member) + parent = entry; + } + ret = dwarf_child(die, &child); + if (ret) + return ret < 0 ? die_error(die, "cannot read child") : 0; + do { + if (index_dies(reader, cu, &child, parent, depth + 1)) + return -1; + ret = dwarf_siblingof(&child, &child); + } while (!ret); + return ret < 0 ? die_error(die, "cannot read sibling") : 0; +} + +static struct identity_cu *read_cu(struct identity_reader *reader, Dwarf_Die *die) +{ + struct identity_die *entry; + struct identity_cu *cu; + Dwarf_Die root; + + if (!dwarf_diecu(die, &root, NULL, NULL) || !is_unit(dwarf_tag(&root))) + return NULL; + entry = find_die(reader, root.addr); + if (entry) + return entry->cu; + cu = calloc(1, sizeof(*cu)); + if (!cu) + return NULL; + list_add_tail(&cu->list, &reader->cus); + if (read_unit_source(cu, &root) || index_dies(reader, cu, &root, NULL, 0)) + return NULL; + return cu; +} + +/* Follow abstract_origin/specification chains to find the name and defining DIE. */ +static int resolve_definition(struct identity_reader *reader, struct identity_die *entry) +{ + struct identity_die *cur = entry, *definition = NULL; + const char *name = NULL; + unsigned int depth = 0; + void *reference; + + while (cur) { + if (++depth > IDENTITY_MAX_DEPTH) + return die_error(&entry->die, + "declaration lookup exceeds the supported depth"); + if (cur->name) { + if (name && strcmp(name, cur->name)) + return die_error(&entry->die, + "conflicting names in declaration chain"); + name = cur->name; + } + if (!cur->declaration && !cur->abstract_origin) { + if (definition) + return die_error(&entry->die, + "multiple definitions in declaration chain"); + definition = cur; + } + reference = cur->abstract_origin ?: cur->specification; + if (!reference) + break; + cur = find_die(reader, reference); + if (!cur) + return die_error(&entry->die, "unavailable declaration reference"); + } + entry->definition = definition; + entry->name = name; + return 0; +} + +/* + * Build the slash-delimited scope string from enclosing named parents. + * + * FIXME: Unnamed lexical scopes are not distinguished. Reject ambiguous + * local statics rather than relying on compiler-generated suffixes. + */ +static int resolve_parent_scope(struct identity_die *entry, unsigned int depth) +{ + struct identity_die *parent = entry->parent; + int tag; + + if (entry->scope) + return 0; + if (depth > IDENTITY_MAX_DEPTH) + return die_error(&entry->die, "scope lookup exceeds the supported depth"); + if (!parent || is_unit(dwarf_tag(&parent->die))) { + entry->scope = strdup("/"); + } else { + if (resolve_parent_scope(parent, depth + 1)) + return -1; + tag = dwarf_tag(&parent->die); + if (parent->name && tag != DW_TAG_lexical_block) { + if (asprintf(&entry->scope, "%s%s/", parent->scope, parent->name) < 0) + return -1; + } else { + entry->scope = strdup(parent->scope); + } + } + return entry->scope ? 0 : -1; +} + +/* If completed by a DW_AT_specification, use the declaration's enclosing scope. */ +static int resolve_definition_scope(struct identity_reader *reader, struct identity_die *entry, + const char **scope) +{ + struct identity_die *start = entry->definition; + + if (start->specification) { + start = find_die(reader, start->specification); + if (!start) + return die_error(&entry->die, "unavailable declaration scope"); + } + if (resolve_parent_scope(start, 0)) + return -1; + *scope = start->scope; + return 0; +} + +static const char *resolve_definition_source(struct identity_die *entry) +{ + return entry->definition->cu->path; +} + +/* Translate libdwfl's relocated address back to objtool's section/offset pair. */ +static int address_section(struct identity_reader *reader, Dwarf_Addr *address, + unsigned int *section) +{ + struct elf *elf = reader->elf; + struct section *sec; + Dwarf_Addr absolute = *address, bias; + GElf_Shdr sh; + Elf *file; + GElf_Word index; + size_t nr, i; + int base; + + base = dwfl_module_relocate_address(reader->module, address); + if (base < 0) + return 1; + if (!dwfl_module_relocation_info(reader->module, base, &index)) + return -1; + if (sec_size(&elf->section_data[index])) { + *section = index; + return 0; + } + /* Empty sections can share the following nonempty section's synthetic address. */ + file = dwfl_module_getelf(reader->module, &bias); + if (!file || elf_getshdrnum(file, &nr)) + return -1; + for (i = 1; i <= nr; i++) { + sec = &elf->section_data[(index + i) % nr]; + if (!(sec->sh.sh_flags & SHF_ALLOC) || !sec_size(sec)) + continue; + if (!gelf_getshdr(elf_getscn(file, sec->idx), &sh)) + return -1; + if (absolute >= sh.sh_addr + bias && absolute - sh.sh_addr - bias < sh.sh_size) { + *section = sec->idx; + *address = absolute - sh.sh_addr - bias; + return 0; + } + } + return 1; +} + +static int add_address(struct identity_reader *reader, Dwarf_Addr address, + const char *source, const char *scope, const char *name, bool external) +{ + struct elf *elf = reader->elf; + struct address_identity *entry; + Dwarf_Addr offset = address; + unsigned int section; + int ret = address_section(reader, &offset, §ion); + + if (ret) + return ret < 0 ? -1 : 0; + entry = calloc(1, sizeof(*entry)); + if (!entry) + return -1; + entry->section = section; + entry->offset = offset; + entry->external = external; + entry->scope = scope ? strdup(scope) : NULL; + entry->source = source ? strdup(source) : NULL; + entry->name = name ? strdup(name) : NULL; + if ((scope && !entry->scope) || (source && !entry->source) || + (name && !entry->name)) + return -1; + elf_hash_add(dwarf_address, &entry->hash, offset ^ section); + return 0; +} + +/* + * The empty features[] in virtio_input.c can share an address with the + * nonempty features[] in trans_virtio.c. Exclude the empty object's DIE + * from address lookup. libdw rejects GCC's upper_bound=-1 encoding, so + * check array bounds too. + */ +static bool is_empty_type(Dwarf_Die *type) +{ + Dwarf_Die array, range; + Dwarf_Attribute attr; + Dwarf_Word size; + Dwarf_Sword lower, upper; + + if (!dwarf_aggregate_size(type, &size)) + return !size; + if (dwarf_peel_type(type, &array) || dwarf_tag(&array) != DW_TAG_array_type || + dwarf_child(&array, &range)) + return false; + do { + if (dwarf_tag(&range) != DW_TAG_subrange_type) + continue; + if (dwarf_attr(&range, DW_AT_count, &attr) && + !dwarf_formudata(&attr, &size) && !size) + return true; + lower = 0; + if (dwarf_attr(&range, DW_AT_lower_bound, &attr) && + dwarf_formsdata(&attr, &lower)) + continue; + if (dwarf_attr(&range, DW_AT_upper_bound, &attr) && + !dwarf_formsdata(&attr, &upper) && upper < lower) + return true; + } while (!dwarf_siblingof(&range, &range)); + return false; +} + +static int collect_addresses(struct identity_reader *reader, struct identity_die *entry) +{ + Dwarf_Die *die = &entry->die, type; + Dwarf_Attribute attr, address_attr; + Dwarf_Addr base, start, end, address; + Dwarf_Op *ops; + const char *source, *scope; + size_t count; + ptrdiff_t off = 0; + bool external = false; + int tag = dwarf_tag(die); + + if (!entry->definition || (tag != DW_TAG_subprogram && tag != DW_TAG_variable)) + return 0; + if (dwarf_attr_integrate(die, DW_AT_external, &attr) && dwarf_formflag(&attr, &external)) + return die_error(die, "invalid external flag"); + /* + * External identities do not need source. DWARF relocations for overridden + * weak definitions can bind to the selected strong definition, giving the + * same address conflicting source CUs (GDB PR 34741): + * https://sourceware.org/bugzilla/show_bug.cgi?id=34741 + */ + source = external ? NULL : resolve_definition_source(entry); + if (tag == DW_TAG_subprogram) { + if (!dwarf_hasattr(die, DW_AT_low_pc) && !dwarf_hasattr(die, DW_AT_ranges)) + return 0; + if (resolve_definition_scope(reader, entry, &scope)) + return -1; + while ((off = dwarf_ranges(die, off, &base, &start, &end)) > 0) + if (start != end && add_address(reader, start + reader->bias, + source, scope, entry->name, external)) + return -1; + return off < 0 ? die_error(die, "invalid code ranges") : 0; + } + if (!dwarf_attr(die, DW_AT_location, &attr)) + return 0; + /* Location lists may vary with the PC; index only fixed storage. */ + if (dwarf_whatform(&attr) == DW_FORM_sec_offset || + dwarf_whatform(&attr) == DW_FORM_loclistx) + return 0; + if (dwarf_getlocation(&attr, &ops, &count)) + return die_error(die, "invalid location expression"); + if (!count) + return 0; + if (dwarf_attr_integrate(die, DW_AT_type, &address_attr)) { + if (!dwarf_formref_die(&address_attr, &type)) + return die_error(die, "invalid variable type reference"); + if (is_empty_type(&type)) + return 0; + } + if (resolve_definition_scope(reader, entry, &scope)) + return -1; + if (ops[0].atom == DW_OP_addr) { + address = ops[0].number; + } else if (ops[0].atom == DW_OP_addrx || ops[0].atom == DW_OP_GNU_addr_index) { + if (dwarf_getlocation_attr(&attr, ops, &address_attr) || + dwarf_formaddr(&address_attr, &address)) + return die_error(die, "invalid location address"); + } else { + return 0; + } + return add_address(reader, address + reader->bias, source, + count == 1 ? scope : NULL, entry->name, external); +} + +/* + * libdwfl permits partial relocation, leaving unapplied entries in place. + * Check only sections read for identities; unused indexes may retain relocations. + */ +static int check_relocations(Dwarf *dwarf) +{ + static const char * const sections[] = { + ".debug_info", ".debug_types", ".debug_abbrev", + ".debug_str", ".debug_line_str", ".debug_str_offsets", + ".debug_addr", ".debug_ranges", ".debug_rnglists", + }; + Elf *elf = dwarf_getelf(dwarf); + Elf_Scn *scn = NULL; + GElf_Shdr sh, target; + const char *name; + size_t strings, i; + + if (!elf || elf_getshdrstrndx(elf, &strings)) + return -1; + while ((scn = elf_nextscn(elf, scn))) { + if (!gelf_getshdr(scn, &sh)) + return -1; + if (!sh.sh_size || (sh.sh_type != SHT_REL && sh.sh_type != SHT_RELA)) + continue; + if (!gelf_getshdr(elf_getscn(elf, sh.sh_info), &target)) + return -1; + name = elf_strptr(elf, strings, target.sh_name); + if (!name) + return -1; + for (i = 0; i < ARRAY_SIZE(sections); i++) + if (!strcmp(name, sections[i])) { + ERROR("DWARF identity: unapplied relocations for %s", name); + return -1; + } + } + return 0; +} + +static int build_identities(struct identity_reader *reader) +{ + static const Dwfl_Callbacks callbacks = { .section_address = dwfl_offline_section_address }; + Dwfl *dwfl = dwfl_begin(&callbacks); + Dwarf_CU *cu = NULL, *next; + Dwarf_Die die; + Dwarf *dwarf; + struct identity_die *entry; + unsigned int bucket; + uint8_t unit_type; + int ret = -1; + + if (!dwfl) + return -1; + reader->module = dwfl_report_offline(dwfl, reader->elf->name, reader->elf->name, -1); + if (!reader->module || dwfl_report_end(dwfl, NULL, NULL)) + goto out; + dwarf = dwfl_module_getdwarf(reader->module, &reader->bias); + if (!dwarf || check_relocations(dwarf)) + goto out; + reader->dwarf = dwarf; + while (!(ret = dwarf_get_units(dwarf, cu, &next, NULL, &unit_type, &die, NULL))) { + if (unit_type == DW_UT_skeleton || + unit_type == DW_UT_split_compile || + unit_type == DW_UT_split_type) { + ret = die_error(&die, "split DWARF is not supported yet"); + goto out; + } + if (!is_unit(dwarf_tag(&die))) { + ret = die_error(&die, "invalid unit root"); + goto out; + } + if (!read_cu(reader, &die)) { + ret = -1; + goto out; + } + cu = next; + } + if (ret < 0) + goto out; + ret = -1; + hash_for_each(reader->dies, bucket, entry, hash) + if (is_symbol(dwarf_tag(&entry->die)) && resolve_definition(reader, entry)) + goto out; + hash_for_each(reader->dies, bucket, entry, hash) + if (collect_addresses(reader, entry)) + goto out; + ret = 0; +out: + dwfl_end(dwfl); + return ret; +} + +int init_identity_cache(struct elf *elf) +{ + struct identity_reader *reader; + struct identity_die *die; + struct identity_cu *cu, *next; + struct hlist_node *tmp; + const char *saved_objname = objname; + unsigned int bucket; + int ret; + + if (elf->dwarf_address_hash) + return 0; + + if (elf->ehdr.e_type != ET_REL) { + ERROR("%s: DWARF identity requires an ET_REL object", elf->name); + return -1; + } + if (!find_section_by_name(elf, ".debug_info")) { + ERROR("%s: missing DWARF debug information; build with -g", elf->name); + return -1; + } + reader = calloc(1, sizeof(*reader)); + if (!reader) + return -1; + if (!elf_alloc_hash(dwarf_address, 1UL << IDENTITY_HASH_BITS)) + return -1; + reader->elf = elf; + INIT_LIST_HEAD(&reader->cus); + objname = elf->name; + ret = build_identities(reader); + hash_for_each_safe(reader->dies, bucket, tmp, die, hash) { + free(die->scope); + free(die); + } + list_for_each_entry_safe(cu, next, &reader->cus, list) { + free(cu->path); + free(cu); + } + free(reader); + if (ret) + ERROR("DWARF identity: input cannot be parsed"); + objname = saved_objname; + return ret; +} + +/* Match DW_AT_name verbatim; accept compiler suffixes only on the ELF name. */ +static bool matches_name(const char *name, const char *identifier) +{ + size_t len = strlen(identifier); + + if (strncmp(name, identifier, len)) + return false; + name += len; + while (*name) { + if (strstarts(name, ".llvm.")) + name += 5; + if (*name++ != '.' || !isdigit((unsigned char)*name)) + return false; + while (isdigit((unsigned char)*name)) + name++; + } + return true; +} + +static bool matches_dwarf_name(struct address_identity *entry, const char *name) +{ + const char *scope; + + if (!entry->name || !entry->scope) + return false; + if (matches_name(name, entry->name)) + return true; + /* + * Clang names function-local statics as func.var[.suffix]. Constant merging + * can leave one such ELF symbol for several variable DIEs, as with oldpiix's + * timings tables. Match the encoded function scope as well as the variable. + */ + for (scope = entry->scope + 1; *scope; scope++, name++) + if (*name != (*scope == '/' ? '.' : *scope)) + return false; + return matches_name(name, entry->name); +} + +int get_identity_for_symbol(struct elf *elf, struct symbol *sym) +{ + struct identity *identity = &sym->identity; + struct section *sec = sym->sec; + struct address_identity *entry; + unsigned int nr = 0, nr_named = 0; + bool filter_name = false, unavailable = false, source_unavailable = false; + const char *error, *saved_objname = objname; + + identity->source = ""; + identity->scope = NULL; + identity->external = false; + elf_hash_for_each_possible(elf, dwarf_address, entry, hash, sym->offset ^ sec->idx) { + if (entry->section != sec->idx || entry->offset != sym->offset) + continue; + nr++; + if (is_rodata_sec(sec) && matches_dwarf_name(entry, sym->name)) + nr_named++; + } + if (!nr) + return 1; + + /* Only folded read-only storage needs name-based disambiguation. */ + if (nr > 1 && is_rodata_sec(sec)) { + filter_name = true; + if (!nr_named) + return 1; + /* + * ICF/constant merging can leave several definitions after name + * filtering. FIXME: Folded objects can remain uncorrelated when + * their checksums change. + */ + if (nr_named > 1 && strstarts(sec->name, ".rodata")) + return 1; + } + + objname = elf->name; + elf_hash_for_each_possible(elf, dwarf_address, entry, hash, sym->offset ^ sec->idx) { + if (entry->section != sec->idx || entry->offset != sym->offset) + continue; + if (filter_name && !matches_dwarf_name(entry, sym->name)) + continue; + /* Scope cannot be resolved from the first address of a compound expression. */ + if (!entry->scope) { + unavailable = true; + /* Keep scanning: later entries may conflict. */ + continue; + } + error = "conflicting declarations"; + if (identity->scope && strcmp(identity->scope, entry->scope)) + goto failed; + error = "conflicting external declarations"; + if (identity->scope && identity->external != entry->external) + goto failed; + /* Source is omitted for external symbols or unavailable from the CU. */ + if (!entry->source) { + source_unavailable = true; + } else { + error = "conflicting source CUs"; + if (*identity->source && strcmp(identity->source, entry->source)) + goto failed; + identity->source = entry->source; + } + identity->scope = entry->scope; + identity->external = entry->external; + } + if (unavailable) { + identity->source = ""; + identity->scope = NULL; + identity->external = false; + } + if (source_unavailable) + identity->source = ""; + objname = saved_objname; + return identity->scope ? 0 : 1; +failed: + ERROR("DWARF identity: %s at %s+%#lx", error, sec->name, sym->offset); + identity->source = ""; + identity->scope = NULL; + identity->external = false; + objname = saved_objname; + return -1; +} diff --git a/tools/objtool/klp-identity.c b/tools/objtool/klp-identity.c new file mode 100644 index 000000000000..627eda694389 --- /dev/null +++ b/tools/objtool/klp-identity.c @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> + */ +#define _GNU_SOURCE +#include <string.h> + +#include <linux/jhash.h> + +#include <objtool/elf.h> +#include <objtool/klp-dwarf.h> +#include <objtool/klp-identity.h> +#include <objtool/warn.h> + +static bool is_dwarf_identity_candidate(struct symbol *sym) +{ + return !sym->dont_correlate && sym->len && !is_undef_sym(sym) && + (is_func_sym(sym) || is_object_sym(sym)); +} + +static bool known_file_name_mismatch(const char *file, const char *source) +{ + const char *file_ext, *source_ext; + + /* Full LTO combines TUs under one synthetic FILE name. */ + if (!strcmp(file, "ld-temp.o")) + return true; + + /* Startup objects prefix FILE names without renaming their DWARF CUs. */ + if (strstarts(file, "__pi_") && !strcmp(file + 5, source)) + return true; + + /* + * GNU AS misidentifies retpoline.S's CU as GEN-for-each-reg.h: + * https://sourceware.org/pipermail/binutils/2026-October/152209.html + */ + if (!strcmp(file, "retpoline.o") && !strcmp(source, "GEN-for-each-reg.h")) + return true; + + /* Ignore extension differences, e.g. foo.o versus foo.S. */ + file_ext = strrchr(file, '.'); + source_ext = strrchr(source, '.'); + if (!file_ext || !source_ext) + return false; + return file_ext - file == source_ext - source && + !strncmp(file, source, file_ext - file); +} + +/* Fill STT_FILE paths from DWARF, never the reverse. */ +static int complete_file_source(struct symbol *sym, const char *source) +{ + struct symbol *file = sym->file; + const char *file_name, *source_name; + + if (!file || !*source) + return 0; + + file_name = basename(file->name); + source_name = basename(source); + if (strcmp(file_name, source_name)) { + if (!known_file_name_mismatch(file_name, source_name) && !file->warned) { + WARN("FILE %s symtab[%u] disagrees with DWARF source %s for %s; skipping source completion", + file->name, file->idx, source, sym->name); + file->warned = true; + } + return 0; + } + + /* Symbols sharing an STT_FILE must agree on the CU path. */ + if (file->identity.source != file->name && + strcmp(file->identity.source, source)) { + ERROR("FILE %s symtab[%u] has conflicting DWARF sources: %s and %s", + file->name, file->idx, file->identity.source, source); + return -1; + } + file->identity.source = source; + return 0; +} + +u32 identity_hash(const struct identity *identity) +{ + return jhash_3words(str_hash(identity->source), str_hash(identity->scope), + str_hash(identity->demangled_name), 0); +} + +bool same_identity(const struct identity *a, const struct identity *b) +{ + return !strcmp(a->source, b->source) && + !strcmp(a->scope, b->scope) && + !strcmp(a->demangled_name, b->demangled_name); +} + +/* Only accept identities from DWARF for uniqueness validation. */ +static int validate_dwarf_identity(struct elf *elf, struct symbol *sym) +{ + struct symbol *other; + u32 key = identity_hash(&sym->identity); + + elf_hash_for_each_possible(elf, symbol_identity, other, identity_hash, key) { + if (same_identity(&sym->identity, &other->identity) && + !strcmp(sym->name, other->name)) { + ERROR("%s: duplicate symbol identity (%s, %s, %s): symtab[%u] %s+%#lx (%u bytes) and symtab[%u] %s+%#lx (%u bytes)", + elf->name, sym->identity.source, sym->identity.scope, sym->name, + other->idx, other->sec->name, other->offset, other->len, + sym->idx, sym->sec->name, sym->offset, sym->len); + return -1; + } + } + elf_hash_add(symbol_identity, &sym->identity_hash, key); + return 0; +} + +int prepare_identities(struct elf *elf) +{ + struct symbol *sym; + int ret; + + if (init_identity_cache(elf) || + !elf_alloc_hash(symbol_identity, 1UL << elf->symbol_bits)) + return -1; + + for_each_sym(elf, sym) { + struct identity *identity = &sym->identity; + + /* Keep the FILE name for complete_file_source() and fallback identities. */ + identity->source = is_file_sym(sym) ? sym->name : ""; + if (!is_dwarf_identity_candidate(sym)) + continue; + ret = get_identity_for_symbol(elf, sym); + if (ret < 0) + return -1; + } + + for_each_sym(elf, sym) { + struct identity *identity = &sym->identity; + + /* A scope here means the identity was obtained from DWARF. */ + if (!identity->scope) + continue; + /* + * DW_AT_external describes source linkage, independent of LTO + * promotion/internalization of ELF binding. Omit source for + * external symbols so their definitions can move between TUs. + */ + if (identity->external) + identity->source = ""; + else if (complete_file_source(sym, identity->source)) + return -1; + if (validate_dwarf_identity(elf, sym)) + return -1; + } + + /* FILE fallback may change keys; rebuild the index below. */ + memset(elf->symbol_identity_hash, 0, + sizeof(*elf->symbol_identity_hash) << elf->symbol_identity_bits); + + for_each_sym(elf, sym) { + struct identity *identity = &sym->identity; + + /* Default to STT_FILE and "/" for missing DWARF information. */ + if (!identity->external && !*identity->source && sym->file) + identity->source = sym->file->identity.source; + if (!identity->scope) + identity->scope = "/"; + if (!sym->dont_correlate) + elf_hash_add(symbol_identity, &sym->identity_hash, identity_hash(identity)); + } + + return 0; +} diff --git a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh index 34f4f3a58e20..4e0e49a9fc56 100755 --- a/tools/objtool/tests/generic/test-thinlto-ambiguity.sh +++ b/tools/objtool/tests/generic/test-thinlto-ambiguity.sh @@ -38,7 +38,7 @@ build_thinlto() # $1 output object, $2 extra flags { local t for t in "" -DTU_B -DTU_C; do - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections \ + $THIN_CC -flto=thin -O2 -g -ffunction-sections -fdata-sections \ $2 $t -c "$FIXTURES_DIR/thinlto_ambiguity.c" \ -o "$workdir/tu$t.o" 2>/dev/null || return 1 done diff --git a/tools/objtool/tests/generic/test-thinlto-local.sh b/tools/objtool/tests/generic/test-thinlto-local.sh index a266263676ed..71156e0b59ce 100755 --- a/tools/objtool/tests/generic/test-thinlto-local.sh +++ b/tools/objtool/tests/generic/test-thinlto-local.sh @@ -15,9 +15,9 @@ find_thinlto_toolchain || build_thinlto() # $1 output object, $2 extra flags { - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 \ + $THIN_CC -flto=thin -O2 -g -ffunction-sections -fdata-sections $2 \ -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_a.o" 2>/dev/null || return 1 - $THIN_CC -flto=thin -O2 -ffunction-sections -fdata-sections $2 -DTU_B \ + $THIN_CC -flto=thin -O2 -g -ffunction-sections -fdata-sections $2 -DTU_B \ -c "$FIXTURES_DIR/thinlto_local.c" -o "$workdir/tu_b.o" 2>/dev/null || return 1 "$THIN_LD" -r "$workdir/tu_a.o" "$workdir/tu_b.o" -o "$1" 2>/dev/null || return 1 } diff --git a/tools/objtool/tests/lib.sh b/tools/objtool/tests/lib.sh index 4da168d0ddca..44ce670c4a6c 100644 --- a/tools/objtool/tests/lib.sh +++ b/tools/objtool/tests/lib.sh @@ -57,7 +57,7 @@ klp_preflight() OBJTOOL="$(realpath "$OBJTOOL")" "$OBJTOOL" klp 2>&1 | grep -q checksum || - bail "objtool was built without klp support; install libxxhash (>= 0.8) and rebuild" + bail "objtool was built without klp support; install libxxhash (>= 0.8) and libdw development files and rebuild" command -v "${CC%% *}" >/dev/null || bail "compiler not found: $CC" @@ -67,7 +67,7 @@ klp_preflight() tmp="$(mktemp -d)" || bail "mktemp failed" echo 'int probe(void) { return 0; }' > "$tmp/probe.c" - $CC -c -o "$tmp/probe.o" "$tmp/probe.c" 2>/dev/null || + $CC -g -c -o "$tmp/probe.o" "$tmp/probe.c" 2>/dev/null || { rm -rf "$tmp"; bail "$CC cannot compile a trivial object"; } # $CC, $ARCH and objtool have to agree about the target, and cross runs @@ -139,6 +139,7 @@ EOF # it lives at the compile where an override cannot drop it. # # -O2 the kernel's default +# -g required for DWARF symbol identity # -ffunction-sections -fdata-sections klp-build passes these itself, through # KCFLAGS, whatever the configuration # -fno-asynchronous-unwind-tables arch/x86/Makefile sets this always, so @@ -150,7 +151,7 @@ EOF # checksum # # A test overrides it; see tools/objtool/Documentation/klp-write-tests.txt. -FIXTURE_CFLAGS="-O2 -ffunction-sections -fdata-sections -fno-common \ +FIXTURE_CFLAGS="-O2 -g -ffunction-sections -fdata-sections -fno-common \ -fno-asynchronous-unwind-tables" test_name="$(basename "$0" .sh)" -- 2.53.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 2/3] objtool/klp: Include symbol identities in reference checksums 2026-10-26 10:19 [RFC PATCH 0/3] objtool/klp: DWARF-based cross-build symbol correlation Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 1/3] objtool/klp: Correlate symbols using DWARF identity Longjun Luo @ 2026-10-26 10:19 ` Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 3/3] objtool/klp: Test DWARF identities and " Longjun Luo 2 siblings, 0 replies; 4+ messages in thread From: Longjun Luo @ 2026-10-26 10:19 UTC (permalink / raw) To: jpoimboe, peterz Cc: mbenes, pmladek, joe.lawrence, song, puranjay, nathan, ndesaulniers, morbo, justinstitt, live-patching, llvm, linux-kernel, Longjun Luo A call or data reference can switch to a same-named symbol without changing the target's demangled name. Hashing that name alone can miss the change, even when the symbols have been correlated correctly. Prepare identities before calculating checksums and hash the target's source/scope/demangled_name path. This distinguishes TU-local and external targets while allowing compiler suffixes to change. Keep the single-object checksum command and its on-disk format unchanged. Tested with GCC and Clang, including same-named call/data targets, function-pointer tables, and four full-kernel vmlinux.o build pairs. This patch introduces no new test failures. The three policy rejections from the identity correlation patch remain; sympos and sympos-vmlinux now fail earlier, in klp checksum, because identities are prepared there. Assisted-by: LLM Signed-off-by: Longjun Luo <luolongjuna@gmail.com> --- tools/objtool/include/objtool/klp-identity.h | 1 + tools/objtool/include/objtool/klp.h | 2 ++ tools/objtool/klp-checksum.c | 25 +++++++++++++------- tools/objtool/klp-diff.c | 2 +- tools/objtool/klp-identity.c | 7 ++++++ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/tools/objtool/include/objtool/klp-identity.h b/tools/objtool/include/objtool/klp-identity.h index 0c7be8cfd49b..8483ce92f2e5 100644 --- a/tools/objtool/include/objtool/klp-identity.h +++ b/tools/objtool/include/objtool/klp-identity.h @@ -14,6 +14,7 @@ struct identity { const char *source; /* physical defining TU path, or "" */ const char *scope; /* slash-delimited enclosing scopes */ const char *demangled_name; /* suffix-stripped symbol name */ + const char *path; /* source/scope/demangled_name */ bool external; }; diff --git a/tools/objtool/include/objtool/klp.h b/tools/objtool/include/objtool/klp.h index c57775d78c71..564890cd1d5f 100644 --- a/tools/objtool/include/objtool/klp.h +++ b/tools/objtool/include/objtool/klp.h @@ -51,6 +51,8 @@ struct objtool_file; struct elf; struct symbol; +bool dont_correlate(struct symbol *sym); + int klp_create_symid_sections(struct objtool_file *file); int klp_sympos_init(struct elf *orig); diff --git a/tools/objtool/klp-checksum.c b/tools/objtool/klp-checksum.c index 1f1b5f068168..54b89c3e6d1b 100644 --- a/tools/objtool/klp-checksum.c +++ b/tools/objtool/klp-checksum.c @@ -98,8 +98,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, __checksum_update_insn(func, insn, buf, len); if (call_dest) { - __checksum_update_insn(func, insn, call_dest->identity.demangled_name, - strlen(call_dest->identity.demangled_name)); + __checksum_update_insn(func, insn, call_dest->identity.path, + strlen(call_dest->identity.path)); } else if (jump_dest) { struct symbol *dest_sym; @@ -115,8 +115,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, if (!dest_sym) goto alts; - __checksum_update_insn(func, insn, dest_sym->identity.demangled_name, - strlen(dest_sym->identity.demangled_name)); + __checksum_update_insn(func, insn, dest_sym->identity.path, + strlen(dest_sym->identity.path)); offset = jump_dest->offset - dest_sym->offset; __checksum_update_insn(func, insn, &offset, sizeof(offset)); @@ -158,8 +158,8 @@ static void checksum_update_insn(struct objtool_file *file, struct symbol *func, offset -= sym->offset; } - __checksum_update_insn(func, insn, sym->identity.demangled_name, - strlen(sym->identity.demangled_name)); + __checksum_update_insn(func, insn, sym->identity.path, + strlen(sym->identity.path)); __checksum_update_insn(func, insn, &offset, sizeof(offset)); alts: @@ -227,9 +227,9 @@ static void checksum_update_object(struct objtool_file *file, struct symbol *sym offset -= target->offset; } - __checksum_update_object(sym, sym_offset, "reloc name", - target->identity.demangled_name, - strlen(target->identity.demangled_name)); + __checksum_update_object(sym, sym_offset, "reloc identity", + target->identity.path, + strlen(target->identity.path)); __checksum_update_object(sym, sym_offset, "reloc addend", &offset, sizeof(offset)); } @@ -325,6 +325,7 @@ static const char * const klp_checksum_usage[] = { int cmd_klp_checksum(int argc, const char **argv) { struct objtool_file *file; + struct symbol *sym; int ret; const struct option options[] = { @@ -349,6 +350,12 @@ int cmd_klp_checksum(int argc, const char **argv) if (ret) goto out; + for_each_sym(file->elf, sym) + sym->dont_correlate = dont_correlate(sym); + ret = prepare_identities(file->elf); + if (ret) + goto out; + ret = calculate_checksums(file); if (ret) goto out; diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c index 957e589b6603..b86886c41950 100644 --- a/tools/objtool/klp-diff.c +++ b/tools/objtool/klp-diff.c @@ -425,7 +425,7 @@ static bool is_metadata_section(struct section *sec) * These symbols should never be correlated, so their local patched versions * are used instead of linking to the originals. */ -static bool dont_correlate(struct symbol *sym) +bool dont_correlate(struct symbol *sym) { return is_file_sym(sym) || is_null_sym(sym) || diff --git a/tools/objtool/klp-identity.c b/tools/objtool/klp-identity.c index 627eda694389..ca8e65d5d392 100644 --- a/tools/objtool/klp-identity.c +++ b/tools/objtool/klp-identity.c @@ -156,12 +156,19 @@ int prepare_identities(struct elf *elf) for_each_sym(elf, sym) { struct identity *identity = &sym->identity; + char *path; /* Default to STT_FILE and "/" for missing DWARF information. */ if (!identity->external && !*identity->source && sym->file) identity->source = sym->file->identity.source; if (!identity->scope) identity->scope = "/"; + if (asprintf(&path, "%s%s%s", identity->source, identity->scope, + identity->demangled_name) < 0) { + ERROR_GLIBC("asprintf"); + return -1; + } + identity->path = path; if (!sym->dont_correlate) elf_hash_add(symbol_identity, &sym->identity_hash, identity_hash(identity)); } -- 2.53.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC PATCH 3/3] objtool/klp: Test DWARF identities and reference checksums 2026-10-26 10:19 [RFC PATCH 0/3] objtool/klp: DWARF-based cross-build symbol correlation Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 1/3] objtool/klp: Correlate symbols using DWARF identity Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 2/3] objtool/klp: Include symbol identities in reference checksums Longjun Luo @ 2026-10-26 10:19 ` Longjun Luo 2 siblings, 0 replies; 4+ messages in thread From: Longjun Luo @ 2026-10-26 10:19 UTC (permalink / raw) To: jpoimboe, peterz Cc: mbenes, pmladek, joe.lawrence, song, puranjay, nathan, ndesaulniers, morbo, justinstitt, live-patching, llvm, linux-kernel, Longjun Luo Cover same-named static state in different scopes and TUs, compiler renumbering, ThinLTO promotion and name drift, Full LTO, and newly added source files. Check that patched functions reference their own original state and that new functions use new state. Verify that read-only descriptors retain their original addresses and reject changes to their contents. Also cover calls, data references and function-pointer tables switching between same-named local and external targets, and an external caller moving between TUs. Tested on x86-64 with tools/objtool/tests/run-tests.sh: GCC 16.2.1: 51 passed, 3 failed, 7 skipped. Clang 22.1.8: 57 passed, 3 failed, 1 skipped. All nine new tests pass with Clang; GCC passes six and skips the three Clang LTO tests. Both DWARF 4 and 5 were exercised. Without the reference checksum patch, only the two new checksum tests fail. The three tests covering behavior that this series rejects by policy still fail: local-to-global-flip (source-level linkage change), and sympos and sympos-vmlinux (duplicate identities from compiling the same source into multiple objects). They are left unchanged for discussion; if the policy is accepted, they will be updated to expect the rejection. Assisted-by: LLM Signed-off-by: Longjun Luo <luolongjuna@gmail.com> --- .../tests/generic/fixtures/identity/common.h | 15 +++++ .../generic/fixtures/identity/consumer.c | 30 +++++++++ .../tests/generic/fixtures/identity/entries.c | 10 +++ .../generic/fixtures/identity/full_left.c | 16 +++++ .../generic/fixtures/identity/full_right.c | 6 ++ .../generic/fixtures/identity/move_left.c | 9 +++ .../generic/fixtures/identity/move_right.c | 8 +++ .../generic/fixtures/identity/new/same.c | 6 ++ .../generic/fixtures/identity/new_driver.c | 15 +++++ .../generic/fixtures/identity/new_provider.c | 4 ++ .../generic/fixtures/identity/old/same.c | 6 ++ .../fixtures/identity/promotion_bounds_a.c | 12 ++++ .../fixtures/identity/promotion_bounds_b.c | 12 ++++ .../fixtures/identity/promotion_bounds_d.c | 8 +++ .../generic/fixtures/identity/provider.c | 9 +++ .../generic/fixtures/identity/readonly.c | 30 +++++++++ .../tests/generic/fixtures/identity/record.h | 8 +++ .../tests/generic/fixtures/identity/rx.c | 4 ++ .../generic/fixtures/identity/source_driver.c | 11 ++++ .../generic/fixtures/identity/static_scope.c | 31 +++++++++ .../fixtures/identity/static_scope_reorder.c | 31 +++++++++ .../tests/generic/fixtures/identity/tx.c | 4 ++ .../generic/test-checksum-identity-source.sh | 24 +++++++ .../tests/generic/test-checksum-identity.sh | 31 +++++++++ .../tests/generic/test-identity-full-lto.sh | 28 ++++++++ .../tests/generic/test-identity-new-tu.sh | 22 +++++++ .../generic/test-identity-readonly-address.sh | 23 +++++++ .../tests/generic/test-identity-source.sh | 26 ++++++++ .../generic/test-identity-static-scope.sh | 35 ++++++++++ .../test-identity-thinlto-name-drift.sh | 65 +++++++++++++++++++ .../test-identity-thinlto-promotion.sh | 25 +++++++ tools/objtool/tests/identity-lib.sh | 65 +++++++++++++++++++ 32 files changed, 629 insertions(+) create mode 100644 tools/objtool/tests/generic/fixtures/identity/common.h create mode 100644 tools/objtool/tests/generic/fixtures/identity/consumer.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/entries.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/full_left.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/full_right.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/move_left.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/move_right.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/new/same.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/new_driver.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/new_provider.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/old/same.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_a.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_b.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/promotion_bounds_d.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/provider.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/readonly.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/record.h create mode 100644 tools/objtool/tests/generic/fixtures/identity/rx.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/source_driver.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/static_scope.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/static_scope_reorder.c create mode 100644 tools/objtool/tests/generic/fixtures/identity/tx.c create mode 100755 tools/objtool/tests/generic/test-checksum-identity-source.sh create mode 100755 tools/objtool/tests/generic/test-checksum-identity.sh create mode 100755 tools/objtool/tests/generic/test-identity-full-lto.sh create mode 100755 tools/objtool/tests/generic/test-identity-new-tu.sh create mode 100755 tools/objtool/tests/generic/test-identity-readonly-address.sh create mode 100755 tools/objtool/tests/generic/test-identity-source.sh create mode 100755 tools/objtool/tests/generic/test-identity-static-scope.sh create mode 100755 tools/objtool/tests/generic/test-identity-thinlto-name-drift.sh create mode 100755 tools/objtool/tests/generic/test-identity-thinlto-promotion.sh create mode 100644 tools/objtool/tests/identity-lib.sh diff --git a/tools/objtool/tests/generic/fixtures/identity/common.h b/tools/objtool/tests/generic/fixtures/identity/common.h new file mode 100644 index 000000000000..d0f56751fcd9 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/common.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#ifndef KLP_TEST_IDENTITY_H +#define KLP_TEST_IDENTITY_H + +#ifdef __clang__ +/* Use noinline on Clang to keep calls visible in these tests. */ +#define NOIPA __attribute__((noinline)) +#else +#define NOIPA __attribute__((noipa)) +#endif +#define MODINFO static const char modinfo[] \ + __attribute__((used, section(".modinfo"))) = "name=vmlinux" + +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/consumer.c b/tools/objtool/tests/generic/fixtures/identity/consumer.c new file mode 100644 index 000000000000..4aad0e941bc3 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/consumer.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +#ifdef PATCHED +static NOIPA int helper(int uid) { return uid == 0 ? 0 : -1; } +#else +int helper(int uid); +#endif +#ifdef DATA +#ifdef PATCHED +static volatile int limit = 1500; +#else +extern volatile int limit; +#endif +#endif +#ifdef TABLE +struct operations { int (*check)(int); }; +struct operations ops = { .check = helper }; +#endif +NOIPA int caller(int uid) +{ +#ifdef DATA + return uid <= limit ? 0 : -90; +#elif defined(TABLE) + return ops.check(uid); +#else + return helper(uid); +#endif +} diff --git a/tools/objtool/tests/generic/fixtures/identity/entries.c b/tools/objtool/tests/generic/fixtures/identity/entries.c new file mode 100644 index 000000000000..53e809957c1c --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/entries.c @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +unsigned int tx(unsigned int n); +unsigned int rx(unsigned int n); +unsigned int entry_tx(unsigned int n) { return tx(n); } +#ifdef PATCHED +unsigned int entry_rx(unsigned int n) { return rx(n); } +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/full_left.c b/tools/objtool/tests/generic/fixtures/identity/full_left.c new file mode 100644 index 000000000000..1a75df588712 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/full_left.c @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +static volatile int state; +#ifdef PATCHED +static NOIPA int helper(void) { return ++state; } +#endif +NOIPA int left_fn(void) +{ +#ifdef PATCHED + return helper(); +#else + return ++state; +#endif +} diff --git a/tools/objtool/tests/generic/fixtures/identity/full_right.c b/tools/objtool/tests/generic/fixtures/identity/full_right.c new file mode 100644 index 000000000000..17583384cf32 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/full_right.c @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +static volatile int state; +static NOIPA int helper(void) { return ++state; } +NOIPA int right_fn(void) { return helper(); } diff --git a/tools/objtool/tests/generic/fixtures/identity/move_left.c b/tools/objtool/tests/generic/fixtures/identity/move_left.c new file mode 100644 index 000000000000..7d3b03bc87e5 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/move_left.c @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +static NOIPA int helper(int n) { return n <= 1500 ? 0 : -90; } +NOIPA int keep_left(int n) { return helper(n); } +#ifndef PATCHED +NOIPA int caller(int n) { return helper(n); } +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/move_right.c b/tools/objtool/tests/generic/fixtures/identity/move_right.c new file mode 100644 index 000000000000..9a5f82a7d9ba --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/move_right.c @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +static NOIPA int helper(int n) { return n <= 4096 ? 0 : -90; } +NOIPA int keep_right(int n) { return helper(n); } +#ifdef PATCHED +NOIPA int caller(int n) { return helper(n); } +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/new/same.c b/tools/objtool/tests/generic/fixtures/identity/new/same.c new file mode 100644 index 000000000000..54550f20fe79 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/new/same.c @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "../common.h" +static volatile int bytes; +static NOIPA int helper(int n) { return bytes += n; } +NOIPA int account_new(int n) { return helper(n); } diff --git a/tools/objtool/tests/generic/fixtures/identity/new_driver.c b/tools/objtool/tests/generic/fixtures/identity/new_driver.c new file mode 100644 index 000000000000..c06a1ac27609 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/new_driver.c @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +#ifdef PATCHED +int clamp(int value); +#endif +NOIPA int target(int value) +{ +#ifdef PATCHED + return clamp(value); +#else + return value; +#endif +} diff --git a/tools/objtool/tests/generic/fixtures/identity/new_provider.c b/tools/objtool/tests/generic/fixtures/identity/new_provider.c new file mode 100644 index 000000000000..69c1b3d0b6a7 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/new_provider.c @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +NOIPA int clamp(int value) { return value < 0 ? 0 : value; } diff --git a/tools/objtool/tests/generic/fixtures/identity/old/same.c b/tools/objtool/tests/generic/fixtures/identity/old/same.c new file mode 100644 index 000000000000..e03f14f16214 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/old/same.c @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "../common.h" +static volatile int bytes; +static NOIPA int helper(int n) { return bytes += n; } +NOIPA int account_old(int n) { return helper(n); } diff --git a/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_a.c b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_a.c new file mode 100644 index 000000000000..74eb8ac0bfc3 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_a.c @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +static volatile int state[4]; + +int a_get(unsigned int index) +{ +#ifdef PATCHED + if (index >= 4) + return 0; +#endif + return state[index]; +} diff --git a/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_b.c b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_b.c new file mode 100644 index 000000000000..2b96569ce6cd --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_b.c @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; + +int a_get(unsigned int index); +int d_get(unsigned int index); + +int target(unsigned int index) +{ + return a_get(index) + d_get(index); +} diff --git a/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_d.c b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_d.c new file mode 100644 index 000000000000..a4d6c35f80a9 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/promotion_bounds_d.c @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +static volatile int state[4]; + +int d_get(unsigned int index) +{ + return state[index]; +} diff --git a/tools/objtool/tests/generic/fixtures/identity/provider.c b/tools/objtool/tests/generic/fixtures/identity/provider.c new file mode 100644 index 000000000000..412c5cb67341 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/provider.c @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +NOIPA int helper(int uid) +{ + return uid == 0 || uid == 1000 ? 0 : -1; +} + +volatile int limit = 4096; diff --git a/tools/objtool/tests/generic/fixtures/identity/readonly.c b/tools/objtool/tests/generic/fixtures/identity/readonly.c new file mode 100644 index 000000000000..f145d3344d60 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/readonly.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +struct protocol { int kind; }; +NOIPA const struct protocol *tcp_type(int valid) +{ +#if defined(PATCHED) && defined(CHANGE_VALUE) + static const struct protocol type = { 4 }; +#else + static const struct protocol type = { 1 }; +#endif +#ifdef PATCHED + if (!valid) + return (void *)0; +#endif + return &type; +} +NOIPA const struct protocol *udp_type(int valid) +{ + static const struct protocol type = { 1 }; + return &type; +} +#ifdef PATCHED +NOIPA const struct protocol *unix_type(int valid) +{ + static const struct protocol type = { 1 }; + return &type; +} +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/record.h b/tools/objtool/tests/generic/fixtures/identity/record.h new file mode 100644 index 000000000000..5af00486c5f4 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/record.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +static inline unsigned int record(unsigned int n) +{ + static volatile unsigned int state; + + return state += n; +} diff --git a/tools/objtool/tests/generic/fixtures/identity/rx.c b/tools/objtool/tests/generic/fixtures/identity/rx.c new file mode 100644 index 000000000000..5d9871527b08 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/rx.c @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "record.h" +unsigned int rx(unsigned int n) { return record(n); } diff --git a/tools/objtool/tests/generic/fixtures/identity/source_driver.c b/tools/objtool/tests/generic/fixtures/identity/source_driver.c new file mode 100644 index 000000000000..0ebcb01ced02 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/source_driver.c @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; +#ifdef PATCHED +int account_new(int n); +NOIPA int account(int n) { return account_new(n); } +#else +int account_old(int n); +NOIPA int account(int n) { return account_old(n); } +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/static_scope.c b/tools/objtool/tests/generic/fixtures/identity/static_scope.c new file mode 100644 index 000000000000..a467f24f1a14 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/static_scope.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; + +NOIPA int tx(int n) +{ + static volatile int bytes; + +#ifdef PATCHED + if (n < 0) + return 0; +#endif + return bytes += n; +} + +NOIPA int rx(int n) +{ + static volatile int bytes; + + return bytes += n; +} + +#ifdef PATCHED +NOIPA int extra(int n) +{ + static volatile int bytes; + + return bytes += n; +} +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/static_scope_reorder.c b/tools/objtool/tests/generic/fixtures/identity/static_scope_reorder.c new file mode 100644 index 000000000000..a451e632a6cb --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/static_scope_reorder.c @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "common.h" +MODINFO; + +#ifdef PATCHED +NOIPA int rx(int n) +{ + static volatile int bytes; + + if (n < 0) + return 0; + return bytes += n; +} +#endif + +NOIPA int tx(int n) +{ + static volatile int bytes; + + return bytes += n; +} + +#ifndef PATCHED +NOIPA int rx(int n) +{ + static volatile int bytes; + + return bytes += n; +} +#endif diff --git a/tools/objtool/tests/generic/fixtures/identity/tx.c b/tools/objtool/tests/generic/fixtures/identity/tx.c new file mode 100644 index 000000000000..d536c3c63549 --- /dev/null +++ b/tools/objtool/tests/generic/fixtures/identity/tx.c @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> */ +#include "record.h" +unsigned int tx(unsigned int n) { return record(n); } diff --git a/tools/objtool/tests/generic/test-checksum-identity-source.sh b/tools/objtool/tests/generic/test-checksum-identity-source.sh new file mode 100755 index 000000000000..b320a57c6a76 --- /dev/null +++ b/tools/objtool/tests/generic/test-checksum-identity-source.sh @@ -0,0 +1,24 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Moving an external caller changes which TU-local helper it calls. +. "$(dirname "$0")/../identity-lib.sh" + +setup +modes=(none) +case "$($CC --version | head -1)" in *clang*) modes+=(thin) ;; esac + +for mode in "${modes[@]}"; do + build_identity_pair "$mode" "" identity/move_left.c identity/move_right.c + [ "$(in_symbols orig.o | awk '$4 == "FUNC" && $8 ~ /^helper($|.llvm.)/' | wc -l)" = 2 ] || + fail "fixture lacks two TU-local helpers" + assert_checksum_differs caller + assert_checksum_matches keep_left + assert_checksum_matches keep_right + run_diff + assert_patched caller + assert_not_patched keep_left + assert_not_patched keep_right +done + +pass "moving a caller between TUs detects the changed local target" diff --git a/tools/objtool/tests/generic/test-checksum-identity.sh b/tools/objtool/tests/generic/test-checksum-identity.sh new file mode 100755 index 000000000000..77248e547040 --- /dev/null +++ b/tools/objtool/tests/generic/test-checksum-identity.sh @@ -0,0 +1,31 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Checksums for calls, data references and function-pointer tables must +# distinguish same-named static and external targets. +. "$(dirname "$0")/../identity-lib.sh" + +setup +modes=(none) +case "$($CC --version | head -1)" in *clang*) modes+=(thin) ;; esac + +for mode in "${modes[@]}"; do + build_identity_pair "$mode" "" identity/provider.c identity/consumer.c + [ "$(in_symbols patched.o | awk '$4 == "FUNC" && $8 ~ /^helper($|.llvm.)/' | wc -l)" = 2 ] || + fail "fixture lacks the local and external helpers" + assert_checksum_differs caller + run_diff + assert_patched caller + + build_identity_pair "$mode" -DTABLE identity/provider.c identity/consumer.c + assert_checksum_differs ops + run_diff 255 + assert_diff_log 'changed data: ops' + + build_identity_pair "$mode" -DDATA identity/provider.c identity/consumer.c + assert_checksum_differs caller + run_diff + assert_patched caller +done + +pass "call, data and function-pointer targets use the full identity" diff --git a/tools/objtool/tests/generic/test-identity-full-lto.sh b/tools/objtool/tests/generic/test-identity-full-lto.sh new file mode 100755 index 000000000000..7d68a7de7982 --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-full-lto.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Full LTO merges FILE entries; a new helper must not replace an existing TU's helper. +. "$(dirname "$0")/../identity-lib.sh" + +setup +build_identity_pair full "" identity/full_left.c identity/full_right.c + +[ "$(in_symbols orig.o | awk '$4 == "FUNC" && $8 ~ /^helper(\.[0-9]+)?$/' | wc -l)" = 1 ] && +[ "$(in_symbols patched.o | awk '$4 == "FUNC" && $8 ~ /^helper(\.[0-9]+)?$/' | wc -l)" = 2 ] || + probe_skip "Full LTO did not emit two same-base helper names" +left_helper="$(identity_relocs patched.o .rela.text.left_fn | awk '{ sub(/^\.text\./, "", $5); print $5 }')" +right_helper="$(identity_relocs patched.o .rela.text.right_fn | awk '{ sub(/^\.text\./, "", $5); print $5 }')" +[ -n "$left_helper" ] && [ -n "$right_helper" ] || fail "missing helper references" +left_state="$(identity_object_for orig.o left_fn)" +[ -n "$left_state" ] || fail "missing left state reference" + +run_diff +assert_patched left_fn +assert_not_patched right_fn +assert_patched "$left_helper" +assert_not_patched "$right_helper" +assert_reloc_sym .text.left_fn "$left_helper" +run_post_link +assert_original_reference "$left_helper" "$left_state" + +pass "Full LTO helpers retain their defining TUs" diff --git a/tools/objtool/tests/generic/test-identity-new-tu.sh b/tools/objtool/tests/generic/test-identity-new-tu.sh new file mode 100755 index 000000000000..31b2e10de83f --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-new-tu.sh @@ -0,0 +1,22 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# A patch may extract validation into a newly added source file. +. "$(dirname "$0")/../identity-lib.sh" + +setup +build_pair identity/new_driver.c +build_one identity/new_provider.c provider.o +mv "$workdir/patched.o" "$workdir/driver.o" +partial_link "$workdir/patched.o" "$workdir/driver.o" "$workdir/provider.o" || fail "partial link failed" + +[ "$(in_symbols patched.o | awk '$4 == "FILE"' | wc -l)" -gt "$(in_symbols orig.o | awk '$4 == "FILE"' | wc -l)" ] || + fail "fixture did not add a FILE entry" + +run_diff +assert_patched target +assert_patched clamp +assert_diff_log 'changed function: target' +assert_diff_log 'new function: clamp' + +pass "new translation unit does not block existing symbol correlation" diff --git a/tools/objtool/tests/generic/test-identity-readonly-address.sh b/tools/objtool/tests/generic/test-identity-readonly-address.sh new file mode 100755 index 000000000000..18c49856833b --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-readonly-address.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Read-only descriptors still need their original pointer identity. +. "$(dirname "$0")/../identity-lib.sh" + +setup +build_pair identity/readonly.c +old="$(identity_object_for orig.o tcp_type)" +[ -n "$old" ] || fail "missing TCP descriptor reference" + +run_diff +assert_patched tcp_type +assert_patched unix_type +assert_not_patched udp_type +run_post_link +assert_original_reference tcp_type "$old" + +build_pair identity/readonly.c -DCHANGE_VALUE +run_diff 255 +assert_diff_log 'changed data:' + +pass "changed function retains the original read-only descriptor address" diff --git a/tools/objtool/tests/generic/test-identity-source.sh b/tools/objtool/tests/generic/test-identity-source.sh new file mode 100755 index 000000000000..1c551cb91afc --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-source.sh @@ -0,0 +1,26 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# old/same.c and new/same.c share the FILE name "same.c"; static state +# from the new file must still be treated as new. +. "$(dirname "$0")/../identity-lib.sh" + +setup +build_pair identity/source_driver.c +build_one identity/old/same.c old.o +build_one identity/new/same.c new.o +mv "$workdir/orig.o" "$workdir/driver-orig.o" +mv "$workdir/patched.o" "$workdir/driver-patched.o" +partial_link "$workdir/orig.o" "$workdir/driver-orig.o" "$workdir/old.o" || fail "original link failed" +partial_link "$workdir/patched.o" "$workdir/driver-patched.o" "$workdir/new.o" || fail "patched link failed" +for obj in orig.o patched.o; do + in_symbols "$obj" | awk '$4 == "FILE" && $8 == "same.c"' | grep -q . || fail "missing same.c FILE" +done + +run_diff +assert_patched account +assert_patched helper +assert_no_klp_sym bytes +assert_section .bss.bytes + +pass "private state from a different TU is new even with the same basename" diff --git a/tools/objtool/tests/generic/test-identity-static-scope.sh b/tools/objtool/tests/generic/test-identity-static-scope.sh new file mode 100755 index 000000000000..fc0372100886 --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-static-scope.sh @@ -0,0 +1,35 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Reordering functions or adding a same-named static must retain old state. +. "$(dirname "$0")/../identity-lib.sh" + +setup + +build_pair identity/static_scope.c +tx_state="$(identity_object_for orig.o tx)" +extra_state="$(identity_object_for patched.o extra)" +[ -n "$tx_state" ] && [ -n "$extra_state" ] || fail "missing static reference" + +run_diff +assert_patched tx +assert_not_patched rx +assert_patched extra +assert_section ".bss.$extra_state" +assert_reloc_sym .text.extra "$extra_state" +run_post_link +assert_original_reference tx "$tx_state" + +build_pair identity/static_scope_reorder.c +rx_state="$(identity_object_for orig.o rx)" +[ -n "$rx_state" ] || fail "missing RX static reference" + +run_diff +assert_patched rx +assert_not_patched tx +out_symbols | awk '$4 == "OBJECT" && $7 != "UND" && $8 ~ /bytes/' | grep -q . && + fail "existing static state was copied" +run_post_link +assert_original_reference rx "$rx_state" + +pass "named scopes preserve state through renumbering and additions" diff --git a/tools/objtool/tests/generic/test-identity-thinlto-name-drift.sh b/tools/objtool/tests/generic/test-identity-thinlto-name-drift.sh new file mode 100755 index 000000000000..03bb6cf2c074 --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-thinlto-name-drift.sh @@ -0,0 +1,65 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# A bounds check stops importing a_get(), leaving its state local. +# With -always-rename-promoted-locals=false (used by the kernel when +# supported), the plain promoted name moves to d.c and ELF-only matching +# silently redirects a_get() to d.c's state. Older LLVM loses correlation +# and copies the state instead. Both must retain a.c's state. +. "$(dirname "$0")/../identity-lib.sh" + +setup +clang_only "ThinLTO requires clang" +find_thinlto_toolchain || probe_skip "no matching clang/lld pair" +CC="$THIN_CC" +FIXTURE_CFLAGS="$FIXTURE_CFLAGS -flto=thin -fsplit-lto-unit -fvisibility=hidden" + +build_one identity/promotion_bounds_a.c orig-a.o +build_one identity/promotion_bounds_a.c patched-a.o -DPATCHED +build_one identity/promotion_bounds_b.c b.o +build_one identity/promotion_bounds_d.c d.o + +# Match the kernel's ThinLTO import limit and promotion naming policy. +link_flags=(--lto-whole-program-visibility -mllvm -import-instr-limit=5) +rename_flags=() +if "$THIN_LD" -r -mllvm -always-rename-promoted-locals=false \ + "$workdir/orig-a.o" -o "$workdir/rename-probe.o" \ + 2>"$workdir/rename-probe.log"; then + rename_flags=(-mllvm -always-rename-promoted-locals=false) +fi +for side in orig patched; do + "$THIN_LD" -r "${link_flags[@]}" "${rename_flags[@]}" \ + "$workdir/$side-a.o" "$workdir/b.o" "$workdir/d.o" \ + -o "$workdir/$side.o" 2>"$workdir/$side-link.log" || + fail "ThinLTO link failed: $(tail -1 "$workdir/$side-link.log")" +done + +state_a="$(identity_relocs orig.o .rela.text.a_get | + awk '$5 ~ /^state(\.llvm\.[0-9]+)?$/ { print $5; exit }')" +state_d="$(identity_relocs orig.o .rela.text.d_get | + awk '$5 ~ /^state(\.llvm\.[0-9]+)?$/ { print $5; exit }')" +[ -n "$state_a" ] && [ -n "$state_d" ] && [ "$state_a" != "$state_d" ] || + probe_skip "ThinLTO did not emit distinct promoted state references" +[ "$(identity_object_for patched.o a_get)" = state ] || + probe_skip "the bounds check did not leave a_get's state local" +in_symbols patched.o | awk '$4 == "OBJECT" && $5 == "LOCAL" && $8 == "state"' | + grep -q . || probe_skip "missing local state after the bounds check" + +if [ "${#rename_flags[@]}" -ne 0 ]; then + [ "$state_a" = state ] && [[ "$state_d" = state.llvm.* ]] || + probe_skip "a_get's state did not claim the original plain name" + in_symbols patched.o | awk '$4 == "OBJECT" && $5 == "GLOBAL" && $8 == "state"' | + grep -q . || probe_skip "the plain promoted name did not move to d.c" +fi + +run_diff +assert_patched a_get +assert_patched target +assert_not_patched d_get +out_symbols | awk '$4 == "OBJECT" && $7 != "UND" && $8 ~ /^state(\.|$)/' | + grep -q . && fail "unchanged state was allocated again" +run_post_link +assert_original_reference a_get "$state_a" +assert_original_reference target "$state_d" + +pass "ThinLTO import changes preserve both TUs' original state" diff --git a/tools/objtool/tests/generic/test-identity-thinlto-promotion.sh b/tools/objtool/tests/generic/test-identity-thinlto-promotion.sh new file mode 100755 index 000000000000..62f9cc70568a --- /dev/null +++ b/tools/objtool/tests/generic/test-identity-thinlto-promotion.sh @@ -0,0 +1,25 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# A new caller makes ThinLTO import rx() and promote rx.c's copy of +# record()'s static state; the patch must keep using the original state. +. "$(dirname "$0")/../identity-lib.sh" + +setup +build_identity_pair thin "" identity/tx.c identity/rx.c identity/entries.c + +in_symbols orig.o | awk '$4 == "OBJECT" && $5 == "LOCAL" && $8 == "record.state"' | + grep -q . || probe_skip "RX state was not local before importing" +[ "$(in_symbols patched.o | awk '$4 == "OBJECT" && $5 == "GLOBAL" && $8 ~ /^record.state.llvm./' | wc -l)" = 2 ] || + probe_skip "ThinLTO did not promote both state instances" +rx_state="$(identity_object_for orig.o rx)" +[ -n "$rx_state" ] || fail "missing RX static reference" + +run_diff +assert_patched entry_rx +out_symbols | awk '$4 == "OBJECT" && $7 != "UND" && $8 ~ /record.state/' | grep -q . && + fail "promoted state was allocated again" +run_post_link +assert_original_reference entry_rx "$rx_state" + +pass "header state retains its defining TU across promotion" diff --git a/tools/objtool/tests/identity-lib.sh b/tools/objtool/tests/identity-lib.sh new file mode 100644 index 000000000000..69d9619ae7d8 --- /dev/null +++ b/tools/objtool/tests/identity-lib.sh @@ -0,0 +1,65 @@ +# SPDX-License-Identifier: GPL-2.0 +# Copyright (C) 2026 Longjun Luo <luolongjuna@gmail.com> +# Shared build helpers for the multi-TU identity fixtures. +. "$(dirname "$0")/../lib.sh" + +FIXTURE_CFLAGS="$FIXTURE_CFLAGS -fno-pie -fno-pic" + +# build_identity_pair <none|thin|full> <extra-cflags> <fixture.c...> +build_identity_pair() +{ + local mode="$1" cflags="$2" side file i + local CC="$CC" + local flags=() parts=() patched=() + shift 2 + if [ "$mode" != none ]; then + clang_only "LTO fixtures require clang" + find_thinlto_toolchain || probe_skip "no matching clang/lld pair" + flags=("-flto=$mode") + CC="$THIN_CC" + fi + orig_obj=orig.o + patched_obj=patched.o + for side in orig patched; do + patched=() + [ "$side" = patched ] && patched=(-DPATCHED) + parts=() + i=0 + for file in "$@"; do + build_one "$file" "$side-$i.o" "${flags[@]}" "${patched[@]}" $cflags + parts+=("$workdir/$side-$i.o") + i=$((i + 1)) + done + if [ "$mode" = none ]; then + partial_link "$workdir/$side.o" "${parts[@]}" || fail "partial link failed" + else + "$THIN_LD" -r "${parts[@]}" -o "$workdir/$side.o" || fail "LTO link failed" + fi + done +} + +# identity_relocs <object> <relocation-section> +identity_relocs() +{ + # readelf prints the section name in single quotes. + in_relocs "$1" | awk -v sec="'$2'" ' + /Relocation section/ { active = $3 == sec } + active && $1 ~ /^[[:xdigit:]]+$/ { print }' +} + +# The fixture's function references one static in .bss or .rodata. +identity_object_for() +{ + identity_relocs "$1" ".rela.text.$2" | awk ' + $5 ~ /^\.(bss|rodata)\./ { + sub(/^\.(bss|rodata)\./, "", $5); print $5; exit + }' +} + +# After post-link, check the reference to the original symbol, not just its presence. +assert_original_reference() +{ + identity_relocs out.o ".klp.rela.orig..text.$1" | + awk -v n=".klp.sym.orig.$2," 'index($5, n) == 1' | grep -q . || + fail "expected '$1' to reference original '$2'" +} -- 2.53.0 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-26 10:20 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-26 10:19 [RFC PATCH 0/3] objtool/klp: DWARF-based cross-build symbol correlation Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 1/3] objtool/klp: Correlate symbols using DWARF identity Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 2/3] objtool/klp: Include symbol identities in reference checksums Longjun Luo 2026-10-26 10:19 ` [RFC PATCH 3/3] objtool/klp: Test DWARF identities and " Longjun Luo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®