From 9ed79c6cf1a93e76ad4d1b9846f582701c7e8373 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Thu, 22 Oct 2026 17:04:12 +0000 Subject: [PATCH] fix(add-lang): check-grammar fails a grammar that crashes or hangs its process (#2808) Root cause: scripts/add-lang/check-grammar.mjs ran the parse loop in its own process and printed "RESULT: PASS" as soon as the loop finished. A grammar can finish every parse and still take the process down after that. With the issue's vim grammar (tree-sitter-wasm 2.0.4) on Node 24, V8's optimizing tier aborts the process from a background compile job ("Fatal process out of memory: Zone") after PASS is printed, so the verdict and the exit code disagree. The check also ran without the V8 flag indexing always uses (--liftoff-only), so it judged grammars in a setup codegraph never runs them in, and an iteration count of 0 or a word passed without a single parse. Fix: the script runs the grammar in a child process, the way indexing does, and prints the verdict only after that child has exited. - The child gets indexing's V8 flags (WASM_RUNTIME_FLAGS) and loads every vendored grammar beside the one under test, like a polyglot index. - It parses the sample with one reused parser, walks every node of each tree and frees it, as extraction does. Every parse of the same sample must give the same tree. - It then parses the sample repeated to 1 MiB, the most indexing reads from one file, within the 10 s indexing gives one file. Only a crash or a hang counts there. - The parent fails the grammar when the child dies (any exit code or signal), runs past a timeout (CODEGRAPH_CHECK_GRAMMAR_TIMEOUT_MS, 120 s by default), or reports ERROR trees or differing trees. An iteration count that is not a whole number above 0 is refused (exit 2). The add-lang skill now says what counts as a FAIL. Validation: - New __tests__/add-lang-check-grammar.test.ts. A preload marks the process that runs WebAssembly, records its flags, and on request kills it or makes it hang at exit, after the parses. With main's script 4 of 5 tests fail: PASS printed for a process that then died, a hang until the test's 60 s limit, no --liftoff-only in the parsing process, and "0" passing. With the fix all 5 pass. The fifth (the tree-sitter-wasms Lua ABI-13 grammar fails with ERROR trees) passes on both. - The issue's vim grammar (sha256 df3906a0, the issue's file) on Node 24.21.0: main's script printed PASS and then died with the Zone OOM, exit 3, in 3 of 3 runs. The new check runs it with --liftoff-only, as indexing does, where it parses cleanly, 1 MiB included, and passes in 3 of 3. A copy of the new script with the flag withheld fails it in 3 of 3: "the parses finished, but the process running them then died (exit code 2147483651, 0x80000003)". - Old vs new on the 19 vendored grammars with real samples, plus the tree-sitter-wasms Lua and the vim grammar, on Node 22.22.0 and 24.21.0 (84 runs): the same verdict every time, except vim on Node 24, where main's PASS came with exit 3. No grammar's parses diverged. Fixes #2464 Co-Authored-By: Claude Opus 5.5 --- .claude/skills/add-lang/SKILL.md | 9 +- __tests__/add-lang-check-grammar.test.ts | 116 +++++++++++ scripts/add-lang/check-grammar.mjs | 234 +++++++++++++++++++---- 3 files changed, 315 insertions(+), 44 deletions(-) create mode 100644 __tests__/add-lang-check-grammar.test.ts diff --git a/.claude/skills/add-lang/SKILL.md b/.claude/skills/add-lang/SKILL.md index 37cbdce5..24ac69ef 100644 --- a/.claude/skills/add-lang/SKILL.md +++ b/.claude/skills/add-lang/SKILL.md @@ -60,11 +60,14 @@ still be unusable:** ```bash node scripts/add-lang/check-grammar.mjs path/to/valid-sample. ``` -It prints the grammar's ABI version and parses a valid sample many times in a -multi-grammar runtime. If it **FAILs** (ERROR trees on valid code — an old ABI +It prints the grammar's ABI version and runs it the way indexing does: in a +child process with indexing's V8 flags, beside the vendored grammars, parsing a +valid sample many times and once at the 1 MiB file-size limit. If it **FAILs** +(ERROR trees, or later parses giving a different tree, on valid code — an old ABI corrupting the shared WASM heap, which silently drops nested calls/imports on every file after the first; e.g. the tree-sitter-wasms **Lua** grammar is ABI 13 -and fails), do NOT use that wasm. **Vendor a newer (ABI 14/15) build instead:** +and fails — or the process running it crashing or hanging, even after every +parse finished), do NOT use that wasm. **Vendor a newer (ABI 14/15) build instead:** ```bash npm pack @tree-sitter-grammars/tree-sitter- # often ships a prebuilt *.wasm # or build one: npx tree-sitter build --wasm (needs Docker/emscripten) diff --git a/__tests__/add-lang-check-grammar.test.ts b/__tests__/add-lang-check-grammar.test.ts new file mode 100644 index 00000000..93e50380 --- /dev/null +++ b/__tests__/add-lang-check-grammar.test.ts @@ -0,0 +1,116 @@ +/** + * scripts/add-lang/check-grammar.mjs decides whether a grammar wasm is safe to + * vendor. It must judge the grammar by how the process running it ends, not by + * a line printed before that: a grammar can finish every parse and then take + * the process down, as V8 does when a background compile job runs out of Zone + * memory, and the old check had already printed PASS by then (#2464). It must + * also run the grammar the way indexing does, with indexing's V8 flags. + * + * A preload stands in for such a grammar. It marks the process that runs + * WebAssembly, records how that process was launched, and on request makes it + * die or hang at exit, after the parses are done. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { WASM_RUNTIME_FLAGS } from '../src/extraction/wasm-runtime-flags'; + +const SCRIPT = path.resolve('scripts/add-lang/check-grammar.mjs'); +const PYTHON = path.resolve('src/extraction/wasm/tree-sitter-python.wasm'); +const roots: string[] = []; + +const PRELOAD = ` +const fs = require('fs'); +const instantiate = WebAssembly.instantiate; +let ranWasm = false; +WebAssembly.instantiate = function (...args) { ranWasm = true; return instantiate.apply(this, args); }; +process.on('exit', () => { + if (!ranWasm) return; + fs.appendFileSync(process.env.CG_TEST_RECORD, JSON.stringify({ execArgv: process.execArgv }) + '\\n'); + // A hard kill rather than abort(): no crash report on macOS. + if (process.env.CG_TEST_AT_EXIT === 'die') process.kill(process.pid, 'SIGKILL'); + if (process.env.CG_TEST_AT_EXIT === 'hang') for (;;) {} +}); +`; + +function kit() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-check-grammar-')); + roots.push(root); + const preload = path.join(root, 'grammar-process.cjs'); + fs.writeFileSync(preload, PRELOAD); + const python = path.join(root, 'sample.py'); + fs.writeFileSync(python, 'def add(a, b):\n return a + b\n\n\nclass Box:\n def size(self):\n return add(1, 2)\n'); + const lua = path.join(root, 'sample.lua'); + fs.writeFileSync(lua, 'local M = {}\n\nlocal function helper(x)\n return x * 2\nend\n\nfunction M.run(items)\n local out = {}\n for i, v in ipairs(items) do\n out[i] = helper(v)\n end\n return out\nend\n'); + return { root, preload, python, lua, record: path.join(root, 'record.jsonl') }; +} + +function check(k: ReturnType, args: string[], env: NodeJS.ProcessEnv = {}) { + const preload = `--require "${k.preload.replace(/\\/g, '/')}"`; + return spawnSync(process.execPath, [SCRIPT, ...args], { + encoding: 'utf8', + timeout: 60_000, + env: { + ...process.env, + NODE_OPTIONS: [process.env.NODE_OPTIONS, preload].filter(Boolean).join(' '), + CG_TEST_RECORD: k.record, + ...env, + }, + }); +} + +function records(k: ReturnType): Array<{ execArgv: string[] }> { + if (!fs.existsSync(k.record)) return []; + return fs.readFileSync(k.record, 'utf8').trim().split('\n').filter(Boolean).map((line) => JSON.parse(line)); +} + +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('add-lang check-grammar (#2464)', () => { + it('passes a healthy grammar, run in a child process with the V8 flags indexing uses', () => { + const k = kit(); + const run = check(k, [PYTHON, k.python]); + expect(run.status, run.stdout + run.stderr).toBe(0); + expect(run.stdout).toMatch(/RESULT: PASS/); + const ran = records(k); + expect(ran).toHaveLength(1); + for (const flag of WASM_RUNTIME_FLAGS) expect(ran[0]?.execArgv).toContain(flag); + }, 90_000); + + it('fails when the process running the grammar dies after the parses', () => { + const k = kit(); + const run = check(k, [PYTHON, k.python], { CG_TEST_AT_EXIT: 'die' }); + expect(run.status, run.stdout + run.stderr).toBe(1); + expect(run.stdout).not.toMatch(/RESULT: PASS/); + expect(run.stdout).toMatch(/RESULT: FAIL — the parses finished, but the process running them then died/); + }, 90_000); + + it('fails when the process running the grammar hangs instead of exiting', () => { + const k = kit(); + const run = check(k, [PYTHON, k.python], { CG_TEST_AT_EXIT: 'hang', CODEGRAPH_CHECK_GRAMMAR_TIMEOUT_MS: '5000' }); + expect(run.status, run.stdout + run.stderr).toBe(1); + expect(run.stdout).not.toMatch(/RESULT: PASS/); + expect(run.stdout).toMatch(/RESULT: FAIL — the parses finished, but the process running them had not exited/); + }, 90_000); + + it('still fails the tree-sitter-wasms Lua grammar, which corrupts the shared heap on reuse', () => { + const k = kit(); + const run = check(k, ['lua', k.lua]); + expect(run.status, run.stdout + run.stderr).toBe(1); + expect(run.stdout).toMatch(/ABI version: 13/); + expect(run.stdout).toMatch(/RESULT: FAIL — \d+\/20 parses produced ERROR trees/); + }, 90_000); + + it('refuses an iteration count that is not a whole number above 0', () => { + const k = kit(); + for (const iterations of ['0', 'many']) { + const run = check(k, [PYTHON, k.python, iterations]); + expect(run.status, `${iterations}: ${run.stdout}${run.stderr}`).toBe(2); + expect(run.stdout).not.toMatch(/RESULT:/); + } + }, 90_000); +}); diff --git a/scripts/add-lang/check-grammar.mjs b/scripts/add-lang/check-grammar.mjs index 461b1296..9aff9354 100755 --- a/scripts/add-lang/check-grammar.mjs +++ b/scripts/add-lang/check-grammar.mjs @@ -10,66 +10,218 @@ // upstream ABI-15 wasm. Run this on any new grammar first; if it FAILs, vendor a // newer build instead of using the tree-sitter-wasms one. // +// The grammar runs the way indexing runs it, in a child process: +// - with indexing's V8 flags (WASM_RUNTIME_FLAGS in +// src/extraction/wasm-runtime-flags.ts), so V8 compiles it as it would there; +// - next to every grammar codegraph vendors, like a polyglot index; +// - parsing the sample with one reused parser, walking every node of each tree +// and freeing it, as extraction does; every parse must give the same tree; +// - then once more at the largest file indexing reads (the sample repeated to +// 1 MiB, MAX_SOURCE_FILE_SIZE_BYTES in src/file-limits.ts), with the time +// indexing gives one file. +// This process prints the verdict only after that child has exited. A grammar +// can finish every parse and still take the process down, as when V8 aborts it +// from a background compile job ("Fatal process out of memory: Zone"), or hang; +// either one is a FAIL (#2464). +// // Usage: node scripts/add-lang/check-grammar.mjs [iterations] -// Exit: 0 healthy, 1 corruption / parse errors, 2 could not run. +// Exit: 0 healthy; 1 parse errors, corruption, or the check process crashed or +// hung; 2 could not run. +// CODEGRAPH_CHECK_GRAMMAR_TIMEOUT_MS: how long the child may run (default 120000). // NOTE: the sample must be SYNTACTICALLY VALID — a broken sample fails for the // wrong reason. -import { readFileSync, existsSync } from 'node:fs'; +import { readFileSync, existsSync, readdirSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; import { createRequire } from 'node:module'; -import { Parser, Language } from 'web-tree-sitter'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; const require = createRequire(import.meta.url); -const fail = (code, msg) => { console.error(`[check-grammar] ${msg}`); process.exit(code); }; - -const [token, sample, iterArg] = process.argv.slice(2); -if (!token || !sample) fail(2, 'usage: check-grammar.mjs [iterations]'); -if (!existsSync(sample)) fail(2, `sample not found: ${sample}`); -const iters = iterArg ? parseInt(iterArg, 10) : 20; - +const SCRIPT = fileURLToPath(import.meta.url); +const VENDORED_DIR = path.resolve(path.dirname(SCRIPT), '../../src/extraction/wasm'); +// Indexing's V8 flags. Keep in step with WASM_RUNTIME_FLAGS: +// __tests__/add-lang-check-grammar.test.ts fails when the child lacks one. +const INDEXING_WASM_FLAGS = ['--liftoff-only']; +// The most indexing reads from one file (MAX_SOURCE_FILE_SIZE_BYTES), and how +// long it gives one file to parse (DEFAULT_PARSE_TIMEOUT_MS in parse-pool.ts). +const MAX_FILE_BYTES = 1024 * 1024; +const FILE_BUDGET_MS = 10_000; +const CHILD_ENV = 'CODEGRAPH_CHECK_GRAMMAR_CHILD'; +const RESULT_TAG = 'CHECK_GRAMMAR_RESULT '; +const DEFAULT_TIMEOUT_MS = 120_000; const SPECIAL = { csharp: 'c_sharp', 'c#': 'c_sharp' }; + +const fail = (code, msg) => { console.error(`[check-grammar] ${msg}`); process.exit(code); }; +const samePath = (a, b) => (process.platform === 'win32' ? a.toLowerCase() === b.toLowerCase() : a === b); + function resolveWasm(t) { - if (t.endsWith('.wasm')) return existsSync(t) ? t : fail(2, `wasm not found: ${t}`); + if (t.endsWith('.wasm')) return existsSync(t) ? path.resolve(t) : fail(2, `wasm not found: ${t}`); const base = SPECIAL[t.toLowerCase()] ?? t.toLowerCase(); try { return require.resolve(`tree-sitter-wasms/out/tree-sitter-${base}.wasm`); } catch { /* try vendored */ } - const vendored = `src/extraction/wasm/tree-sitter-${base}.wasm`; + const vendored = path.join(VENDORED_DIR, `tree-sitter-${base}.wasm`); if (existsSync(vendored)) return vendored; return fail(2, `no grammar for "${t}" — not in tree-sitter-wasms and not vendored`); } -const wasmPath = resolveWasm(token); -const source = readFileSync(sample, 'utf8'); +/** The parent: run the grammar in a child process, then judge how it ended. */ +function judge([token, sample, iterArg]) { + if (!token || !sample) fail(2, 'usage: check-grammar.mjs [iterations]'); + if (!existsSync(sample)) fail(2, `sample not found: ${sample}`); + const iters = iterArg === undefined ? 20 : Number(iterArg); + if (!Number.isInteger(iters) || iters < 1) fail(2, `iterations must be a whole number above 0, not "${iterArg}"`); + const wasmPath = resolveWasm(token); + const timeoutMs = Number(process.env.CODEGRAPH_CHECK_GRAMMAR_TIMEOUT_MS) || DEFAULT_TIMEOUT_MS; -try { await Parser.init(); } -catch { await Parser.init({ locateFile: () => require.resolve('web-tree-sitter/tree-sitter.wasm') }); } + const run = spawnSync(process.execPath, [...INDEXING_WASM_FLAGS, SCRIPT, wasmPath, path.resolve(sample), String(iters)], { + env: { ...process.env, [CHILD_ENV]: '1' }, + encoding: 'utf8', + timeout: timeoutMs, + maxBuffer: 64 * 1024 * 1024, + windowsHide: true, + }); + const timedOut = run.error?.code === 'ETIMEDOUT'; + if (run.error && !timedOut) fail(2, `could not run the check: ${run.error.message}`); -// Load a second, known-good grammar — the corruption surfaces under the -// multi-grammar runtime that real indexing uses, not a single grammar in isolation. -try { await Language.load(require.resolve('tree-sitter-wasms/out/tree-sitter-python.wasm')); } catch { /* ok */ } + const lines = (run.stdout ?? '').split(/\r?\n/); + const tagged = lines.find((line) => line.startsWith(RESULT_TAG)); + const result = tagged ? JSON.parse(tagged.slice(RESULT_TAG.length)) : null; + const passthrough = lines.filter((line) => !line.startsWith(RESULT_TAG)).join('\n').trim(); + if (passthrough) console.log(passthrough); + if (run.stderr) process.stderr.write(run.stderr.endsWith('\n') ? run.stderr : `${run.stderr}\n`); + if (run.status === 2 && !result) { process.exitCode = 2; return; } // the child said why it could not run -let language; -try { language = await Language.load(wasmPath); } -catch (e) { fail(2, `failed to load ${wasmPath}: ${e.message}`); } + console.log(`grammar: ${path.basename(wasmPath)}`); + if (result) { + console.log(` ABI version: ${result.abi}`); + console.log(` runtime: node ${[result.node, ...result.execArgv].join(' ')}, with ${result.companions} other grammars loaded`); + console.log(` parses: ${result.clean} clean / ${result.errors} with errors (of ${result.iters})`); + if (result.large) { + const { copies, bytes, nodes, ms } = result.large; + console.log(nodes === null + ? ` largest file: the sample ${copies}x (${bytes} bytes) was still parsing after ${FILE_BUDGET_MS / 1000} s, ` + + 'as long as indexing gives one file, and was stopped (not judged)' + : ` largest file: the sample ${copies}x (${bytes} bytes, ${nodes} nodes) in ${ms} ms`); + } + } -const parser = new Parser(); -parser.setLanguage(language); - -let ok = 0, err = 0; -for (let i = 0; i < iters; i++) { - const tree = parser.parse(source); - if (tree.rootNode.hasError) err++; else ok++; + const [code, verdict] = verdictFor(run, result, timedOut, timeoutMs); + console.log(verdict); + process.exitCode = code; } -console.log(`grammar: ${wasmPath.split('/').pop()}`); -console.log(` ABI version: ${language.abiVersion}`); -console.log(` parses: ${ok} clean / ${err} with errors (of ${iters})`); -if (err > 0) { - console.log( - `RESULT: FAIL — ${err}/${iters} parses produced ERROR trees on a valid sample. ` + - `This grammar corrupts under web-tree-sitter; vendor a newer (ABI 14/15) wasm ` + - `(see SKILL.md "Find a grammar"). Confirm your sample is syntactically valid first.` - ); - process.exit(1); +function verdictFor(run, result, timedOut, timeoutMs) { + const seconds = Math.round(timeoutMs / 1000); + const finishedAnyway = 'Finishing the parses is not the same as being safe to use: indexing would ' + + 'go down the same way. Do not vendor this wasm.'; + if (timedOut) { + return [1, result + ? `RESULT: FAIL — the parses finished, but the process running them had not exited after ${seconds} s ` + + `and was stopped. ${finishedAnyway}` + : `RESULT: FAIL — the parses had not finished after ${seconds} s, so the process running them was stopped: ` + + 'the grammar hangs, or is far too slow, on this sample. Do not vendor this wasm.']; + } + if (run.status !== 0 || !result) { + return [1, result + ? `RESULT: FAIL — the parses finished, but the process running them then died (${describeEnd(run)}). ${finishedAnyway}` + : `RESULT: FAIL — the process running the parses died (${describeEnd(run)}) before they finished. ` + + 'Indexing would go down the same way. Do not vendor this wasm.']; + } + if (result.errors > 0) { + return [1, `RESULT: FAIL — ${result.errors}/${result.iters} parses produced ERROR trees on a valid sample. ` + + 'This grammar corrupts under web-tree-sitter; vendor a newer (ABI 14/15) wasm ' + + '(see SKILL.md "Find a grammar"). Confirm your sample is syntactically valid first.']; + } + if (result.diverged > 0) { + return [1, `RESULT: FAIL — ${result.diverged}/${result.iters - 1} later parses of the same sample gave a ` + + 'different tree than the first. This grammar corrupts under web-tree-sitter; vendor a newer ' + + '(ABI 14/15) wasm (see SKILL.md "Find a grammar").']; + } + return [0, 'RESULT: PASS — grammar parses cleanly and reuses safely, and the process running it exited cleanly.']; } -console.log('RESULT: PASS — grammar parses cleanly and reuses safely.'); -process.exit(0); + +function describeEnd(run) { + if (run.signal) return `signal ${run.signal}`; + const code = run.status; + return code > 255 ? `exit code ${code}, 0x${(code >>> 0).toString(16).toUpperCase()}` : `exit code ${code}`; +} + +/** The child: load and exercise the grammar the way indexing does, then report. */ +async function runGrammar(wasmPath, samplePath, itersArg) { + const iters = Number(itersArg); + const { Parser, Language } = await import('web-tree-sitter'); + try { await Parser.init(); } + catch { await Parser.init({ locateFile: () => require.resolve('web-tree-sitter/tree-sitter.wasm') }); } + + // The other grammars an index loads beside this one: the corruption surfaces + // under the multi-grammar runtime that real indexing uses, not a single + // grammar in isolation. + let companions = 0; + for (const file of readdirSync(VENDORED_DIR).filter((f) => f.endsWith('.wasm')).sort()) { + const companion = path.join(VENDORED_DIR, file); + if (samePath(companion, wasmPath)) continue; + try { await Language.load(companion); companions++; } catch { /* not the grammar under test */ } + } + + let language; + try { language = await Language.load(wasmPath); } + catch (e) { + // No process.exit: on Windows a pipe write can still be pending. + console.error(`[check-grammar] failed to load ${wasmPath}: ${e.message}`); + process.exitCode = 2; + return; + } + + const parser = new Parser(); + parser.setLanguage(language); + const source = readFileSync(samplePath, 'utf8'); + + let clean = 0, errors = 0, diverged = 0, first = null; + for (let i = 0; i < iters; i++) { + const tree = parser.parse(source); + if (!tree) throw new Error('the parser returned no tree'); + const shape = walk(tree); + if (tree.rootNode.hasError) errors++; else clean++; + if (!first) first = shape; + else if (shape.nodes !== first.nodes || shape.hash !== first.hash) diverged++; + tree.delete(); + } + + // The largest file indexing reads, with the time indexing gives one file, + // unless the grammar has already failed. Repeating a sample can make code the + // language rejects, and recovering from that can be slow, so here only a + // crash or a hang counts. + let large = null; + if (errors === 0 && diverged === 0) { + const copies = Math.max(1, Math.floor(MAX_FILE_BYTES / (Buffer.byteLength(source) + 1))); + const big = Array(copies).fill(source).join('\n'); + const started = performance.now(); + const bigTree = parser.parse(big, null, { progressCallback: () => performance.now() - started > FILE_BUDGET_MS }); + large = { copies, bytes: Buffer.byteLength(big), nodes: bigTree ? walk(bigTree).nodes : null, ms: Math.round(performance.now() - started) }; + bigTree?.delete(); + } + parser.delete(); + + console.log(RESULT_TAG + JSON.stringify({ + abi: language.abiVersion, iters, clean, errors, diverged, companions, large, + node: process.version, execArgv: process.execArgv, + })); +} + +/** Visit every node, as extraction does; returns the tree's size and a hash of its shape. */ +function walk(tree) { + const cursor = tree.walk(); + let nodes = 0, hash = 2166136261; + const mix = (value) => { hash = Math.imul(hash ^ value, 16777619) >>> 0; }; + for (;;) { + nodes++; + mix(cursor.nodeTypeId); mix(cursor.currentFieldId); mix(cursor.startIndex); mix(cursor.endIndex); + if (cursor.gotoFirstChild()) continue; + while (!cursor.gotoNextSibling()) { + if (!cursor.gotoParent()) { cursor.delete(); return { nodes, hash }; } + } + } +} + +if (process.env[CHILD_ENV] === '1') await runGrammar(...process.argv.slice(2)); +else judge(process.argv.slice(2));