diff --git a/.github/workflows/_test.yml b/.github/workflows/_test.yml index 2f8c2af769..1b04d42d5f 100644 --- a/.github/workflows/_test.yml +++ b/.github/workflows/_test.yml @@ -370,16 +370,19 @@ jobs: # artifact — the local ladder cannot answer that, having no faithful x86-64 # emulation. It has now answered it, and the answer SPLIT the list: # - # - the five deep-recursion suites overflow their thread stacks on x86-64 - # too, so that limit is NOT architectural; + # - five suites aborted with "stack-overflow ... nested bug" on both + # architectures. That was not deep recursion: it was MSan crashing while + # it reported real uninitialized reads (reporting with the slow unwinder + # exhausted the main thread's stack). scripts/msan.sh now unwinds with + # frame pointers, the + # reports print, the reads are fixed, and the five suites run again; # - `cli` does not overflow here at all — it fails in the install path for # an unrelated, still-undiagnosed reason; # - `incremental` was a shadow-memory RSS artifact, now fixed in the test # itself rather than skipped, so it stays in the lane. # # Per-cause detail lives in scripts/msan.sh, which is the single authoritative - # list both venues now share. Zero use-of-uninitialized-value findings were - # reported across the lane on either architecture. + # list both venues now share. test-msan: if: ${{ inputs.lanes == 'all' || contains(fromJSON(inputs.lanes), 'msan') }} runs-on: ubuntu-latest diff --git a/internal/cbm/vendored/grammars/MANIFEST.md b/internal/cbm/vendored/grammars/MANIFEST.md index 3ee1344a55..889306a518 100644 --- a/internal/cbm/vendored/grammars/MANIFEST.md +++ b/internal/cbm/vendored/grammars/MANIFEST.md @@ -111,6 +111,7 @@ row instead. | purescript | `purescript/scanner.c`, serialize | guard `memcpy(buffer, indents->data, to_copy)` with `if (to_copy > 0)` | UBSan: zero-length `memcpy` with a NULL/0-size source when the indent vector is empty (formal UB, harmless) | | plsql | `plsql/parser.c`, include | `#include ` → `#include "tree_sitter/parser.h"` | The older ABI-14 generator emits angle brackets; every other vendored grammar uses the quoted form, which resolves the per-grammar `tree_sitter/` header from the including file's directory | | properties | `properties/scanner.c`, whole file | move the `reached_eof` flag out of a file-scope `static bool` and into a per-parser payload allocated by `..._external_scanner_create()` | **Data race, and the only one of its kind in 103 vendored scanners.** That flag is how the scanner refuses to emit a second end-of-input `FAKE_EOL`, and upstream keeps it in ONE process-wide object. With several worker threads indexing `.properties` files at once, whichever reached EOF first set the flag, so another thread's `!reached_eof` was false and its parse never received the `FAKE_EOL` it needed to finish — it sat at end-of-input asking for a token that would never arrive. Measured 2026-09-19: ~106 M parse operations on a 253-byte file before the per-file budget stopped it, after which the file was dropped from the graph and java/kotlin indexes differed run to run. Interleaved A/B under identical load, 40 runs each: unpatched 6 stalls, patched 0. The serialize/deserialize wire format is unchanged (the state is still the returned length). Upstream `6310671b24d4` still carries the static, so a re-vendor must re-apply this | +| wit | `wit/scanner.c`, `..._external_scanner_create` | `{}` → `{ return NULL; }` | **Uninitialized read (MSan).** The function returns `void *` but had no `return`, and tree-sitter's runtime stores the result as the scanner payload, so every WIT parse used an indeterminate value (C11 6.9.1p12 makes that undefined). The scanner never reads its payload, so ordinary builds were unaffected, and the vendored build's `-w` hid `-Wreturn-type`. Under MSan the report of this read crashed and posed as a "stack-overflow", which is why five grammar/pipeline suites were excluded from the MSan lane. Upstream bytecodealliance/tree-sitter-wit still carries it at `059bfdd1af72` (2025-06-23, the last change to `src/scanner.c`) | | swift | `swift/scanner.c`, `OP_SYMBOL_SUPPRESSOR` + `eat_operators` | `1UL <<` / `1 <<` → `1ULL <<` | UBSan: `1 << suppressor` shifts an `int` by up to `TOKEN_COUNT` bits, undefined once the index reaches 31, while the mask it feeds is `uint64_t`. `1UL << FAKE_TRY_BANG` is the same defect on Windows, where `unsigned long` is 32 bits and `FAKE_TRY_BANG` is 32; the CLANGARM64 leg runs UBSan in trap mode, so there it is an illegal instruction rather than a log line. Upstream already carries both changes: `fb63a7004f07` (2026-04-06, upstream #558) for `eat_operators`, `6ab8d1d74ebd` (2026-08-10) for the `OP_SYMBOL_SUPPRESSOR` entry. Our pin `8abb3e8b3325` (2026-03-20) predates both, so this is a backport rather than a local invention — a re-vendor past 2026-08-10 should delete this row, not re-apply it | ## Vendored from verified upstream diff --git a/internal/cbm/vendored/grammars/wit/scanner.c b/internal/cbm/vendored/grammars/wit/scanner.c index 829611a7bd..3a98cbf675 100644 --- a/internal/cbm/vendored/grammars/wit/scanner.c +++ b/internal/cbm/vendored/grammars/wit/scanner.c @@ -10,7 +10,7 @@ enum TokenType { LINE_DOC_CONTENT, }; -void *tree_sitter_wit_external_scanner_create(void) {} +void *tree_sitter_wit_external_scanner_create(void) { return NULL; } void tree_sitter_wit_external_scanner_destroy(void *payload) {} diff --git a/scripts/ci/msan-lane.sh b/scripts/ci/msan-lane.sh index 3112585966..c8e4f3a101 100755 --- a/scripts/ci/msan-lane.sh +++ b/scripts/ci/msan-lane.sh @@ -38,13 +38,14 @@ run_suite() { # excluded suite and the cause it is excluded for). # # This leg used to force it empty to settle whether the local arm64 - # exclusions were an aarch64 artifact. It answered that: the five - # deep-recursion suites overflow on x86-64 too, `cli` fails for an unrelated - # install-path reason, and `incremental` was a shadow-memory RSS artifact - # that is now fixed in the test rather than skipped. Keeping the override - # would re-red the gate for causes already recorded, so the question is - # closed and the venues share one list. scripts/msan.sh still warns loudly - # that the lane is partial, which is the honest signal to keep. + # exclusions were an aarch64 artifact. They were not: `cli` fails for an + # install-path reason, `incremental` was a shadow-memory RSS artifact (now + # fixed in the test), and the five suites that "overflowed" on both + # architectures were hiding real uninitialized reads whose reports crashed + # under the slow unwinder. With fast unwinding (scripts/msan.sh) those reports + # print, the reads are fixed, and the five suites are back in the lane. + # Only `cli` is still excluded, and scripts/msan.sh warns that the lane is + # partial while anything is. echo "=== MSan suite (exclusions per scripts/msan.sh) ===" docker run --rm -v "$PWD:/src" -w /src "$IMAGE" } diff --git a/scripts/msan.sh b/scripts/msan.sh index 954608890a..20bb8fafe7 100755 --- a/scripts/msan.sh +++ b/scripts/msan.sh @@ -9,13 +9,9 @@ # the clang-analyzer lane do not cover dynamically. halt_on_error stays ON: # a finding is a bug (or an interceptor gap to triage), never board data. # -# VENUES: the CI test-msan job (x86-64) is AUTHORITATIVE and runs with no -# exclusions. The local container is arm64, where deep-recursion suites -# overflow their thread stacks under instrumentation — MSan's shadow/stack -# handling is materially better supported on x86-64, so that limitation may -# be architectural, and the local ladder has no faithful x86-64 emulation to -# decide it. An accepted venue divergence for this lane specifically: the -# exclusions below are the LOCAL default only, and CI overrides them away. +# VENUES: the CI test-msan job (x86-64, authoritative) and the local arm64 +# container (test-infrastructure/docker-compose.yml, service test-msan) both +# run this script with the same exclusion list below. # # Usage: scripts/msan.sh [suite ...] (default: full suite) set -euo pipefail @@ -33,12 +29,10 @@ fi # one cannot be shadowed); its headers live under the MSan prefix. # MSAN_ORIGINS: 2 = full origin chains (best reports, heaviest frames), # 1 = immediate origin only, 0 = none. Detection is IDENTICAL at every level; -# only report quality differs, so lowering it is the first lever to try when -# frame inflation overflows a deep-recursion suite. -# Default 1 on this lane: identical DETECTION at every level (only report -# depth differs), and the frame savings are what lets the deep-recursion -# grammar suites fit their stacks under instrumentation. Export -# MSAN_ORIGINS=2 locally when chasing a specific report's origin chain. +# only report quality differs. +# Default 1 on this lane: the immediate origin is enough to locate a read, and +# the lighter frames keep the lane cheaper. Export MSAN_ORIGINS=2 locally when +# chasing a specific report's origin chain. MSAN_ORIGINS="${MSAN_ORIGINS:-1}" if [ "$MSAN_ORIGINS" = "0" ]; then MSAN_ORIGIN_FLAG="" @@ -70,11 +64,20 @@ make -j"$(nproc)" -f Makefile.cbm build/msan/test-runner \ CXX_STDLIB_FLAGS="-stdlib=libc++ -nostdinc++ -isystem $MSAN_PREFIX/include/c++/v1" \ CXX_STDLIB="-L$MSAN_PREFIX/lib -Wl,-rpath,$MSAN_PREFIX/lib -lc++ -lc++abi" -export MSAN_OPTIONS="${MSAN_OPTIONS:-halt_on_error=1:print_stats=0}" -# Origin tracking inflates every frame; the grammar-corpus suites drive the -# deepest parser recursion in the tree. Raise what can be raised (main-thread -# stack via RLIMIT_STACK, worker stacks via the sanitized-build-only knob in -# cbm_thread_create) — see the exclusion note below for what this does NOT fix. +# fast_unwind_on_fatal=1: a report's stack is walked with frame pointers (this +# build keeps them, -fno-omit-frame-pointer) instead of the default slow +# unwinder, which in this image is the MSan-instrumented libunwind. With the +# slow unwinder, reporting an uninitialized read exhausted the main thread's +# stack, so the report never printed and MSan aborted with "stack-overflow" +# followed by "nested bug in the same thread" instead. That disguise kept five +# suites excluded as deep recursion while they hid a real read (the WIT +# scanner's create function returned nothing; see the grammar MANIFEST.md). +# Measured on the same tree and test: slow unwind aborts at the 256 MiB stack +# limit, fast unwind prints the use-of-uninitialized-value report. +export MSAN_OPTIONS="${MSAN_OPTIONS:-halt_on_error=1:print_stats=0:fast_unwind_on_fatal=1}" +# Instrumented frames are several times larger than plain ones, so give the +# stacks headroom: the main thread via RLIMIT_STACK, worker threads via the +# sanitized-build-only knob in cbm_thread_create. ulimit -s 262144 2>/dev/null || true export CBM_THREAD_STACK_MB="${CBM_THREAD_STACK_MB:-256}" export LD_LIBRARY_PATH="$MSAN_PREFIX/lib${LD_LIBRARY_PATH:+:$LD_LIBRARY_PATH}" @@ -102,22 +105,22 @@ fi echo "=== MSan lane: $(clang --version | head -1) ===" -# KNOWN RED — THREE DISTINCT CAUSES, whitelisted per cause (O10) +# KNOWN RED — whitelisted per cause (O10). Only (B) is still excluded. # -# The x86-64 CI leg deliberately runs this lane WITHOUT these exclusions, to -# settle which limits are architectural. It has now done so, and it SPLIT the -# seven suites an earlier version of this block lumped together as one cause. -# That earlier claim -- "seven suites abort with stack-overflow" -- was wrong -# for two of them and is corrected here. -# -# (A) STACK-OVERFLOW under instrumentation — FIVE suites: -# grammar_regression grammar_labels pipeline lang_contract grammar_probe_e -# Each aborts with "MemorySanitizer: stack-overflow" followed by "nested bug -# in the same thread, aborting". The fault is always the same pc (inside -# MSan's memset interceptor -- the instruction that happens to touch the -# guard page, not the recursion source) on a pipeline worker thread. It -# reproduces on BOTH arm64 (local) and x86-64 (CI logged 5 overflows), so it -# is NOT the aarch64 shadow-mapping artifact an earlier note here claimed. +# (A) "STACK-OVERFLOW" — FIXED, no longer excluded. Five suites +# (grammar_regression grammar_labels pipeline lang_contract +# grammar_probe_e) aborted on both arm64 and x86-64 with "MemorySanitizer: +# stack-overflow" followed by "nested bug in the same thread, aborting". +# It was never deep recursion. MSan crashed while REPORTING real +# uninitialized reads (see fast_unwind_on_fatal above): the WIT scanner's +# create function returned nothing (the four grammar suites), and +# githistory_coupling_limits_output left its commits' timestamps +# uninitialized (pipeline). Both are fixed and the suites are back in the +# lane. The crashing thread was the MAIN thread: MSan's "T" is the OS +# thread id, and every abort showed T equal to the pid. An earlier +# version of this block read it as a worker-thread ordinal, which is why +# the worker-stack knob, MSAN_ORIGINS and CBM_WORKERS=1 never changed +# anything and why it proposed a byte-measuring recursion guard. # # (B) INSTALL/ACTIVATION failures — cli. NOT an overflow. On x86-64 the suite # runs to completion: 253 passed, 5 failed, every failure in the install or @@ -126,10 +129,9 @@ echo "=== MSan lane: $(clang --version | head -1) ===" # suite is green on every other venue. MSan reported ZERO # use-of-uninitialized-value here, so excluding it costs no uninit coverage # -- which is the only thing this lane exists to provide. -# NOT DIAGNOSED, and honestly so: the local lane is arm64, where these -# suites hit (A) long before reaching this code, so there is no faithful -# venue to iterate in and each attempt costs a ~30min CI round trip. -# Recorded as a follow-up rather than guessed at from a log. +# NOT DIAGNOSED yet. Recorded as a follow-up rather than guessed at from a +# log; with (A) understood, the local arm64 container is a usable venue +# for it (scripts/msan.sh cli). # # (C) RSS BUDGET — incremental. FIXED rather than excluded, so it is no longer # in the list below: the budget assertion is now skipped under @@ -139,51 +141,45 @@ echo "=== MSan lane: $(clang --version | head -1) ===" # instead would blind the guard on the platforms where it does work. The # suite stays IN this lane and keeps its uninitialized-read coverage. # -# WHAT WAS TRIED for (A) — each disproven by measurement, do not repeat: -# 1. RLIMIT_STACK 8 -> 64 -> 256 MiB, and `ulimit -s unlimited`. No effect; -# the crashing thread is not the main thread. -# 2. CBM_THREAD_STACK_MB at 256 MiB and at 1024 MiB (the cap). The knob is -# verified compiled in (CBM_SANITIZED_BUILD is defined for this lane) and -# the floor is applied in cbm_thread_create for both the default and -# explicit-size paths. The fault address did not move by a single byte -# between 256 MiB and 1024 MiB -- a 4x stack increase changing nothing is -# what rules out "the stack is merely too small". -# 3. MSAN_ORIGINS 2 -> 1 -> 0. Detection is identical at every level and 0 -# gives the smallest frames; same thread, same address. Frame inflation is -# not the trigger. -# 4. One suite per process (the sharding below). It removed the cumulative -# thread-ordinal effect an earlier note suspected, and thousands of tests -# now run before the wall, but the deep suites still abort. -# 5. CBM_WORKERS=1, to push the recursion onto the main thread where the -# rlimit does apply. Still a worker thread, still overflows. -# -# WHAT THIS POINTS AT (the follow-up, not a guess to act on blindly): the -# recursion guards this tree does have -- see the stack_overflow_a/b/c suites, -# which pass -- bound DEPTH, while the resource actually exhausted is BYTES. -# Instrumented frames are several times larger, so the byte budget is gone -# before the depth counter trips. If that is right, the fix is a guard that -# measures remaining stack rather than counted depth, and it would make these -# suites pass under every sanitizer rather than papering over one lane. +# WHY (B) IS EXCLUDED RATHER THAN LEFT RED: the lane is gating. A permanently +# red gate teaches everyone to ignore it and hides the findings the other +# suites produce, which is the entire reason this lane exists. The exclusion is +# narrow, named, and expires when the install failure is diagnosed. It is not a +# claim that cli is covered, and the lane says so when it runs (below). # -# WHY (A) AND (B) ARE EXCLUDED RATHER THAN LEFT RED: the lane is gating. A -# permanently red gate teaches everyone to ignore it, and it hides the -# uninitialized-read findings the other ~130 suites DO produce -- which is the -# entire reason this lane exists. Each exclusion is narrow, named, and expires -# with its own fix: (A) when the recursion guard measures bytes, (B) when the -# install failure is diagnosed on a venue that can run it. Neither is a claim -# that those suites are covered. -MSAN_EXCLUDE="${MSAN_EXCLUDE-grammar_regression grammar_labels pipeline cli lang_contract grammar_probe_e}" +# LESSON FROM (A): an MSan abort that is not a use-of-uninitialized-value +# report is a reporting failure until proven otherwise. Look for the read +# first (fast unwinding prints it), not for a stack limit. +MSAN_EXCLUDE="${MSAN_EXCLUDE-cli}" if [ "$#" -gt 0 ]; then ./build/msan/test-runner "$@" -elif true; then +else # One suite per process: a fresh process per suite keeps one suite's # thread/allocator state from reaching the next, and makes a failure name # exactly one suite. Suite enumeration comes from --list-suites, whose # completeness the sharding union guard already proves; the known-red set # documented above is skipped by name and nothing else is. + all_suites="$(./build/msan/test-runner --list-suites)" + # Fail loudly if an entry matches no suite: a typo (or a TEST name given + # where a SUITE name is required) would otherwise exclude nothing silently. + # A case match, not `printf | grep -q`, which pipefail can turn into a + # false failure when grep exits early. + for e in $MSAN_EXCLUDE; do + case $'\n'"$all_suites"$'\n' in + *$'\n'"$e"$'\n'*) ;; + *) + echo "FATAL: MSAN_EXCLUDE entry '$e' is not a suite name" >&2 + exit 1 + ;; + esac + done + if [ -n "$MSAN_EXCLUDE" ]; then + echo "WARNING: running a PARTIAL sanitizer lane — excluded: $MSAN_EXCLUDE" >&2 + echo "WARNING: a green result here does NOT mean the tree is MSan-clean." >&2 + fi fails="" - for suite in $(./build/msan/test-runner --list-suites); do + for suite in $all_suites; do case " $MSAN_EXCLUDE " in *" $suite "*) echo "=== msan: $suite SKIPPED (known-red, see the block above) ===" @@ -197,18 +193,4 @@ elif true; then echo "=== MSan lane FAILED suites:$fails ===" >&2 exit 1 fi -else - echo "WARNING: running a PARTIAL sanitizer lane — excluded: $MSAN_EXCLUDE" >&2 - echo "WARNING: a green result here does NOT mean the tree is MSan-clean." >&2 - excl_pattern="$(printf '%s\n' $MSAN_EXCLUDE | tr '\n' '|' | sed 's/|$//')" - suites="$(./build/msan/test-runner --list-suites | grep -Evx "$excl_pattern")" - # Fail loudly if an entry matched no suite: a typo (or a TEST name given - # where a SUITE name is required) would otherwise exclude nothing silently. - for e in $MSAN_EXCLUDE; do - ./build/msan/test-runner --list-suites | grep -qx "$e" || { - echo "FATAL: MSAN_EXCLUDE entry '$e' is not a suite name" >&2; exit 1; } - done - echo "--- excluded: $MSAN_EXCLUDE (see the comment in $0) ---" - # shellcheck disable=SC2086 # deliberate word-splitting of the suite list - ./build/msan/test-runner $suites fi diff --git a/scripts/vendored-checksums.txt b/scripts/vendored-checksums.txt index 224b6cd803..a34a5eacf7 100644 --- a/scripts/vendored-checksums.txt +++ b/scripts/vendored-checksums.txt @@ -5,7 +5,7 @@ c5cfb43042b6b72045f4ba997834d0a7786d2793d91680868b5815b39f14fc78 internal/cbm/v b29c1c9fb7cc82f58c84b376df1297d6e2737a1d655fd356db0859e3c29c2fea internal/cbm/vendored/common/tree_sitter/alloc.h 31e60a1bff6f715afacce03b5b70efe42b58371b4f9595dd4af52a577ff9608c internal/cbm/vendored/common/tree_sitter/array.h 180b893c8734778fd32f372dfbc27bd6ad1cd2221f26150b31256ff6716320d2 internal/cbm/vendored/common/tree_sitter/parser.h -44bade69b14ac382b08835965eeec5f0fd6e54ccb7a80da21c0161128a10503f internal/cbm/vendored/grammars/MANIFEST.md +57006171adb332d760ede4b413d900b2bbb57f55d3bf94ceecb6e74a720bc722 internal/cbm/vendored/grammars/MANIFEST.md ad8425038de519f8c4e3e9339feebf99dfad8a6002dcf79227d91402780a32cc internal/cbm/vendored/grammars/ada/LICENSE 02805ec13939b749c891567be36bf024b09034e04c80683a1ae667272458d549 internal/cbm/vendored/grammars/ada/parser.c 115a75d000bef9c70c4de7dfbb7f2a80b90cbb14ba265056dc7abb1ef3b9b6db internal/cbm/vendored/grammars/ada/tree_sitter/alloc.h @@ -899,7 +899,7 @@ d1b7f7ca8ec73d915cec4aa64d9e8b7f2d89b874225d4532e00d9c36c98528fe internal/cbm/v df898ca6d3eb880b9fdf7d3179363d7657abcd4b0a0565d8fd6b47215d8cc149 internal/cbm/vendored/grammars/wgsl/tree_sitter/parser.h 3b26cd02b9d9bf324bce59edd88c02cb93fa9a908528b805264db8cde53295e6 internal/cbm/vendored/grammars/wit/LICENSE d2c4cab0df0ba3cc692ae0731de4c644ad41114fcfd5b5a7a8ae669df39f8434 internal/cbm/vendored/grammars/wit/parser.c -9749b186b3b7620995d9a9b747d34c39949fc497c20d7b79391799014bad9470 internal/cbm/vendored/grammars/wit/scanner.c +0ec9697a583cf96af605af0f9a7a660e01e42c81cae847580bb2a44dbbbde8a7 internal/cbm/vendored/grammars/wit/scanner.c 115a75d000bef9c70c4de7dfbb7f2a80b90cbb14ba265056dc7abb1ef3b9b6db internal/cbm/vendored/grammars/wit/tree_sitter/alloc.h db7d66e4305e3f37b5d31a3291acc01fed95054474a75b3c7cff72048ef9b797 internal/cbm/vendored/grammars/wit/tree_sitter/array.h c4b9482069f61a2a26a590181baa34f08eb6dcc57a138def578e6f9720c35ea5 internal/cbm/vendored/grammars/wit/tree_sitter/parser.h diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 7684868e42..60a639ae1b 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -15707,7 +15707,9 @@ TEST(githistory_coupling_limits_output) { int idx = 0; char *pair_files[2450][2]; /* 50*49/2 pairs * 3 repetitions = 3675 commits */ char pair_bufs[2450][2][32]; - cbm_commit_files_t commits[3675]; + /* Zeroed: the loop sets only files/count, and cbm_compute_change_coupling + * also reads each commit's timestamp (MSan found it uninitialized). */ + cbm_commit_files_t commits[3675] = {0}; int ci = 0; for (int i = 0; i < 50 && ci < 3675; i++) { for (int j = i + 1; j < 50 && ci < 3675; j++) {