commit 152604e1ae971b6f30497fc904a7d9857d6968bb
parent 0cf3dff4e27ef007e400dc08a2216d98d85ba21a
Author: Ryan Sepassi <rsepassi@gmail.com>
Date: Tue, 16 Jun 2026 15:24:07 -0700
link reports: fix --print-memory-usage alias double-count + high-half region, --cref imports
Three correctness fixes in the linker report writers (src/api/link.c):
- Bug 7 (--print-memory-usage): a section placed into multiple PT_LOAD phdrs
appends ALIAS LinkSegments (nsections==0) describing the SAME bytes; the
memory-usage loop charged each, double-counting an N-phdr section N times.
Skip alias/phdr-only segments (nsections==0); real primaries and BSS/COMMON
zero-fill carry nsections>=1, so genuine bytes stay counted exactly once.
- Bug 8 (--print-memory-usage): the region-window test computed
origin+length as u64, which wraps to 0 for a region touching the top of the
address space (e.g. high-half kernel ORIGIN=0xFFFFFFFF80000000
LENGTH=0x80000000) → nothing attributed (used=0). Use overflow-safe
subtract-based membership: base-origin < length.
- Bug 9 (--cref): the table omitted imported (DSO-resolved) externals — the
cross-references a cref table exists to show — because it shared the nm-style
symbol predicate that drops imports. Build the cref symbol set independently
(defined-non-imported OR imported, excluding FILE/SECTION), dedup by name,
sort by name; for an imported symbol the def line names the providing DSO
(basename via dso_input_id) or '-'.
Tests in test/buildcmds/run.sh (red-then-green verified): dual-phdr used equals
single-phdr used (Bug 7); high-half region reports nonzero used (Bug 8, skips
under the pre-existing link_script.c hex-parser UBSan trap on the debug build);
--cref over an imported libc_marker lists the symbol with its DSO def (Bug 9).
Diffstat:
| M | src/api/link.c | | | 112 | ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------- |
| M | test/buildcmds/run.sh | | | 112 | +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ |
2 files changed, 197 insertions(+), 27 deletions(-)
diff --git a/src/api/link.c b/src/api/link.c
@@ -995,48 +995,96 @@ static KitStatus link_report_write_map_image(KitLinkSession* s,
return link_report_write_symbols_nm(img, out);
}
-/* --cref: cross-reference table. For each global/weak defined or imported
- * symbol (sorted by name for determinism) emit the defining file, then the
- * set of distinct input files that reference it via a relocation. Input names
- * are normalized to basenames. */
+/* --cref predicate: a symbol participates in the cross-reference table when it
+ * is a named non-FILE/non-SECTION global that is either locally defined OR
+ * imported from a DSO. Imported (DSO-resolved) externals are exactly the
+ * cross-references a cref table exists to show, so — unlike the nm-style
+ * "symbols" listing's link_report_symbol_include — they are NOT dropped. */
+static int link_report_cref_include(const LinkSymbol* s) {
+ if (!s || !s->name) return 0;
+ if (s->kind == SK_FILE || s->kind == SK_SECTION) return 0;
+ return (s->defined && !s->imported) || s->imported;
+}
+
+/* --cref: cross-reference table. For each defined-or-imported global/weak
+ * symbol (deduped by name, sorted by name for determinism) emit the defining
+ * file — the providing DSO basename for an imported symbol, the defining object
+ * otherwise — then the set of distinct input files that reference it via a
+ * relocation. Input names are normalized to basenames. The symbol set is built
+ * INDEPENDENTLY of the nm-style "symbols" listing so imports are included. */
static KitStatus link_report_write_cref_image(KitLinkSession* s,
KitWriter* out) {
LinkImage* img = s->image;
- LinkReportSymbols syms;
+ Heap* h;
+ LinkSymId* ids = NULL;
+ u32 n = 0, cap, i;
KitStatus st;
- u32 i;
if (!img) return KIT_INVALID;
+ h = img->heap;
st = link_report_cstr(out, "cross reference table\n");
if (st != KIT_OK) return st;
- st = link_report_collect_symbols(img, &syms);
- if (st != KIT_OK) return st;
- for (i = 0; i < syms.n; ++i) {
- const LinkSymbol* sym = LinkSyms_at(&img->syms, syms.ids[i] - 1u);
+ cap = LinkSyms_count(&img->syms);
+ if (!cap) return kit_writer_status(out);
+ ids = (LinkSymId*)h->alloc(h, sizeof(*ids) * cap, _Alignof(LinkSymId));
+ if (!ids) return KIT_NOMEM;
+ /* Collect one canonical slot per qualifying name, deduped by Sym (the
+ * interned name handle): a name's first qualifying slot wins. Prefer a
+ * locally-defined slot over an imported one when both exist (so a symbol that
+ * is both defined and referenced through a DSO reports its real def). */
+ for (i = 0; i < cap; ++i) {
+ const LinkSymbol* sy = LinkSyms_at(&img->syms, i);
+ u32 k;
+ int seen = 0;
+ if (!link_report_cref_include(sy)) continue;
+ for (k = 0; k < n; ++k) {
+ LinkSymbol* prev = LinkSyms_at(&img->syms, ids[k] - 1u);
+ if (prev->name != sy->name) continue;
+ seen = 1;
+ /* Upgrade a previously-recorded imported slot to a real definition. */
+ if (sy->defined && !sy->imported && (!prev->defined || prev->imported))
+ ids[k] = sy->id;
+ break;
+ }
+ if (!seen) ids[n++] = sy->id;
+ }
+ /* Insertion sort by name (small lists; deterministic). */
+ for (i = 1; i < n; ++i) {
+ LinkSymId key = ids[i];
+ KitSlice kn =
+ link_report_sym_name(img, LinkSyms_at(&img->syms, key - 1u)->name);
+ u32 j = i;
+ while (j > 0) {
+ KitSlice pn = link_report_sym_name(
+ img, LinkSyms_at(&img->syms, ids[j - 1u] - 1u)->name);
+ if (link_report_slice_cmp(pn, kn) <= 0) break;
+ ids[j] = ids[j - 1u];
+ --j;
+ }
+ ids[j] = key;
+ }
+ for (i = 0; i < n; ++i) {
+ const LinkSymbol* sym = LinkSyms_at(&img->syms, ids[i] - 1u);
KitSlice name = link_report_sym_name(img, sym->name);
KitSlice deffile;
LinkInputId defining_input = LINK_INPUT_NONE;
u32 r, k;
LinkInputId last = LINK_INPUT_NONE;
- /* Aggregate by NAME, not by per-input slot id: collect_symbols includes a
- * resolved slot per input, so a name can repeat (its sorted by vaddr then
- * name → adjacent). Skip a name already emitted as the previous entry. */
- if (i > 0) {
- const LinkSymbol* prev = LinkSyms_at(&img->syms, syms.ids[i - 1u] - 1u);
- if (prev->name == sym->name) continue;
- }
- /* Find the defining file: the canonical global slot (img->globals) is the
- * real definition; resolution copies its value into per-input reference
- * slots, so scanning slots by `defined` alone would mis-attribute. Fall
- * back to any defined same-name slot for locals (never in globals). */
- {
+ if (sym->imported) {
+ /* Imported symbol: the "definition" is the providing DSO input. */
+ defining_input = sym->dso_input_id;
+ } else {
+ /* Find the defining file: the canonical global slot (img->globals) is the
+ * real definition; resolution copies its value into per-input reference
+ * slots, so scanning slots by `defined` alone would mis-attribute. Fall
+ * back to any defined same-name slot for locals (never in globals). */
LinkSymId canon = symhash_get(&img->globals, sym->name);
if (canon != LINK_SYM_NONE) {
const LinkSymbol* c = LinkSyms_at(&img->syms, canon - 1u);
if (c->defined && !c->imported) defining_input = c->input_id;
}
if (defining_input == LINK_INPUT_NONE) {
- for (k = 0; k < syms.n; ++k) {
- const LinkSymbol* c = LinkSyms_at(&img->syms, syms.ids[k] - 1u);
+ for (k = 0; k < cap; ++k) {
+ const LinkSymbol* c = LinkSyms_at(&img->syms, k);
if (c->name != sym->name) continue;
if (c->defined && !c->imported && c->input_id != LINK_INPUT_NONE) {
defining_input = c->input_id;
@@ -1078,7 +1126,7 @@ static KitStatus link_report_write_cref_image(KitLinkSession* s,
}
if (st != KIT_OK) break;
}
- link_report_free_symbols(img, &syms);
+ h->free(h, ids, sizeof(*ids) * cap);
return st == KIT_OK ? kit_writer_status(out) : st;
}
@@ -1106,9 +1154,19 @@ static KitStatus link_report_write_memory_usage_image(KitLinkSession* s,
for (j = 0; j < img->nsegments; ++j) {
const LinkSegment* seg = &img->segments[j];
u64 base = img->load_base + seg->vaddr;
+ /* Alias / program-header-only segments (nsections==0) describe the SAME
+ * bytes as a primary segment under an additional :phdr; charging them
+ * would double-count a section placed into multiple PT_LOADs. Real
+ * primaries (incl. BSS/COMMON zero-fill) carry nsections>=1, so genuine
+ * mem_size stays counted exactly once. */
+ if (seg->nsections == 0) continue;
/* A segment belongs to a region when its load address sits within the
- * region window. Count memsz (covers BSS). */
- if (base >= rg->origin && base < rg->origin + rg->length)
+ * [origin, origin+length) window. Count memsz (covers BSS). The end of
+ * the window is computed overflow-safe via a subtract — a region that
+ * touches the top of the address space (origin+length wraps to 0) would
+ * otherwise mis-test `base < 0` and attribute nothing. */
+ if (base >= rg->origin &&
+ (rg->length == 0 ? 0 : base - rg->origin < rg->length))
used += seg->mem_size;
}
free_bytes = (rg->length > used) ? rg->length - used : 0;
diff --git a/test/buildcmds/run.sh b/test/buildcmds/run.sh
@@ -771,6 +771,118 @@ contains fx2-fs-hosted-cross-diag "$work/fx2-fs-hosted-cross.err" \
# Same-arch hosted freestanding link still succeeds (no false positive).
run_ok fx2-fs-hosted-same "$KIT" build-exe -target x86_64-linux -ffreestanding \
-nostdlib -static -no-pie -e _start kstart.c fx2_fs_x64.o -o fx2_fs_same.elf
+# Link report-writer correctness fixes (kernel-FY): --print-memory-usage alias
+# double-count + high-half region overflow, and --cref imported-symbol coverage.
+# ===========================================================================
+
+# Helper: extract a region's "used=0x..." token from a --print-memory-usage
+# output file by region name (first match wins).
+fy_memuse_used() {
+ awk -v rg="$1" '$1==rg { for (i=1;i<=NF;i++) if ($i ~ /^used=/) { print substr($i,6); exit } }' "$2"
+}
+
+# ---- Bug 7: a section placed into N program headers (alias segments) must be
+# charged to its MEMORY region ONCE, not N times. We compare the single-phdr
+# `used` to the dual-phdr `used` for an otherwise-identical layout; they must be
+# equal (no double-count). (Repro: dual-phdr was used=0x84 vs single 0x44.)
+printf 'void _start(void){for(;;){}}\nint fydata=7;\n' > fymem.c
+cat > fy_single.ld <<'LDEOF'
+PHDRS { a PT_LOAD; }
+MEMORY { RAM (rwx) : ORIGIN = 0x80000000, LENGTH = 0x100000 }
+SECTIONS {
+ . = 0x80000000;
+ .text : { *(.text*) } :a
+ .data : { *(.data*) } :a
+ .bss : { *(.bss*) } :a
+}
+LDEOF
+cat > fy_dual.ld <<'LDEOF'
+PHDRS { a PT_LOAD; b PT_LOAD; }
+MEMORY { RAM (rwx) : ORIGIN = 0x80000000, LENGTH = 0x100000 }
+SECTIONS {
+ . = 0x80000000;
+ .text : { *(.text*) } :a :b
+ .data : { *(.data*) } :a
+ .bss : { *(.bss*) } :a
+}
+LDEOF
+run_ok fy-memuse-single "$KIT" build-exe -target riscv64-none-elf -ffreestanding \
+ -nostdlib -static -no-pie -T fy_single.ld --print-memory-usage fymem.c \
+ -o fymem_single.elf
+run_ok fy-memuse-dual "$KIT" build-exe -target riscv64-none-elf -ffreestanding \
+ -nostdlib -static -no-pie -T fy_dual.ld --print-memory-usage fymem.c \
+ -o fymem_dual.elf
+fy_used_single=$(fy_memuse_used RAM "$work/fy-memuse-single.out")
+fy_used_dual=$(fy_memuse_used RAM "$work/fy-memuse-dual.out")
+if [ -n "$fy_used_single" ] && [ "$fy_used_single" = "$fy_used_dual" ]; then
+ ok fy-memuse-alias-not-doubled
+else
+ { printf 'alias segment double-counts region usage\n';
+ printf ' single-phdr used=%s\n' "$fy_used_single";
+ printf ' dual-phdr used=%s (expected equal)\n' "$fy_used_dual";
+ } > "$work/fy-memuse-alias-not-doubled.diag"
+ not_ok fy-memuse-alias-not-doubled "$work/fy-memuse-alias-not-doubled.diag"
+fi
+
+# ---- Bug 8: a MEMORY region touching the TOP of the address space
+# (ORIGIN=0xFFFFFFFF80000000, LENGTH=0x80000000 → origin+length wraps to 0) must
+# still attribute the image that lives inside it (nonzero used / nonzero %age).
+# Under an ASan/UBSan build the linker-script hex parser traps on the 16-digit
+# ORIGIN (a separate, pre-existing defect in src/link/link_script.c, owned by
+# another agent); in that case the high-half script can't even be parsed, so we
+# skip rather than fail for an unrelated reason.
+cat > fy_high.ld <<'LDEOF'
+MEMORY { RAM (rwx) : ORIGIN = 0xFFFFFFFF80000000, LENGTH = 0x80000000 }
+SECTIONS {
+ . = 0xFFFFFFFF80000000;
+ .text : { *(.text*) }
+ .data : { *(.data*) }
+ .bss : { *(.bss*) }
+}
+LDEOF
+if "$KIT" build-exe -target riscv64-none-elf -ffreestanding -nostdlib -static \
+ -no-pie -T fy_high.ld --print-memory-usage fymem.c -o fymem_high.elf \
+ > "$work/fy-memuse-high.out" 2> "$work/fy-memuse-high.err"; then
+ fy_used_high=$(fy_memuse_used RAM "$work/fy-memuse-high.out")
+ if [ -n "$fy_used_high" ] && [ "$fy_used_high" != "0x0" ]; then
+ ok fy-memuse-high-half-nonzero
+ else
+ { printf 'high-half region reports zero usage (origin+length wrap)\n';
+ sed 's/^/out: /' "$work/fy-memuse-high.out";
+ } > "$work/fy-memuse-high-half-nonzero.diag"
+ not_ok fy-memuse-high-half-nonzero "$work/fy-memuse-high-half-nonzero.diag"
+ fi
+elif grep -q 'link_script.c' "$work/fy-memuse-high.err"; then
+ skip_test fy-memuse-high-half-nonzero \
+ "linker-script hex parser traps on 16-digit ORIGIN (pre-existing UBSan in link_script.c)"
+else
+ not_ok fy-memuse-high-half-nonzero "$work/fy-memuse-high.err"
+fi
+
+# ---- Bug 9: --cref must list imported (DSO-resolved) symbols and their
+# referencing files. Two PIC objects reference libc_marker, resolved from the
+# hand-built libc.so DSO fixture; libc_marker must appear in the cref table.
+if [ -f "$repo_root/test/driver/fixtures/libc.so" ]; then
+ cp "$repo_root/test/driver/fixtures/libc.so" fy_libc.so
+ printf 'extern int libc_marker(void);\nint fy_use_a(void){return libc_marker();}\nvoid _start(void){fy_use_a();for(;;){}}\n' > fy_crefa.c
+ printf 'extern int libc_marker(void);\nint fy_use_b(void){return libc_marker()+1;}\n' > fy_crefb.c
+ run_ok fy-cref-obja "$KIT" build-obj -target x86_64-linux -fPIC fy_crefa.c -o fy_crefa.o
+ run_ok fy-cref-objb "$KIT" build-obj -target x86_64-linux -fPIC fy_crefb.c -o fy_crefb.o
+ run_ok fy-cref-link "$KIT" ld -pie --cref fy_cref.out \
+ fy_crefa.o fy_crefb.o fy_libc.so -o fy_crefexe.elf
+ contains fy-cref-imported-symbol fy_cref.out "libc_marker"
+ # The imported symbol's def line should name the providing DSO (basename),
+ # never "-" and never an absolute host path.
+ if awk '/^libc_marker$/{f=1;next} f&&/^ def fy_libc.so$/{ok=1} f&&/^[^ ]/&&!/^libc_marker$/{f=0} END{exit !ok}' fy_cref.out; then
+ ok fy-cref-imported-def-dso
+ else
+ cp fy_cref.out "$work/fy-cref-imported-def-dso.diag"
+ not_ok fy-cref-imported-def-dso "$work/fy-cref-imported-def-dso.diag"
+ fi
+else
+ skip_test fy-cref-imported-symbol "no libc.so fixture"
+ skip_test fy-cref-imported-def-dso "no libc.so fixture"
+fi
kit_summary build-driver
kit_exit
