From 77ab4bea87008547acee60435a8e3df29d90a5cb Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sat, 26 Sep 2026 20:59:00 +0200 Subject: [PATCH] fix(extract): definitions walk stack grows without a ceiling walk_defs keeps its pending frames in a growable stack that was bounded by an 8M-frame ceiling (env CBM_WALK_DEFS_MAX). At the ceiling wd_push logged "extract.walk_defs_capped" once and then stopped pushing. Children are pushed last-to-first, so a file wider than the ceiling lost its FIRST definitions: a work cap decided graph content. Fix: no ceiling. The stack doubles on demand, heap frames now come from the memory core (cbm_realloc/cbm_free, class EXTRACT) instead of safe_realloc and raw free, and the doubling is overflow-guarded (int capacity and size_t byte count). The only thing that can stop the walk is an allocation failure, and that is loud: logged at error level as "extract.walk_stack_alloc_failed" (walker=walk_defs, pending, path) -- the same event the complexity walkers use -- and the file's result is marked has_error with "definitions walk: stack allocation failed", which the definitions passes already report as a per-file extract error. The arena-backed path (ctx->scratch) gets the same failure handling. Traversal order is unchanged. CBM_WALK_DEFS_MAX is removed. It was read only in wd_stack_max(); it was not documented anywhere (docs, --help, config, tests), so nothing else changes. extract_defs.c loses its raw free() call, so the memory-core baseline for the file drops from 5 to 4. Test (extraction suite): walk_defs_wide_file_extracts_every_definition extracts a C file with 1000 top-level functions with CBM_WALK_DEFS_MAX=256 set, and asserts every wd0..wd999 definition is present. RED on main (extract.walk_defs_capped limit=256; wd0 missing), GREEN with the fix, RED again with the production change reverted. The extraction (379), complexity (5), ast_profile (10) and semantic (35) suites pass. Real input: Linux kernel, index-only, fresh cache and runtime dirs, same machine. main and fix both produce 8,530,336 nodes and 15,764,128 edges, SEMANTICALLY_RELATED 607 each, and the full node set (label, QN, lines) and edge set (type, source QN, target QN) hash identically -- no kernel file approaches the old ceiling. Index wall time 210.0 s (main) vs 194.4 s (fix), within run-to-run noise. Signed-off-by: Martin Vogel --- internal/cbm/extract_defs.c | 114 ++++++++++++++++--------------- scripts/memory-core-baseline.txt | 2 +- tests/test_extraction.c | 33 +++++++++ 3 files changed, 94 insertions(+), 55 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 916f1136f..388f460b0 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3,15 +3,16 @@ #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 "foundation/log.h" // cbm_log_error +#include "foundation/mem_core.h" // cbm_realloc/cbm_free -- walk_defs stack #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 // INT_MAX +#include // uint32_t, SIZE_MAX #include // snprintf (ObjectScript storage/trigger sidecars) -#include // getenv, atoi +#include // malloc/free (child-collection scratch) #include #include @@ -7195,74 +7196,75 @@ typedef struct { * 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. */ + * 4096. It became a growable stack bounded by an 8M-frame ceiling + * (env CBM_WALK_DEFS_MAX) that still stopped pushing once reached — and since + * children are pushed last-to-first, a file wider than the ceiling lost its + * FIRST definitions. No work cap may decide graph content, so there is no + * ceiling: the stack doubles on demand, and only an allocation failure stops + * the walk. That failure is loud — logged at error level and reported as a + * per-file extract error (result->has_error) — never a silent partial file. */ 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; + const char *path; // for the error log on allocation failure (may be NULL) + bool failed; // growth failed: pending frames dropped, walk drains /* The per-file traversal scratch (ctx->scratch) when there is one: frames * then come from memory the thread reuses file after file, where a malloc * of 256 frames per file was 28 k allocations and 255 MB never written on * the Go corpus (waste sanitizer, 2026-09-17). Growth copies into a * doubled buffer and abandons the old one to the arena, like TSNodeStack. - * NULL: the heap, freed by the walk. */ + * NULL: the memory core (class EXTRACT), released by the walk. */ CBMArena *arena; } 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 -} +enum { WD_STACK_INITIAL = 256 }; -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 = NULL; +/* Double `s` (or give it its first WD_STACK_INITIAL frames). Returns false, + * after logging, when the doubled size is not representable or cannot be + * allocated; the old buffer is left intact so the walk can still release it. */ +static bool wd_grow(wd_stack_t *s) { + walk_defs_frame_t *nd = NULL; + int ncap = WD_STACK_INITIAL; + bool fits = true; + if (s->cap > 0) { + fits = s->cap <= INT_MAX / 2; + ncap = fits ? s->cap * 2 : s->cap; + } + fits = fits && (size_t)ncap <= SIZE_MAX / sizeof(walk_defs_frame_t); + if (fits) { + size_t bytes = (size_t)ncap * sizeof(walk_defs_frame_t); if (s->arena) { - nd = (walk_defs_frame_t *)cbm_arena_alloc(s->arena, - (size_t)ncap * sizeof(walk_defs_frame_t)); + nd = (walk_defs_frame_t *)cbm_arena_alloc(s->arena, bytes); if (nd && s->top > 0) { memcpy(nd, s->data, (size_t)s->top * sizeof(walk_defs_frame_t)); } } else { - nd = safe_realloc(s->data, (size_t)ncap * sizeof(walk_defs_frame_t)); + nd = (walk_defs_frame_t *)cbm_realloc(CBM_MEM_CLASS_EXTRACT, s->data, bytes); } - 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; + } + if (!nd) { + char pending[24]; + snprintf(pending, sizeof(pending), "%d", s->top); + cbm_log_error("extract.walk_stack_alloc_failed", "walker", "walk_defs", "pending", pending, + "path", s->path ? s->path : ""); + return false; + } + s->data = nd; + s->cap = ncap; + return true; +} + +static void wd_push(wd_stack_t *s, TSNode node, const char *enclosing_qn) { + if (s->failed) { + return; + } + if (s->top >= s->cap && !wd_grow(s)) { + /* Drop every pending frame so the walk_defs loop drains and exits; + * walk_defs then marks the file's result as an extract error. */ + s->failed = true; + s->top = 0; + return; } s->data[s->top++] = (walk_defs_frame_t){node, enclosing_qn}; } @@ -8070,7 +8072,11 @@ static void walk_defs(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec, wd_push_children_reverse(&s, node, frame.enclosing_class_qn); } if (!s.arena) { - free(s.data); + cbm_free(CBM_MEM_CLASS_EXTRACT, s.data); + } + if (s.failed) { + ctx->result->has_error = true; + ctx->result->error_msg = "definitions walk: stack allocation failed"; } } diff --git a/scripts/memory-core-baseline.txt b/scripts/memory-core-baseline.txt index ee54b498b..fe1cb426c 100644 --- a/scripts/memory-core-baseline.txt +++ b/scripts/memory-core-baseline.txt @@ -6,7 +6,7 @@ internal/cbm/ac.c 20 internal/cbm/arena.c 3 internal/cbm/cbm.c 13 -internal/cbm/extract_defs.c 5 +internal/cbm/extract_defs.c 4 internal/cbm/helpers.c 3 internal/cbm/lsp/c_lsp.c 4 internal/cbm/lsp/perl_lsp.c 21 diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 1e6506ed1..c1797b8a1 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -5380,6 +5380,38 @@ TEST(complexity_access_depth_and_params) { PASS(); } +/* The definitions walk kept its pending frames under a ceiling (8M frames, + * env CBM_WALK_DEFS_MAX) and stopped pushing once it was reached. Children are + * pushed last-to-first, so a file wider than the ceiling lost its FIRST + * top-level definitions. The ceiling and its env knob are gone; the knob is set + * here to a value this file exceeds to prove it no longer decides content. */ +enum { WIDE_DEFS = 1000, WIDE_DEFS_OLD_CAP = 256 }; + +TEST(walk_defs_wide_file_extracts_every_definition) { + size_t cap = (size_t)WIDE_DEFS * 32 + 64; + char *src = malloc(cap); + ASSERT_NOT_NULL(src); + size_t pos = 0; + for (int i = 0; i < WIDE_DEFS; i++) { + pos += (size_t)snprintf(src + pos, cap - pos, "int wd%d(void) { return %d; }\n", i, i); + } + char lim[16]; + snprintf(lim, sizeof(lim), "%d", WIDE_DEFS_OLD_CAP); + cbm_setenv("CBM_WALK_DEFS_MAX", lim, 1); + CBMFileResult *r = extract(src, CBM_LANG_C, "t", "wide_defs.c"); + cbm_unsetenv("CBM_WALK_DEFS_MAX"); + ASSERT_NOT_NULL(r); + ASSERT_FALSE(r->has_error); + for (int i = 0; i < WIDE_DEFS; i++) { + char name[16]; + snprintf(name, sizeof(name), "wd%d", i); + ASSERT_NOT_NULL(find_def(r, name)); + } + cbm_free_result(r); + free(src); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════ * Perl call-graph noise (#459 follow-up) * ═══════════════════════════════════════════════════════════════════ */ @@ -8635,6 +8667,7 @@ SUITE(extraction) { RUN_TEST(complexity_go_method_receiver_self_recursion); RUN_TEST(complexity_delegation_receivers_not_recursive_issue876); RUN_TEST(complexity_access_depth_and_params); + RUN_TEST(walk_defs_wide_file_extracts_every_definition); RUN_TEST(extract_c_ifdef_split_brace_fn_recovered_issue961); RUN_TEST(extract_cpp_preproc_signature_gap_issue946); RUN_TEST(extract_cpp_preproc_macro_generated_callable_skipped_issue949);