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 via generator.go.
  • Target isolation: Windows targets in generator.go drop -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 to master.
  • Non-regression check on linux/amd64: make mptest passed cleanly with 0 errors across 13,000+ operations in DELETE, WAL, PERSIST, and TRUNCATE modes.

2. Design

  • Shared memory preserved: The shared memory mapping (*-shm via MapViewOfFile) is preserved in its entirety. The private heap copy and ReadFile/WriteFile synchronization approach has been completely discarded.

3. SEH Emulation & Verification

  • C bookkeeping: internal/sqlite_issue221.patch enables SQLITE_USE_SEH bookkeeping under __CCGO__ in SQLite 3.53.4, routing the nine WAL SEH sites through modernc_seh_try.
  • Go trampoline (libsqlite3_windows.go): Implements _modernc_seh_try using debug.SetPanicOnFault(true) and recover().
  • Address discrimination: Extracts the fault address via interface{ Addr() uintptr }, verifies that the address lies within pWal->apWiData[], and re-panics (panic(r)) for any fault outside the WAL shared memory mapping.
  • Error reporting: On fault inside -shm, it invokes walHandleException (or returns SQLITE_IOERR_IN_PAGE / 8714 directly).
  • Connection recovery: The test suite verifies that the exact same database connection remains usable after fault recovery (rc = SQLITE_OK on subsequent statements), matching the MSVC contract.
  • Two-tier test suite (issue221_windows_test.go):
    1. Deterministic unit test: Tests nominal handling, xExcept, and re-panic on foreign addresses outside apWiData.
    2. 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-patch master behavior). The child process is killed with 0xc0000005 inside _walIndexTryHdr during Xmemcpy. The parent test catches this crash, then executes the patched path under SEH, proving that the patched engine intercepts the fault, returns SQLITE_IOERR_IN_PAGE (8714), and allows subsequent queries on the same connection. The test is strictly self-contained and produces output only via t.Logf.

4. Compiler Observation (ccgo) & Discretionary Flags

  • ccgo constant evaluation bug (WALINDEX_PGSZ = 0): During implementation, we discovered that WALINDEX_PGSZ (sizeof(ht_slot)*HASHTABLE_NSLOT + HASHTABLE_NPAGE*sizeof(u32)) evaluates to 0 during ccgo's macro-to-Go-constant emission (ccgo_windows.go:21168), as does WALINDEX_HDR_SIZE (:21166).

    This reproduces with ccgo -eval-all-macros (as generator.go:259 passes); 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 = 0

While 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 & mptest flags: In generator.go, -DSQLITE_OMIT_SEH remains in the CFLAGS for Windows testfixture (line 473) and mptest (line 625) because those standalone CLI tools do not link package libsqlite3 Go shims. We left this as-is for your arbitrage.
  • Downstream sqlite test note: The test in modernc.org/sqlite currently calls t.Skip explaining that sqlite/lib was generated upstream with SQLITE_OMIT_SEH, pending your builder farm's make 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
Edited by HazyLab

Merge request reports

Loading
Loading