libsqlite3: fix WAL index access crash on Windows (issue #221)
Thanks for the detailed guidance on #221. We took your review feedback fully into account and rebuilt the solution strictly according to the architecture and process you requested:
1. Process and reproducibility
- Patch location: The C changes are isolated cleanly as a unified patch under
libsqlite3/internal/sqlite_issue221.patch(11.2 KiB), applied to SQLite 3.53.4 viagenerator.go. - Target isolation: Windows targets in
generator.godrop-DSQLITE_OMIT_SEH. - Zero hand-generated
lib/files: No generated Go files (ccgo_windows*.go,sqlite_windows*.go) are committed or hand-edited in this contribution. Transpilation is left entirely to the canonical builder farm. - Zero undeclared compiler flags: No flags were added (no
-DSQLITE_MAX_MMAP_SIZE=0). All 17 non-Windows targets remain 100% byte-identical tomaster. - Non-regression check on
linux/amd64:make mptestpassed cleanly with 0 errors across 13,000+ operations in DELETE, WAL, PERSIST, and TRUNCATE modes.
2. Design
- Shared memory preserved: The shared memory mapping (
*-shmviaMapViewOfFile) is preserved in its entirety. The private heap copy andReadFile/WriteFilesynchronization approach has been completely discarded.
3. SEH Emulation & Verification
- C bookkeeping:
internal/sqlite_issue221.patchenablesSQLITE_USE_SEHbookkeeping under__CCGO__in SQLite 3.53.4, routing the nine WAL SEH sites throughmodernc_seh_try. - Go trampoline (
libsqlite3_windows.go): Implements_modernc_seh_tryusingdebug.SetPanicOnFault(true)andrecover(). - Address discrimination: Extracts the fault address via
interface{ Addr() uintptr }, verifies that the address lies withinpWal->apWiData[], and re-panics (panic(r)) for any fault outside the WAL shared memory mapping. - Error reporting: On fault inside
-shm, it invokeswalHandleException(or returnsSQLITE_IOERR_IN_PAGE/ 8714 directly). - Connection recovery: The test suite verifies that the exact same database connection remains usable after fault recovery (
rc = SQLITE_OKon subsequent statements), matching the MSVC contract. - Two-tier test suite (
issue221_windows_test.go):- Deterministic unit test: Tests nominal handling,
xExcept, and re-panic on foreign addresses outsideapWiData. - Integration test with unencapsulated engine negative witness: Spawns a child process that sets up a real WAL database, protects page 0 of the WAL index, and invokes the unencapsulated engine read path (
_walBeginReadTransaction, matching the pre-patchmasterbehavior). The child process is killed with0xc0000005inside_walIndexTryHdrduringXmemcpy. The parent test catches this crash, then executes the patched path under SEH, proving that the patched engine intercepts the fault, returnsSQLITE_IOERR_IN_PAGE(8714), and allows subsequent queries on the same connection. The test is strictly self-contained and produces output only viat.Logf.
- Deterministic unit test: Tests nominal handling,
4. Compiler Observation (ccgo) & Discretionary Flags
-
ccgoconstant evaluation bug (WALINDEX_PGSZ = 0): During implementation, we discovered thatWALINDEX_PGSZ(sizeof(ht_slot)*HASHTABLE_NSLOT + HASHTABLE_NPAGE*sizeof(u32)) evaluates to0duringccgo's macro-to-Go-constant emission (ccgo_windows.go:21168), as doesWALINDEX_HDR_SIZE(:21166).This reproduces with
ccgo -eval-all-macros(asgenerator.go:259passes); without the flag the expression is evaluated correctly at use sites.Here is a minimal standalone C reproducer (compiles cleanly under
gcc -std=c11 -Wall -Wextra -c):
#include <stdint.h>
typedef uint8_t u8;
typedef uint16_t u16;
typedef uint32_t u32;
typedef uint16_t ht_slot;
typedef struct WalIndexHdr {
u32 iVersion;
u32 unused;
u32 iChange;
u8 isInit;
u8 big_endian;
u16 szPage;
u32 mxFrame;
u32 nPage;
u32 aFrameCksum[2];
u32 aSalt[2];
u32 aCksum[2];
} WalIndexHdr;
#define HASHTABLE_NPAGE 4096
#define HASHTABLE_NSLOT 8192
#define WALINDEX_HDR_SIZE ((sizeof(WalIndexHdr) + 7) & ~7)
#define WALINDEX_PGSZ (sizeof(ht_slot)*HASHTABLE_NSLOT + HASHTABLE_NPAGE*sizeof(u32))
int test_hdr_sz = WALINDEX_HDR_SIZE;
int test_pg_sz = WALINDEX_PGSZ;Compiling with ccgo -eval-all-macros -o repro.go repro.c emits:
const HASHTABLE_NPAGE = 4096
const HASHTABLE_NSLOT = 8192
// ...
const WALINDEX_HDR_SIZE = 0
const WALINDEX_PGSZ = 0While at use sites (test_hdr_sz, test_pg_sz), the expressions evaluate correctly.
Because of this, libsqlite3_windows.go explicitly hardcodes the physical page size constant 32768 (32 KiB) instead of using WALINDEX_PGSZ. We share this reproducer in case you wish to address it in ccgo.
testfixture&mptestflags: Ingenerator.go,-DSQLITE_OMIT_SEHremains in the CFLAGS for Windowstestfixture(line 473) andmptest(line 625) because those standalone CLI tools do not link packagelibsqlite3Go shims. We left this as-is for your arbitrage.- Downstream
sqlitetest note: The test inmodernc.org/sqlitecurrently callst.Skipexplaining thatsqlite/libwas generated upstream withSQLITE_OMIT_SEH, pending your builder farm'smake vendor.
5. Builder Farm Regeneration Note
Because generator.go drops -DSQLITE_OMIT_SEH for Windows targets, the Windows transpilations need regeneration. To request regeneration by your canonical builder farm, this commit blanks internal/autogen/windows_{386,amd64,arm64}.mod according to the documented procedure in CLAUDE.md. Alternatively, running make windows windows_386 before running tests will regenerate the Windows targets locally.
Raw Test Execution Traces (Wine64)
1. Unencapsulated Engine Negative Witness Crash Trace (0xc0000005 in _walIndexTryHdr)
=== RUN TestIssue221_Integration_RealWal
unexpected fault address 0x7f5c61020000
fatal error: fault
[signal 0xc0000005 code=0x0 addr=0x7f5c61020000 pc=0x140209da2]
goroutine 21 [running]:
modernc.org/libc.Xmemcpy(...)
modernc.org/libsqlite3._walIndexTryHdr(...)
modernc.org/libsqlite3._walIndexReadHdr(...)
modernc.org/libsqlite3._walTryBeginRead(...)
modernc.org/libsqlite3._walBeginReadTransaction(...)
modernc.org/libsqlite3.runNegativeWitness()
modernc.org/libsqlite3.TestIssue221_Integration_RealWal(...)2. Patched Engine Execution Trace (Fault Interception & Connection Recovery)
rcFault=10, extCode=8714, rcRecover=0
PASS