Skip to content

fix(parser): use alias-safe Haskell arrays - #1232

Merged
DeusData merged 2 commits into
DeusData:mainfrom
WarGloom:fix/haskell-scanner-strict-aliasing
Jul 30, 2026
Merged

fix(parser): use alias-safe Haskell arrays#1232
DeusData merged 2 commits into
DeusData:mainfrom
WarGloom:fix/haskell-scanner-strict-aliasing

Conversation

@WarGloom

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the optimized-build heap corruption in the pinned Haskell external scanner without reducing indexing parallelism.

The scanner vendors an older Tree-sitter array helper that type-puns concrete Array structs through Array(void) pointers. At -O2, the Haskell lookahead growth path can retain the old contents pointer across realloc and write into freed memory. Valgrind identified the first invalid write in advance -> take_line -> newline_lookahead -> tree_sitter_haskell_external_scanner_scan.

The Haskell grammar wrapper now preloads the repository existing strict-aliasing-safe Tree-sitter array helpers, using the shared include guard to skip the unsafe pinned copy. A C11 static assertion locks the helper signature so the unsafe implementation cannot silently return.

Fixes #1231

Validation

  • Reproduced before the fix on oraios/serena at dd7eb6d72ae179aa940e50cd6276ec5646f306f8 with 2, 4, and 20 workers.
  • Production -O2 indexing passed after the fix on fresh copies with 2, 4, and 20 workers: 10,138 nodes each.
  • Sanitized suite: 6,780 passed, 0 failed, 4 skipped across 121 suites.
  • Production parent and worker watchdog tests passed.
  • make -f Makefile.cbm lint-ci passed.
  • scripts/build.sh --with-ui passed.
  • Full local scripts/lint.sh was also attempted; its clang-tidy leg reports existing findings in untouched files under the local toolchain, while the CI lint target above passes.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

Preload the shared Tree-sitter array helpers before the pinned Haskell scanner so optimized builds do not retain a stale contents pointer across realloc. Keep a compile-time signature guard to prevent regression.

Fixes DeusData#1231

Signed-off-by: wargloom <wargloom@gmail.com>
@WarGloom
WarGloom requested a review from DeusData as a code owner July 23, 2026 15:46
@DeusData DeusData added the bug Something isn't working label Jul 24, 2026
@DeusData DeusData added this to the 0.9.1-rc milestone Jul 24, 2026
@DeusData DeusData added stability/performance Server crashes, OOM, hangs, high CPU/memory parsing/quality Graph extraction bugs, false positives, missing edges security Security vulnerabilities, hardening priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 24, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thanks for the very small and plausible fix. Reusing the already-vendored strict-aliasing-safe Tree-sitter array helper is the right shape, introduces no new vendor or dependency, and the head is fully green. The remaining blocker is reproduce-first coverage: no test changed, and the _Static_assert only protects the selected helper signature, not the reported optimized multi-worker heap corruption. Please add a minimized Haskell fixture/runtime test that fails on current main under the relevant optimized strict-aliasing build and passes with this patch.

@DeusData

Copy link
Copy Markdown
Owner

Thanks, and sorry for the slow acknowledgement. Queued for review.

A four-line change with 28 green checks is about as reviewable as a contribution gets — nothing blocking on your side.

Alias-safe Haskell arrays is a precise fix to a real parsing hazard.

@DeusData

Copy link
Copy Markdown
Owner

Thank you — merged, and this one was genuinely satisfying to verify.

The mechanism checks out end to end. Haskell's pinned tree_sitter/array.h carries the old type-punning _array__grow — the known upstream strict-aliasing bug (tree-sitter#5242) where -O2 can keep a stale contents pointer across a realloc. Because the common copy uses the same include guard, preloading it makes the scanner's own include a no-op, so both the macros and the helper come from the alias-safe version. Four lines, no third-party code added, and the only include is an in-repo header we already vendored with its LICENSE.

The _Static_assert over _Generic(&_array__grow, …) is the part I liked most: it matches the safe signature exactly, so if a future grammar refresh ever restores the unsafe copy the build breaks instead of silently regressing to a heisenbug. Your checklist says the change is "covered by a test", which slightly overstates it — there is no runtime repro, and realistically there cannot be a practical one in-suite since it needs -O2 plus a specific corpus. The static assert is the real binding guard, and it is the right one.

I am adding a line to internal/cbm/vendored/grammars/MANIFEST.md recording this override, so that a future refresh-grammars pass does not quietly drop the preload. The longer-term cleanup is to refresh the Haskell grammar from upstream, which has since fixed this — your shim is the correct minimal fix in the meantime.

@DeusData
DeusData merged commit 7e1d0e2 into DeusData:main Jul 30, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. security Security vulnerabilities, hardening stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parallel indexing can corrupt the heap in the Haskell scanner

2 participants