[AUTOMATED] feat(p1): msvcfpconst — recover MSVC __real@ FP-constant COMDATs from their mangled names (DIV-96)

[mahaloz]

Implements the proposal. The design question it was held on has dissolved: PR #328 (dynrelocs) shipped the exact mechanism route (3) needed, so Architecture::dynreloc_const is now a live "these ranges fold even with readonlypropagate off" exception list, plumbed loader -> engine.rs -> build_arch_handle -> the dynreloc_fold branch in coreaction_early.rs. Route (3) is therefore "push onto a Vec that already exists", not the ArchSeam project the proposal costed. readonlypropagate, AGGRESSIVE_OVERRIDES and apply_mode are untouched -- that is a separate PR and a separate decision.

What ships

--option msvcfpconst on|off, default on (DIV-90), loader-tier through the established env bridge (KUNA_MSVCFPCONST).

MSVC never encodes a floating-point literal into the instruction stream -- x87 and SSE both load one from memory -- so it emits each literal as a COMDAT whose name spells the value. COMDAT folding keeps the definition in exactly one translation unit, so in every other object that symbol is undefined: no section, no bytes, and the loader binds it to a synthetic extern slot with nothing behind it. The defined half is no better at the shipped defaults: its bytes are mapped and read-only, but folding a read-only global is gated by the program-wide option readonly, default-off.

On, the value is decoded from the name, the undefined half's bytes are materialised at its extern slot, and both halves' ranges are pushed onto ObjectLoadImage::dynreloc_const. Both are required: listing only one leaves an expression with a literal on one side and dat_<addr> on the other, which reads worse than either extreme.

Three decoder defects fixed (all were inert -- nothing called decode())

(a) Modern MSVC float spelling was dropped. Today's MSVC emits __real@3f800000 for 1.0f -- 8 hex digits of raw IEEE float bits. The bare-form arm required exactly 16, so every float literal from a post-VS2005 object was refused. Now accepted as f32 bits. (Also corrected in the prose: __real@<16 hex> is the newer form, not "the older form" -- the 20-hex x87 form is the VC6-era one, which is what a 1990s bounce.obj is full of.)

(b) Two encoding classes returned wrong values. exp == 0 with mantissa != 0 was off by 2x (the true x87 subnormal scale is 2^(-16382-63), the code used -16383-63), and exp == 0x7fff (Inf/NaN) silently decoded to +inf -- both violating the module's own "refuse everything that is not this mangling" contract. Both are now refused, along with unnormals (integer bit clear) and any value that leaves f64 range, or float range at @4@. A saturated 0.0 from an underflowing powi is refused too: a wrong datum is worse than an opaque address.

(c) Two PR-body labels and one test literal were wrong. Re-derived with exact rational arithmetic (Fraction, not float):

• 3fe08000000200000000 is not 2^-31. The mantissa is 0x8000000002000000 = 2^63 + 2^25, so the value is 2^-31 + 2^-61 = 4.656612877414201e-10.
• 3fef8000800000000000 is not 2^-16. The mantissa is 0x8000800000000000 = 2^63 + 2^47, so the value is 2^-16 + 2^-32 = 1.5259021893143654e-05.
• The @4@ test's expected literal 1.5259021896696368e-05 was wrong in the 10th significant digit and passed only because the tolerance was 1e-11 absolute against a 1.5e-05 value -- roughly six orders of slack. The tests now compare with ==: every one of these decodes is bit-exact (a double widened to the x87 format leaves at most 53 significant mantissa bits, so (mantissa as f64) * 2^k is lossless), and a tolerance would only hide a regression.

The full decode table, re-derived

Exact rational arithmetic, then rounded to the storage width. The first twelve are the constants in the oracle object; the last four are the modern spellings.

