amd64: fix GOT-indirect addressing bug in dictionary fast-path lookup
vm_find_word() and dict_find_word_heat_aware() reference the same extern globals (sf_fc_list/sf_fc_count/sf_fc_cap) but GCC compiled cross-TU references to them with GOT-indirect addressing (R_X86_64_REX_GOTPCRELX) under -fPIC. This freestanding, statically-linked UEFI PE image has no dynamic linker to populate a GOT, so those reads silently returned NULL instead of the array's real address -- amd64-only, and exquisitely sensitive to unrelated code-size changes since the choice between direct and GOT-indirect addressing is a per-call-site GCC heuristic. Fix: -fno-pic -fno-pie for amd64 only (ARCH_CFLAGS, overriding COMMON_CFLAGS's -fPIC, which riscv64's -shared loader link still needs). Also removes -DPLATFORM_TIME_NO_INLINE, a prior one-off workaround for the identical bug applied to sf_monotonic_ns() specifically, now redundant. Adds R_X86_64_PC32/R_X86_64_PLT32 handling to elf_apply_relocations() as a robustness fix for the non-monolithic split-build path (dead code for the current monolithic boot, where OVMF's own PE loader relocates the image, not this loader). Verified: all three architectures boot clean and pass the full item-4.2 Hermes self-test, including MSG-DELIVER-ALL, which previously triggered the corruption on amd64 only. Write-up in FABRIC.md under item 4.2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
56ad128e2e
commit
0a7f144367
@@ -3424,6 +3424,74 @@ document and committing that amendment as its own item.*
|
||||
> boundaries, so this may no longer be blocked the way G8 describes. Raise during
|
||||
> implementation; do not resolve by assumption.
|
||||
>
|
||||
> **Bug found and fixed during implementation, 2026-08-06 — amd64-only dictionary
|
||||
> corruption, root cause was a missing `-fno-pic`, not Stadium logic.** While exercising
|
||||
> Hermes's migrated words in this item's self-test (`kernel_main.c`), `MSG-COOL-ALL`
|
||||
> became unreachable via `vm_find_word()` immediately after `MSG-DELIVER-ALL` ran —
|
||||
> amd64 only; aarch64 and riscv64 never showed it. Initial hypotheses (capsule-loader
|
||||
> forward-reference retry interaction, GDB-perturbed timing, arena exhaustion, a stray
|
||||
> `dict_reorganize_buckets_by_heat()` race) were each tested and ruled out by direct
|
||||
> print-based bisection (GDB is unusable on this kernel — see below). Root cause,
|
||||
> confirmed by disassembling the actual booted `starkernel_loader.efi`:
|
||||
> `dict_find_word_heat_aware()` (`dictionary_heat_optimization.c`) and `vm_find_word()`
|
||||
> (`dictionary_management.c`) both reference the same extern globals
|
||||
> (`sf_fc_list`/`sf_fc_count`/`sf_fc_cap`, the dictionary's first-character lookup index),
|
||||
> but GCC compiled the two files' references differently: `vm_find_word()`, in the same
|
||||
> translation unit as the arrays' definition, got direct `lea sym(%rip), %reg` addressing;
|
||||
> `dict_find_word_heat_aware()`, a genuine cross-TU extern reference, got GOT-indirect
|
||||
> `mov sym@GOTPCREL(%rip), %reg` addressing (`R_X86_64_REX_GOTPCRELX`). The latter requires
|
||||
> a populated Global Offset Table slot — normally a dynamic linker's job. This kernel is a
|
||||
> freestanding, statically-linked UEFI PE image with no dynamic linker and no `.got`
|
||||
> section; the "GOT slot" GCC emitted the reference against is just an ordinary
|
||||
> zero-initialized `.bss` cell that nothing ever writes. The load silently returns NULL
|
||||
> instead of the array's real address, `vm_find_word()`'s `!bucket || n==0` guard reads it
|
||||
> as "empty," and the word reports `UNKNOWN WORD` even though its `DictEntry` is fully
|
||||
> intact (verified by a manual `vm->latest`→`link` chain walk). Whether a given reference
|
||||
> gets the safe or unsafe addressing mode is a per-call-site GCC codegen heuristic
|
||||
> sensitive to surrounding code size — which is why the symptom appeared and disappeared
|
||||
> across unrelated one-line changes (even hitting an unrelated symbol, `BIRTH`'s own
|
||||
> dictionary entry, once), and why it looked for a long time like a timing-sensitive
|
||||
> memory-corruption bug rather than a static codegen/build-flag one.
|
||||
>
|
||||
> **Fix:** `Makefile.starkernel`'s amd64 `ARCH_CFLAGS` now appends `-fno-pic -fno-pie`,
|
||||
> overriding `COMMON_CFLAGS`'s `-fPIC` for amd64 only (GCC takes the last flag on the
|
||||
> command line; `ARCH_CFLAGS` is appended after `-fPIC` in `COMMON_CFLAGS`'s definition).
|
||||
> amd64 is a fixed-base, statically-linked image with no dynamic-linker use for PIC in the
|
||||
> first place, so this is a correctness fix, not a workaround. aarch64/riscv64 keep
|
||||
> `-fPIC` — riscv64's loader link step (`ld -shared -Bsymbolic`) genuinely requires it and
|
||||
> fails to link without it; aarch64 was never observed to hit this bug (different
|
||||
> toolchain, `clang`+`lld-link`, different codegen heuristics). Also removed
|
||||
> `-DPLATFORM_TIME_NO_INLINE` from `COMMON_CFLAGS` (and its now-redundant explanatory
|
||||
> comment) — a prior one-off workaround for the identical bug class, applied specifically
|
||||
> to `sf_monotonic_ns()`'s access to `sf_time_backend`, made unnecessary once amd64 got
|
||||
> the real fix. Confirmed no regression on any architecture: all three still boot to
|
||||
> `ok>` and pass the full self-test with the flag removed. `shim.c`/
|
||||
> `physics_hotwords_cache.c`'s own local `#define PLATFORM_TIME_NO_INLINE` (their concrete,
|
||||
> non-inline implementations of `sf_monotonic_ns()` etc.) were left as-is — out of scope
|
||||
> for this fix, and harmless either way.
|
||||
>
|
||||
> **Also fixed as a side effect, kept though not the active bug:** `elf_apply_relocations()`
|
||||
> (`src/starkernel/boot/elf_loader.c`) didn't handle `R_X86_64_PC32`/`R_X86_64_PLT32`
|
||||
> either, discovered while chasing an earlier (wrong) theory that this was a runtime ELF
|
||||
> relocation bug. That code path turned out to be dead for this build — `uefi_loader.c`
|
||||
> calls `kernel_main()` as a direct function call under `MONOLITHIC_BUILD` (the default
|
||||
> here), never invoking `elf_load_kernel()`/`elf_apply_relocations()` at all; the actual
|
||||
> boot image is a standard PE32+ UEFI application, relocated by OVMF's own PE loader, not
|
||||
> by this custom ELF loader. The relocation-type gap is real for the non-monolithic
|
||||
> split-build path though (`elf_load_kernel()` returns 0 — hard failure — on any
|
||||
> unhandled type, and the loop aborts the rest of that RELA section on the first one hit),
|
||||
> so the handling was kept as a legitimate robustness fix rather than reverted.
|
||||
>
|
||||
> **Process note:** GDB+QEMU is confirmed unusable for debugging this kernel — the custom
|
||||
> UEFI loader relocates/loads the image such that static-symbol software breakpoints never
|
||||
> fire, and a hardware breakpoint (`hbreak`) not only never fired but its mere presence
|
||||
> caused a different, more severe corruption (`BIRTH` itself became `UNKNOWN WORD`) before
|
||||
> any breakpoint triggered — likely a parity/dict-hash boot-gate reacting to the debugger
|
||||
> session, not "timing perturbation" as first guessed. Print-based bisection
|
||||
> (`console_puts`/`print_uint`, plus `log_message(LOG_ERROR, ...)` — `LOG_INFO` is below
|
||||
> the active log threshold and never appears in the serial log, a separate dead end closed
|
||||
> along the way) is the only viable method for this kernel today.
|
||||
>
|
||||
> *Done when:*
|
||||
> - The seven `STADIUM-*` FORTH primitives exist, are kernel-only (not in the shared/
|
||||
> vendored word set), and are exercised by at least one Hermes word each.
|
||||
|
||||
+21
-7
@@ -121,7 +121,18 @@ ifeq ($(ARCH),amd64)
|
||||
LD := ld
|
||||
OBJCOPY := objcopy
|
||||
endif
|
||||
ARCH_CFLAGS := -m64 -march=x86-64 -mno-red-zone -DARCH_AMD64
|
||||
# -fno-pic -fno-pie: this is a fixed-base, statically-linked freestanding
|
||||
# image with no dynamic linker to populate a GOT at load time. -fPIC (set
|
||||
# in COMMON_CFLAGS, needed by riscv64's -shared link pipeline) makes GCC
|
||||
# emit R_X86_64_REX_GOTPCRELX (GOT-indirect) addressing for some cross-TU
|
||||
# extern globals depending on per-call-site codegen heuristics; the "GOT
|
||||
# slot" is just an unpopulated .bss cell here, so those reads silently
|
||||
# return NULL instead of the real address. Root-caused 2026-08-06 via
|
||||
# dict_find_word_heat_aware() reading sf_fc_count as NULL while
|
||||
# vm_find_word() (same globals, same TU as the definition) read it
|
||||
# correctly. These flags come after COMMON_CFLAGS on the command line so
|
||||
# they win for amd64 only; riscv64/aarch64 keep -fPIC.
|
||||
ARCH_CFLAGS := -m64 -march=x86-64 -mno-red-zone -DARCH_AMD64 -fno-pic -fno-pie
|
||||
LOADER_LINKER_SCRIPT := linker/starkernel-loader-amd64.ld
|
||||
KERNEL_LINKER_SCRIPT := linker/starkernel-kernel-amd64.ld
|
||||
|
||||
@@ -173,10 +184,14 @@ LOADER_LD ?= $(LD)
|
||||
# ==============================================================================
|
||||
|
||||
# Common flags for all arches / both loader and kernel.
|
||||
# -fPIC + -fvisibility=hidden: keeps all symbols local, prevents GOT references
|
||||
# for internal function pointers (PE has no GOT).
|
||||
# -DPLATFORM_TIME_NO_INLINE: routes sf_monotonic_ns() through shim wrappers to
|
||||
# avoid GOTPCREL relocations that crash at runtime in the PE environment.
|
||||
# -fPIC + -fvisibility=hidden: needed by riscv64's -shared loader link
|
||||
# pipeline (amd64 overrides back to -fno-pic/-fno-pie in its ARCH_CFLAGS
|
||||
# above -- see that comment for why: -fPIC made GCC emit GOT-indirect
|
||||
# addressing for some cross-TU extern globals, and this freestanding image
|
||||
# has no dynamic linker to populate a GOT, so those reads silently returned
|
||||
# NULL. PLATFORM_TIME_NO_INLINE (removed 2026-08-06) was a symbol-specific
|
||||
# workaround for the same underlying bug, made unnecessary once amd64 got
|
||||
# the real fix; see FABRIC.md for the write-up).
|
||||
COMMON_CFLAGS := \
|
||||
-std=c99 -Wall -Werror -Wextra \
|
||||
-ffreestanding -nostdlib -fno-builtin \
|
||||
@@ -184,8 +199,7 @@ COMMON_CFLAGS := \
|
||||
$(ARCH_CFLAGS) \
|
||||
-I$(KERNEL_INC) -Iinclude -Isrc \
|
||||
-include $(STARFORTH_CONFIG_HEADER) \
|
||||
-DPARITY_MODE=$(PARITY_MODE) \
|
||||
-DPLATFORM_TIME_NO_INLINE
|
||||
-DPARITY_MODE=$(PARITY_MODE)
|
||||
|
||||
VMCORE_CFLAGS_COMMON := \
|
||||
$(filter-out -I$(KERNEL_INC),$(COMMON_CFLAGS)) \
|
||||
|
||||
@@ -131,6 +131,7 @@
|
||||
#define R_X86_64_RELATIVE 8
|
||||
#define R_X86_64_32 10
|
||||
#define R_X86_64_32S 11
|
||||
#define R_X86_64_PLT32 4
|
||||
|
||||
/* Relocation types (aarch64) */
|
||||
#define R_AARCH64_NONE 0
|
||||
|
||||
@@ -226,6 +226,10 @@ static int elf_load_segments(const uint8_t *elf_data, Elf64_Addr load_base)
|
||||
* - @c R_X86_64_RELATIVE — absolute address = @c load_base + @c r_addend.
|
||||
* - @c R_X86_64_64 — @c load_base + symbol value + @c r_addend (64-bit).
|
||||
* - @c R_X86_64_32 / @c R_X86_64_32S — same, truncated to 32 bits.
|
||||
* - @c R_X86_64_PC32 / @c R_X86_64_PLT32 — @c symbol + @c r_addend - P
|
||||
* (PC-relative, 32 bits), where P is the relocation's own runtime
|
||||
* address; PLT32 has no real PLT indirection in this static, non-PIE
|
||||
* link, so it resolves the same as PC32.
|
||||
*
|
||||
* - **aarch64 (@c ARCH_AARCH64)**:
|
||||
* - @c R_AARCH64_NONE — no-op.
|
||||
@@ -301,6 +305,19 @@ static int elf_apply_relocations(const uint8_t *elf_data, Elf64_Addr load_base)
|
||||
*target32 = (uint32_t)value;
|
||||
break;
|
||||
}
|
||||
case R_X86_64_PC32:
|
||||
case R_X86_64_PLT32: {
|
||||
/* value = S + A - P, where P is the address of the relocation
|
||||
* itself (reloc_addr already includes load_base). PLT32 is
|
||||
* treated identically to PC32: this is a static, non-PIE
|
||||
* link with every symbol resolved in-image, so there is no
|
||||
* real PLT indirection to apply. */
|
||||
uint32_t *target32 = (uint32_t *)reloc_addr;
|
||||
int64_t value = (int64_t)(load_base + sym_value + rela[j].r_addend) -
|
||||
(int64_t)reloc_addr;
|
||||
*target32 = (uint32_t)value;
|
||||
break;
|
||||
}
|
||||
default:
|
||||
return 0;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user