kit

kit
git clone https://git.ryansepassi.com/git/kit.git
Log | Files | Refs | README

commit d21ba7ec4b06541644f97ea0956b9cb355c10772
parent 01dc12b56bc1ff4d3630b4330ebd89d87e81a2dd
Author: Ryan Sepassi <rsepassi@gmail.com>
Date:   Fri, 12 Jun 2026 12:18:24 -0700

fix(opt): keep bitfield-store address live across value materialization

A C bitfield assignment lowers to a read-modify-write: load the storage
word at the field's address, clear the field bits, OR in the new value,
store back. The destination address must stay live across the sequence.

The -O1 native emitter (pass_native_emit IR_BITFIELD_STORE) materialized
the value without telling the allocator to avoid the address's base/index
registers — it passed REG_NONE/REG_NONE. Under register pressure the
struct pointer spilled and reloaded into scratch[0]; the value-0
materialization then also chose scratch[0], emitting `mov w9,#0` over the
just-reloaded address in x9 and nulling it. The next `ldr w16,[x9]`
dereferenced 0 — a SIGSEGV in optimized SQLite (ecosystem Bug C), which
had been masked until the -O1 link bug (Bug B) was fixed.

Materialize the value avoiding the address base/index regs, and collapse
the address to a single scratch first when no other scratch is free —
exactly what IR_STORE and IR_AGG_SET already do.

