From 174e56b447ca4bb894e5368168e7b821958dbd04 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 2 Jul 2026 18:39:17 +0200 Subject: [PATCH] =?UTF-8?q?fix(extract):=20heap-allocate=20walk=5Fdefs=20f?= =?UTF-8?q?rame=20stack=20=E2=80=94=20fix=20overflow=20+=20>4096=20truncat?= =?UTF-8?q?ion=20(#668)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit walk_defs used a fixed `walk_defs_frame_t stack[4096]` — a single ~160 KB frame on the C stack. Two problems: - On small thread stacks (notably the pre-2026-03 Windows 1 MB main thread) that huge frame overflowed the stack during the definitions pass — the 0xC00000FD crash a reporter hit indexing a large SQL file (#668). Reproduced locally by running extraction under a constrained stack (~208 KB) + ASan. - The `top < 4096` push guards SILENTLY DROPPED every top-level definition past 4096, so a file with more than 4096 top-level defs was only partially indexed. Replace the fixed array with a growable heap stack (wd_stack_t + wd_push): a 256-frame initial buffer that doubles on demand, bounded by a generous, env-configurable ceiling (CBM_WALK_DEFS_MAX, default 8M frames) that WARNs once instead of dropping, and returns cleanly on realloc OOM (no NULL deref). A normal file now uses ~10 KB of heap instead of 160 KB of stack, and files with thousands of top-level defs are fully extracted. The frame stack is a pure index-addressed LIFO (frames copied by value on pop), so heap-backing is semantically identical. Reproduce-first: walk_defs_no_truncation_over_4096_issue668 — 5000 top-level defs must all be extracted (RED on the old 4096 cap, GREEN after). Full suite 5748/0, no ASan/UBSan reports on the new alloc. Root cause was pinned by reproduction (constrained-stack + ASan), refuting earlier grammar/scanner hypotheses — extraction and the vendored tree-sitter tree ops are otherwise iterative. The separate error-recovery assertion mode (symbol --- internal/cbm/extract_defs.c | 119 ++++++++++++++++++++++++++---------- tests/test_extraction.c | 28 +++++++++ 2 files changed, 116 insertions(+), 31 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 92df9b350..9db45d624 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3,11 +3,15 @@ #include "helpers.h" #include "lang_specs.h" #include "foundation/constants.h" +#include "foundation/platform.h" // safe_realloc (frees old on failure) +#include "foundation/log.h" // cbm_log_warn #include "extract_node_stack.h" #include "simhash/minhash.h" #include "semantic/ast_profile.h" #include "tree_sitter/api.h" // TSNode, ts_node_* #include // uint32_t +#include // snprintf +#include // getenv, atoi #include #include @@ -5538,12 +5542,70 @@ typedef struct { const char *enclosing_class_qn; // saved context for class nesting } walk_defs_frame_t; -#define CBM_WALK_DEFS_STACK_CAP 4096 +/* #668: walk_defs previously used a fixed `walk_defs_frame_t stack[4096]` — a + * single ~160 KB C-stack frame. That overflowed small thread stacks (the + * pre-2026-03 Windows 1 MB main thread) on the definitions pass, and its + * `top < 4096` push guards SILENTLY DROPPED every top-level definition past + * 4096. Use a growable heap stack instead: a tiny initial footprint that doubles + * on demand, bounded by a generous, env-configurable ceiling that WARNs (once) + * rather than dropping — so a file with thousands of top-level defs is fully + * extracted, and a pathological one degrades to a warned, bounded skip instead + * of an OOM or a stack overflow. */ +typedef struct { + walk_defs_frame_t *data; + int top; + int cap; + const char *path; // for the WARN when the ceiling is hit (may be NULL) + bool warned; +} wd_stack_t; + +// Generous safety ceiling (frames), env-overridable via CBM_WALK_DEFS_MAX. +// Realistic files never approach this; it only bounds a pathological/adversarial +// file so extraction degrades to a warned skip rather than unbounded memory. +static int wd_stack_max(void) { + const char *e = getenv("CBM_WALK_DEFS_MAX"); + if (e) { + int v = atoi(e); + if (v > 0) { + return v; + } + } + return 8 * 1024 * 1024; // 8M frames (~320 MB) default +} + +static void wd_push(wd_stack_t *s, TSNode node, const char *enclosing_qn) { + if (s->top >= s->cap) { + int ncap = s->cap ? s->cap * 2 : 256; + if (ncap > wd_stack_max()) { + if (!s->warned) { + char lim[24]; + snprintf(lim, sizeof(lim), "%d", wd_stack_max()); + cbm_log_warn("extract.walk_defs_capped", "limit", lim, "path", + s->path ? s->path : ""); + s->warned = true; + } + return; // bounded: stop growing (warned, not silent) + } + walk_defs_frame_t *nd = safe_realloc(s->data, (size_t)ncap * sizeof(walk_defs_frame_t)); + if (!nd) { + /* OOM — safe_realloc already freed the old buffer. Bail cleanly: drop + * pending frames so the walk_defs loop drains and exits without a NULL + * deref; extraction keeps whatever was already emitted. */ + s->data = NULL; + s->cap = 0; + s->top = 0; + return; + } + s->data = nd; + s->cap = ncap; + } + s->data[s->top++] = (walk_defs_frame_t){node, enclosing_qn}; +} // Push nested class nodes from a class body container onto the defs stack. // Iteratively walks into wrapper nodes (field_declaration, template_declaration). -static void push_nested_class_nodes(TSNode body, const CBMLangSpec *spec, walk_defs_frame_t *stack, - int *top, const char *enclosing_qn, CBMArena *arena) { +static void push_nested_class_nodes(TSNode body, const CBMLangSpec *spec, wd_stack_t *s, + const char *enclosing_qn, CBMArena *arena) { TSNodeStack nc_stack; ts_nstack_init(&nc_stack, arena, NESTED_CLASS_STACK_CAP); ts_nstack_push(&nc_stack, arena, body); @@ -5554,9 +5616,7 @@ static void push_nested_class_nodes(TSNode body, const CBMLangSpec *spec, walk_d for (int i = (int)nc - SKIP_CHAR; i >= 0; i--) { TSNode child = ts_node_child(cur, (uint32_t)i); if (cbm_kind_in_set(child, spec->class_node_types)) { - if (*top < CBM_WALK_DEFS_STACK_CAP) { - stack[(*top)++] = (walk_defs_frame_t){child, enclosing_qn}; - } + wd_push(s, child, enclosing_qn); } else { const char *ck = ts_node_type(child); if (strcmp(ck, "field_declaration") == 0 || @@ -5624,8 +5684,8 @@ static const char *compute_class_qn(CBMExtractCtx *ctx, TSNode node, const char } // Push nested class children from a class body container onto the walk stack. -static void push_class_body_children(TSNode node, const CBMLangSpec *spec, walk_defs_frame_t *stack, - int *top, const char *new_enclosing, CBMArena *arena) { +static void push_class_body_children(TSNode node, const CBMLangSpec *spec, wd_stack_t *s, + const char *new_enclosing, CBMArena *arena) { uint32_t nc = ts_node_child_count(node); for (uint32_t ci = 0; ci < nc; ci++) { TSNode child = ts_node_child(node, ci); @@ -5638,13 +5698,13 @@ static void push_class_body_children(TSNode node, const CBMLangSpec *spec, walk_ // double-extracted) as top-level functions. Gated to Groovy so other // grammars that also name a node "closure" are unaffected. (strcmp(ck, "closure") == 0 && spec->language == CBM_LANG_GROOVY)) { - push_nested_class_nodes(child, spec, stack, top, new_enclosing, arena); + push_nested_class_nodes(child, spec, s, new_enclosing, arena); return; } } // No body found — push all children directly - for (int ci = (int)nc - SKIP_CHAR; ci >= 0 && *top < CBM_WALK_DEFS_STACK_CAP; ci--) { - stack[(*top)++] = (walk_defs_frame_t){ts_node_child(node, (uint32_t)ci), new_enclosing}; + for (int ci = (int)nc - SKIP_CHAR; ci >= 0; ci--) { + wd_push(s, ts_node_child(node, (uint32_t)ci), new_enclosing); } } @@ -6030,12 +6090,12 @@ static void recover_kotlin_error_classes(CBMExtractCtx *ctx, TSNode err_node) { static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, int depth_unused) { (void)depth_unused; - walk_defs_frame_t stack[CBM_WALK_DEFS_STACK_CAP]; - int top = 0; - stack[top++] = (walk_defs_frame_t){root, ctx->enclosing_class_qn}; + wd_stack_t s = {0}; + s.path = ctx->rel_path; + wd_push(&s, root, ctx->enclosing_class_qn); - while (top > 0) { - walk_defs_frame_t frame = stack[--top]; + while (s.top > 0) { + walk_defs_frame_t frame = s.data[--s.top]; TSNode node = frame.node; ctx->enclosing_class_qn = frame.enclosing_class_qn; const char *kind = ts_node_type(node); @@ -6072,9 +6132,8 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, // field, double-mint). Push children so nested tags/defs are still // traversed, then skip the generic func path. uint32_t cc = ts_node_child_count(node); - for (int i = (int)cc - SKIP_CHAR; i >= 0 && top < CBM_WALK_DEFS_STACK_CAP; i--) { - stack[top++] = - (walk_defs_frame_t){ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn}; + for (int i = (int)cc - SKIP_CHAR; i >= 0; i--) { + wd_push(&s, ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn); } continue; } @@ -6087,9 +6146,8 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, // still carries the quotes. Push children so nested defines are still // traversed, then skip the generic func path. uint32_t cc = ts_node_child_count(node); - for (int i = (int)cc - SKIP_CHAR; i >= 0 && top < CBM_WALK_DEFS_STACK_CAP; i--) { - stack[top++] = - (walk_defs_frame_t){ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn}; + for (int i = (int)cc - SKIP_CHAR; i >= 0; i--) { + wd_push(&s, ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn); } continue; } @@ -6116,9 +6174,8 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, ts_node_is_null( find_first_descendant_by_kind(node, "func_type", CBM_DESCENDANT_MAX_DEPTH))) { uint32_t cc = ts_node_child_count(node); - for (int i = (int)cc - SKIP_CHAR; i >= 0 && top < CBM_WALK_DEFS_STACK_CAP; i--) { - stack[top++] = - (walk_defs_frame_t){ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn}; + for (int i = (int)cc - SKIP_CHAR; i >= 0; i--) { + wd_push(&s, ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn); } continue; } @@ -6158,8 +6215,8 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, if (is_namespace_scope_kind(ctx->language, kind)) { const char *new_enclosing = compute_class_qn(ctx, node, frame.enclosing_class_qn); uint32_t nsc = ts_node_child_count(node); - for (int i = (int)nsc - SKIP_CHAR; i >= 0 && top < CBM_WALK_DEFS_STACK_CAP; i--) { - stack[top++] = (walk_defs_frame_t){ts_node_child(node, (uint32_t)i), new_enclosing}; + for (int i = (int)nsc - SKIP_CHAR; i >= 0; i--) { + wd_push(&s, ts_node_child(node, (uint32_t)i), new_enclosing); } continue; } @@ -6167,16 +6224,16 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, if (cbm_kind_in_set(node, spec->class_node_types)) { extract_class_def(ctx, node, spec); const char *new_enclosing = compute_class_qn(ctx, node, frame.enclosing_class_qn); - push_class_body_children(node, spec, stack, &top, new_enclosing, ctx->arena); + push_class_body_children(node, spec, &s, new_enclosing, ctx->arena); continue; } uint32_t count = ts_node_child_count(node); - for (int i = (int)count - SKIP_CHAR; i >= 0 && top < CBM_WALK_DEFS_STACK_CAP; i--) { - stack[top++] = - (walk_defs_frame_t){ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn}; + for (int i = (int)count - SKIP_CHAR; i >= 0; i--) { + wd_push(&s, ts_node_child(node, (uint32_t)i), frame.enclosing_class_qn); } } + free(s.data); } void cbm_extract_definitions(CBMExtractCtx *ctx) { diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 7878d0488..36a6e001c 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -3112,6 +3112,33 @@ TEST(extract_non_perl_method_call_not_flagged_is_method) { PASS(); } +/* #668: walk_defs used a fixed `walk_defs_frame_t stack[4096]` — a ~160 KB + * C-stack frame that overflowed small thread stacks (the reporter's crash was in + * the "definitions pass" on a large SQL file), and whose `top < 4096` push guards + * SILENTLY DROPPED every top-level definition past 4096. The growable heap stack + * fixes both: a file with > 4096 top-level defs must extract ALL of them. RED + * before the fix (extracted count capped near 4096), GREEN after. */ +TEST(walk_defs_no_truncation_over_4096_issue668) { + enum { N = 5000 }; + /* N top-level Python defs → N Function defs, all direct module children, so + * walk_defs pushes all N children at once — the >4096 truncation site. */ + size_t cap = (size_t)N * 24 + 1; + char *src = (char *)malloc(cap); + ASSERT_NOT_NULL(src); + size_t off = 0; + for (int i = 0; i < N; i++) { + off += (size_t)snprintf(src + off, cap - off, "def f%d(): pass\n", i); + } + CBMFileResult *r = extract(src, CBM_LANG_PYTHON, "t", "big.py"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + int nfuncs = count_defs_with_label(r, "Function"); + ASSERT(nfuncs >= N); /* all N present — not truncated at the old 4096 cap */ + cbm_free_result(r); + free(src); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════ * Suite * ═══════════════════════════════════════════════════════════════════ */ @@ -3354,6 +3381,7 @@ SUITE(extraction) { RUN_TEST(complexity_recursion_in_loop_unguarded); RUN_TEST(complexity_guarded_recursion); RUN_TEST(complexity_access_depth_and_params); + RUN_TEST(walk_defs_no_truncation_over_4096_issue668); cbm_shutdown(); }