symbol	form	width	decodes to	what it is
__real@8@3fff8000000000000000	x87 80-bit	8	1.0	1.0
__real@8@4000a000000000000000	x87 80-bit	8	2.5	2.5
__real@8@4000c000000000000000	x87 80-bit	8	3.0	3.0
__real@8@00000000000000000000	x87 80-bit	8	0.0	0.0
__real@8@3ffec90fdaa22168c000	x87 80-bit	8	0.7853981633974483	pi/4
__real@8@4001c90fdaa22168c000	x87 80-bit	8	6.283185307179586	2*pi
__real@8@3fe08000000200000000	x87 80-bit	8	4.656612877414201e-10	2^-31 + 2^-61 (the RNG scale; not 2^-31)
__real@8@bff39d495182a9930800	x87 80-bit	8	-0.0003	-0.0003
__real@8@bff2d1b71758e2196800	x87 80-bit	8	-0.0002	-0.0002
__real@4@3fef8000800000000000	x87 80-bit	4	1.5259021893143654e-05	2^-16 + 2^-32 (not 2^-16)
__real@4@40069600000000000000	x87 80-bit	4	150.0	150.0
__real@4@3fff8000000000000000	x87 80-bit	4	1.0	1.0f
__real@3ff0000000000000	IEEE bits, 16 hex	8	1.0	1.0
__real@3fb999999999999a	IEEE bits, 16 hex	8	0.1	0.1
__real@3f800000	IEEE bits, 8 hex	4	1.0	1.0f -- was refused
__real@3f000000	IEEE bits, 8 hex	4	0.5	0.5f -- was refused

Before/after on the oracle

bounce.obj (i386 MSVC, Command & Conquer): 12 __real@ symbols, 7 defined and 5 undefined. Every one is a recognisable literal from the original source, which is what makes it a sound oracle. kuna decompile-all, --option msvcfpconst off vs the default:

// BounceClass::Init -- the expression the proposal quoted
- v9 = ((a6 - a5) * (double)(int)Random2Class::operator()(0x7ffffffe00000000) * dat_402020 + a5) * dat_400af0;
+ v9 = ((a6 - a5) * (double)(int)Random2Class::operator()(0x7ffffffe00000000) * 4.656612877414201e-10 + a5) * 3.0;
- v4 = (double)v1 * dat_402020 * dat_402030;
+ v4 = (double)v1 * 4.656612877414201e-10 * 6.283185307179586;
- v7 = ((double)(int)Random2Class::operator()(0,0x7ffffffe) * dat_402020 + dat_402040) * dat_400ae0;
+ v5 = ((double)(int)Random2Class::operator()(0,0x7ffffffe) * 4.656612877414201e-10 + 1.0) * 0.7853981633974483;

That last line is a 31-bit random scaled into [0,1), biased by 1, times pi/4 -- readable as an angle jitter. Every other affected expression in the object:

// BounceClass::Get_Bounce_Coord
- v2 = (float)v10 * dat_400b00;      + v2 = (float)v10 * 1.5259022e-05;
- v6 = (float)a2  * dat_400b00;      + v6 = (float)a2  * 1.5259022e-05;
- v7 = (float)v11 * dat_400b00;      + v7 = (float)v11 * 1.5259022e-05;
- if (v4 != (long double)dat_402080) {
+ if (v4 != (long double)0.0) {

// BounceClass::AI
- if ((dat_402080 < v2) && ((long double)v2 < (long double)Vector3::Length())) {
+ if ((0.0 < v2)        && ((long double)v2 < (long double)Vector3::Length())) {
- ... && (*(float *)&a0[4] - dat_400b30 < (float)v11)) {
+ ... && (*(float *)&a0[4] - 150.0      < (float)v11)) {
- ... (((v47 < (float)dat_400b20 && (0.0 < (float)v7[8])) || (((float)dat_400b10 <= v47 && (v47 + 0.0 + dat_4021d0 < (float)v7[8]))))))
+ ... (((v46 < -0.0002          && (0.0 < (float)v7[8])) || ((-0.0003           <= v46 && (v46 + 0.0 + 1.0          < (float)v7[8]))))))

// BounceClass::Get_Remaining_Motion
- if (v33 < (long double)dat_400ad0)
+ if ((long double)BounceClass::Get_Remaining_Motion() < (long double)2.5)

Whole-object dat_ references 14 -> 3. The three that remain are all dat_402120, which is ?BRIDGE_LEPTON_HEIGHT@@3HB -- an integer extern, correctly untouched.

combat.obj (the sibling object): dat_ 99 -> 75, recovering 0.01, 100.0, 10.0, 4.0, 0.3, and the pair below -- pi/2 and -32768/pi, the radians-to-binary-angle conversion:

- v1 = (long long)((v18 - dat_404460) * dat_404470);
+ v1 = (long long)((v18 - 1.5707963267948966) * -10430.060040584269);

Collateral

kuna decompile-all --json, off vs on, over every non-MSVC binary to hand -- three linked x86-64 ELFs, an ARM Cortex-M firmware, a shared object, a Mach-O .o and two ELF ET_REL .os:

binary	lines	changed
grep	17,975	0
sort	11,692	0
libselinux.so.1	17,143	0
faillog	1,117	0
betaflight_STM32F405.elf	132,874	0
/usr/bin/ls	14,327	0
ptx.o (ELF ET_REL)	2,057	0
fid/lib.o (ELF ET_REL)	57	0
macho_dwarf.o	17	0

Byte-identical, as expected: the pass runs only inside from_relocatable and only fires on a symbol literally named __real@, which only an MSVC-ABI compiler emits. Stated honestly, this is a gate that makes the result true by construction rather than an independent finding -- the two ELF .os are the meaningful arm, since they do take the relocatable path and do have synthetic extern slots, and nothing changed there either.

The off arm is byte-identical to the pre-change binary on bounce.obj (diff clean over all 438 lines), so the flag genuinely restores the old behaviour rather than approximating it.

Speed

kuna decompile-all, interleaved min-of-4 with the arms alternating inside each round. This box runs several other implementation agents concurrently (8-10 cargo test processes were live during the run), so a one-shot before/after would be measuring contention; min-of-N over interleaved rounds is what survives that. Budget is 5%.

binary	off (min-of-4)	on (min-of-4)	delta
bounce.obj (MSVC, 12 constants)	0.2857 s	0.2680 s	-6.21%
combat.obj (MSVC, 24 constants)	2.0027 s	1.9835 s	-0.96%
msvcfpconst_i386.obj (fixture)	0.0972 s	0.0929 s	-4.45%
grep (no __real@ symbols)	14.912 s	14.953 s	+0.27%

Inside budget in both directions. The affected objects come out faster: a folded constant collapses dataflow that otherwise has to be carried through the whole function -- the same effect measured when readonly is turned on. grep is noise; the pass does not run on a linked image at all.

Tests

• tests/stages/kuna-msvcfpconst.xml, two-pass, 6 assertions. A loader-tier gate's off-arm is reachable in stage XML after all: the <binaryimage> harness leaves a live architecture, so option msvcfpconst off before clear architecture/load file sets the env var the next load reads. (That is why pass 2 names the option explicitly -- pass 1 leaves it set in the same process. The fix(analysis): C++ anonymous namespaces stop aborting the load, and symbolnamerepair backstops the rest (DIV-88) #335 symbolnamerepair PR could not do this for a different reason: its off-arm makes the load fail outright, which errors the whole script out.)
• decompiler/crates/kuna-console/tests/verify_msvcfpconst.rs, 4 tests: the shipped default with nothing set anywhere emits literals; both halves are reported foldable and off reports none; the materialised slots read back through load_fill as their decoded values; and the decoder agrees with a real MSVC-ABI object's own defined bytes.
• 6 decoder unit tests, including the refusal classes.
• New fixture msvcfpconst_i386.{c,obj} (clang --target=i686-pc-windows-msvc -O1, build line in the .c header): all four spellings in one object -- two defined (the bare-bits pair a modern compiler emits) and two undefined externs (the VC6-era x87 pair), which is the state COMDAT folding produces.

Gates

make test          675/675 assertions passed -- PARITY OK   (docs/baseline.json UNTOUCHED)
make test-stages   544/544 assertions passed -- PARITY OK   (re-recorded: +6 new keys, additive only)
make rust-test     324 result blocks, 6,893 passed, 0 failed, 38 ignored  (*)
make check-spec    check-spec OK (lenient mode) / check-spec OK (strict mode)
kuna catalog --check   catalog OK: documents exactly the registered kuna options

(*) The workspace suite was run in three commands rather than one: this box's background-command watchdog killed the single cargo test --workspace twice at ~40 min, mid-run and after a completed target both times. The three runs together cover every workspace member (ws.log: kuna-analysis / kuna-base / kuna-cli / kuna-console + the head of kuna-decomp; ws2.log: kuna-num / kuna-sleigh / kuna-slacomp / kuna-harness / kuna-lift-diff / kuna-ghidra / kuna-wasm; ws3.log: all of kuna-decomp), with a small overlap on kuna-decomp's first two integration tests. docs/options.md was separately re-diffed against a fresh kuna catalog --markdown and is byte-identical.

Hard-coded counts bumped

where	old -> new
kuna_phases/tests.rs kuna_num_settables() / SETTABLE_TABLE.len()	119 -> 120
kuna_phases/tests.rs tier counts (analysis)	(26,50,43) -> (26,50,44)
kuna_phases/tests.rs catalog JSON row commas	118 -> 119
kuna_phases/tests.rs PASS_GATES	+ "msvcfpconst" (no codegen live reader; console-side gate)
catalog_bytecompat.rs "option": / "tier": / "symptoms":	119 -> 120 (fixture regenerated)
kuna-base/src/xml.rs corpus file count	215 -> 216

tests/stages/kuna-catalog.xml needed no change: its counted rows are source_decompiler: angr and the change_kind buckets, and this option is kuna / correctness-fix.

Not in this PR

The scout's readonly measurement corrects this PR's own: it tested fmt_aarch64 (0 changed lines) and concluded ARM is unaffected. On betaflight_STM32F405.elf, --option readonly on changes 86,927 of 138,669 lines (63%) and drops dat_ references 21% (27,923 -> 22,043), because every Cortex-M FP/mask constant lives in a PC-relative flash literal pool. That belongs to the separate readonly PR, along with the apply_mode dual-dispatch route (option (1) is blocked as described, but IfcOption and apply_runtime_options are both already dual-dispatch, so making apply_mode match is ~10 lines and needs no KUNA_OPTION_NAMES change).

Also unaddressed here: an ET_REL read-only section holds pre-relocation bytes, so a program-wide readonly fold on a .o yields wrong constants. This PR is immune -- a __real@ COMDAT carries no relocation and its bytes are cross-checked against its name -- but the readonly PR must handle it.

DIV number: DIV-90. Highest on main is DIV-89; siblings may claim 90..96, so the row in docs/history.md and its 10 in-tree references may need renumbering at merge: grep -rn 'DIV-90' finds them in docs/history.md (1), architecture.rs (2), kuna_phases/tests.rs (3), options.rs (1), decompile.rs (1), decompile_all.rs (1), and the two kuna_msvcfpconst.rs module docs (1 each).

Merge note: this PR adds a tests/stages/*.xml, so it bumps the corpus file count in kuna-base/src/xml.rs (215 -> 216) and re-records docs/baseline-stages.json. Several siblings are doing the same: resolve the count to base + all merged, and re-record the baseline rather than hand-merging it.

Closes the proposal.

🤖 Generated with Claude Code

https://claude.ai/code/session_011KpG7qK6BDFZyPnyo4r1c1