Regression: test/asm/roundtrip/bitfield_store_spill.c (24 live struct
pointers force the store's pointer to spill); crashed at -O1 before,
green at -O0 and -O1 after. SQLite 3.50.2 at -O1 now runs and matches
clang byte-for-byte; ecosystem gate green at both opt levels.

Diffstat:
Msrc/opt/pass_native_emit.c | 17+++++++++++++++--
Atest/asm/roundtrip/bitfield_store_spill.c | 19+++++++++++++++++++
Atest/asm/roundtrip/bitfield_store_spill.expected | 1+
Mtest/ecosystem/known_bugs/README.md | 14+++++---------
Mtest/ecosystem/known_bugs/sqlite-o1-runtime-segfault.md | 40+++++++++++++++++++++++++++++++++++-----
Mtest/ecosystem/recipes/sqlite.recipe | 13++++++-------
6 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/src/opt/pass_native_emit.c b/src/opt/pass_native_emit.c @@ -915,9 +915,22 @@ static void emit_inst(NativeEmitCtx* e, u32 block, u32 order_index, Inst* in, IRBitFieldAux* aux = (IRBitFieldAux*)in->extra.aux; addr = addr_from_operand(e, &in->opnds[0], in->loc); src = loc_from_operand(e, &in->opnds[1], in->loc); - if (src.kind != NATIVE_LOC_REG) + /* The field store is a read-modify-write that keeps the address live + * across the value placement, so the value must not be materialized into + * the address's base/index register. Mirror IR_STORE/IR_AGG_SET: when no + * other scratch is free, collapse the address to a single scratch first, + * then materialize the value avoiding the address regs. Without this, a + * spilled pointer reloaded into scratch[0] and a value reusing scratch[0] + * collide — the value `mov`s over the address, nulling it (SIGSEGV in + * optimized SQLite). */ + if (src.kind != NATIVE_LOC_REG) { + if (!scratch_available(e, class_for_type(e, in->opnds[1].type), + addr_base_reg(&addr), addr_index_reg(&addr))) + collapse_addr_to_reg(e, &addr, in->loc); src = materialize(e, src, class_for_type(e, in->opnds[1].type), - in->opnds[1].type, REG_NONE, REG_NONE, in->loc); + in->opnds[1].type, addr_base_reg(&addr), + addr_index_reg(&addr), in->loc); + } e->target->bitfield_store(e->target, addr, src, aux->access); return; } diff --git a/test/asm/roundtrip/bitfield_store_spill.c b/test/asm/roundtrip/bitfield_store_spill.c @@ -0,0 +1,19 @@ +/* Bitfield store under register pressure. A 2-bit bitfield assignment is a + * read-modify-write that must keep the destination word's address live while + * the new field value is materialized. With enough live struct pointers the + * store's pointer spills and is reloaded into a scratch register; the value + * must NOT be materialized into that same scratch register. The -O1 native + * emitter once placed the value into the address's scratch, nulling the + * pointer (a SIGSEGV in optimized SQLite). Each f0=i%4, f1 must stay intact; + * sum of f0 over 24 structs = (0+1+2+3)*6 = 36. */ +struct S { long pad[3]; unsigned f0 : 2; unsigned f1 : 30; }; + +int test_main(void) { + struct S s[24]; + for (int i = 0; i < 24; i++) { s[i].f0 = 3; s[i].f1 = (unsigned)(0x100 + i); } + struct S *p0 = &s[0],*p1 = &s[1],*p2 = &s[2],*p3 = &s[3],*p4 = &s[4],*p5 = &s[5],*p6 = &s[6],*p7 = &s[7],*p8 = &s[8],*p9 = &s[9],*p10 = &s[10],*p11 = &s[11],*p12 = &s[12],*p13 = &s[13],*p14 = &s[14],*p15 = &s[15],*p16 = &s[16],*p17 = &s[17],*p18 = &s[18],*p19 = &s[19],*p20 = &s[20],*p21 = &s[21],*p22 = &s[22],*p23 = &s[23]; + p0->f0 = 0; p1->f0 = 1; p2->f0 = 2; p3->f0 = 3; p4->f0 = 0; p5->f0 = 1; p6->f0 = 2; p7->f0 = 3; p8->f0 = 0; p9->f0 = 1; p10->f0 = 2; p11->f0 = 3; p12->f0 = 0; p13->f0 = 1; p14->f0 = 2; p15->f0 = 3; p16->f0 = 0; p17->f0 = 1; p18->f0 = 2; p19->f0 = 3; p20->f0 = 0; p21->f0 = 1; p22->f0 = 2; p23->f0 = 3; + int sum = (int)p0->f0 + (int)p1->f0 + (int)p2->f0 + (int)p3->f0 + (int)p4->f0 + (int)p5->f0 + (int)p6->f0 + (int)p7->f0 + (int)p8->f0 + (int)p9->f0 + (int)p10->f0 + (int)p11->f0 + (int)p12->f0 + (int)p13->f0 + (int)p14->f0 + (int)p15->f0 + (int)p16->f0 + (int)p17->f0 + (int)p18->f0 + (int)p19->f0 + (int)p20->f0 + (int)p21->f0 + (int)p22->f0 + (int)p23->f0; + int f1ok = (s[0].f1 == (unsigned)(0x100 + 0)) + (s[1].f1 == (unsigned)(0x100 + 1)) + (s[2].f1 == (unsigned)(0x100 + 2)) + (s[3].f1 == (unsigned)(0x100 + 3)) + (s[4].f1 == (unsigned)(0x100 + 4)) + (s[5].f1 == (unsigned)(0x100 + 5)) + (s[6].f1 == (unsigned)(0x100 + 6)) + (s[7].f1 == (unsigned)(0x100 + 7)) + (s[8].f1 == (unsigned)(0x100 + 8)) + (s[9].f1 == (unsigned)(0x100 + 9)) + (s[10].f1 == (unsigned)(0x100 + 10)) + (s[11].f1 == (unsigned)(0x100 + 11)) + (s[12].f1 == (unsigned)(0x100 + 12)) + (s[13].f1 == (unsigned)(0x100 + 13)) + (s[14].f1 == (unsigned)(0x100 + 14)) + (s[15].f1 == (unsigned)(0x100 + 15)) + (s[16].f1 == (unsigned)(0x100 + 16)) + (s[17].f1 == (unsigned)(0x100 + 17)) + (s[18].f1 == (unsigned)(0x100 + 18)) + (s[19].f1 == (unsigned)(0x100 + 19)) + (s[20].f1 == (unsigned)(0x100 + 20)) + (s[21].f1 == (unsigned)(0x100 + 21)) + (s[22].f1 == (unsigned)(0x100 + 22)) + (s[23].f1 == (unsigned)(0x100 + 23)); + return sum + (f1ok == 24 ? 0 : 100); +} diff --git a/test/asm/roundtrip/bitfield_store_spill.expected b/test/asm/roundtrip/bitfield_store_spill.expected @@ -0,0 +1 @@ +36 diff --git a/test/ecosystem/known_bugs/README.md b/test/ecosystem/known_bugs/README.md @@ -15,16 +15,11 @@ When the gate fails, the failing build's artifacts are preserved under ## Open bugs -| id | file | trigger | symptom | -|----|------|---------|---------| -| C | [sqlite-o1-runtime-segfault.md](sqlite-o1-runtime-segfault.md) | SQLite shell at **-O1** | runs then SIGSEGVs (null-pointer deref); -O1 optimizer miscompile, surfaced once Bug B unblocked the -O1 link | +None. All seven projects — cJSON, LZ4, miniz, tinyexpr, Lua, yyjson, **and +SQLite** — are green at both -O0 and -O1 and match clang byte-for-byte. -SQLite is green at -O0 (matches clang byte-for-byte) and links at -O1 but -crashes at runtime (Bug C). The other six projects — cJSON, LZ4, miniz, -tinyexpr, Lua, **and yyjson** — are green at both -O0 and -O1 and match clang. - -Each bug doc has a self-contained reproduction against the provisioned cache -(`make provision-ecosystem` first). +Each fixed-bug doc keeps a self-contained reproduction against the provisioned +cache (`make provision-ecosystem` first). ## Fixed bugs @@ -32,3 +27,4 @@ Each bug doc has a self-contained reproduction against the provisioned cache |----|------|-----|----------| | A | [yyjson-write-label.md](yyjson-write-label.md) | yyjson.c at -O0 → `MCEmitter: label NNNN placed twice` | front end: a goto label first seen inside a constant-false (codegen-suppressed) region now gets a real CG-label id | | B | [sqlite-o1-lkit-ro.md](sqlite-o1-lkit-ro.md) | `kit cc src.c archive.a` at -O1 → `undefined reference to '.Lkit_ro.N'` | linker: one-shot in-memory link now honors `removed` (tombstoned) symbols/relocs, matching a serialized object | +| C | [sqlite-o1-runtime-segfault.md](sqlite-o1-runtime-segfault.md) | SQLite shell at -O1 → runtime SIGSEGV (null-pointer deref), surfaced once Bug B unblocked the -O1 link | -O1 native emit: a bitfield store (read-modify-write) materialized its value into the address's own scratch register; the value now avoids the address base/index regs (mirrors `IR_STORE`/`IR_AGG_SET`) | diff --git a/test/ecosystem/known_bugs/sqlite-o1-runtime-segfault.md b/test/ecosystem/known_bugs/sqlite-o1-runtime-segfault.md @@ -1,9 +1,39 @@ # Bug C — SQLite segfaults at runtime when built at `-O1` -**Status:** open. Kept red by the ecosystem gate (`sqlite:O1:run`). -**Component:** -O1 optimizer/codegen (a null-pointer dereference in optimized -SQLite code; not the linker, not the front end). -**Severity:** the sqlite shell built at -O1 crashes before producing output. +**Status:** FIXED. The ecosystem gate is green for `sqlite:O1` (golden + vs-clang). +**Component:** -O1 native emit (`src/opt/pass_native_emit.c`, +`IR_BITFIELD_STORE`) — a register-allocation clobber, not the linker or front +end. +**Severity (when open):** the sqlite shell built at -O1 crashed before +producing output. + +## Root cause and fix + +A C bitfield assignment `p->field = v` (here a 2-bit field, `v == 0`) lowers to +a read-modify-write: load the storage word at the field's address, clear the +field bits, OR in the new value, store the word back. The address must stay +live across the whole sequence. + +The -O1 native emitter materialized the *value* without telling the allocator +to avoid the *address* registers (it passed `REG_NONE, REG_NONE` as the +registers-to-avoid). Under register pressure SQLite's pointer spilled and was +reloaded into `scratch[0]`; the value-`0` materialization then also picked +`scratch[0]`, emitting `mov w9, #0` over the just-reloaded address in `x9` — +nulling it. The following `ldr w16, [x9]` dereferenced 0. + +``` +ldr x9, [sp, #..] ; reload spilled pointer (address) into scratch x9 +mov w9, #0 ; materialize the value 0 — into the SAME scratch x9 (clobber!) +ldr w16, [x9] ; load storage word from 0 → SIGSEGV +``` + +Fix: `IR_BITFIELD_STORE` now materializes the value avoiding the address +base/index registers, and collapses the address to a single scratch first when +no other scratch is free — exactly what `IR_STORE` and `IR_AGG_SET` already do. +Regression test: `test/asm/roundtrip/bitfield_store_spill.c` (red at -O1 before +the fix: crashed; green after at both -O0 and -O1). + +## How this surfaced ## How this surfaced @@ -52,7 +82,7 @@ DEFS="-DSQLITE_THREADSAFE=0 -DSQLITE_OMIT_LOAD_EXTENSION" $KIT cc -O1 --sysroot "$SDK" $DEFS -I "$SQ" -c "$SQ/sqlite3.c" -o /tmp/sqlite3.o $KIT ar rcs /tmp/libsqlite3.a /tmp/sqlite3.o $KIT cc -O1 --sysroot "$SDK" $DEFS -I "$SQ" "$SQ/shell.c" /tmp/libsqlite3.a -lc -o /tmp/sqlite3 -/tmp/sqlite3 :memory: < test/ecosystem/scripts/sqlite.sql # SIGSEGV +/tmp/sqlite3 :memory: < test/ecosystem/scripts/sqlite.sql # SIGSEGV before the fix; now runs clean ``` ## Notes for digging diff --git a/test/ecosystem/recipes/sqlite.recipe b/test/ecosystem/recipes/sqlite.recipe @@ -33,13 +33,12 @@ ECO_INCLUDES="." ECO_DEFINES="-DSQLITE_THREADSAFE=0 -DSQLITE_OMIT_LOAD_EXTENSION" ECO_LIBS="" ECO_DRIVER=shell.c -# KNOWN-RED at -O1 only (O0 builds, runs, and matches clang). The -O1 link now -# succeeds (the old `.Lkit_ro` one-shot-link bug is fixed — -# known_bugs/sqlite-o1-lkit-ro.md), but the -O1 binary then SIGSEGVs at runtime -# on a null-pointer deref: a separate -O1 optimizer miscompile that the link -# failure used to mask. Kept red on purpose. Details + repro: -# test/ecosystem/known_bugs/sqlite-o1-runtime-segfault.md. -ECO_BUG="known_bugs/sqlite-o1-runtime-segfault.md" +# Green at both -O0 and -O1 (matches clang byte-for-byte). The two historical +# -O1 failures are fixed: the `.Lkit_ro` one-shot-link bug +# (known_bugs/sqlite-o1-lkit-ro.md) and the runtime null-deref it had masked, +# an -O1 native-emit bitfield-store register clobber +# (known_bugs/sqlite-o1-runtime-segfault.md). +ECO_BUG="" eco_lib_srcs() { echo "$ECO_SRC/sqlite3.c"; }