Merge pull request #2373 from DeusData/fix/defs-walk-no-cap

fix(extract): definitions walk stack grows without a ceiling

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 <stdint.h>          // uint32_t
+#include <limits.h>          // INT_MAX
+#include <stdint.h>          // uint32_t, SIZE_MAX
 #include <stdio.h>           // snprintf (ObjectScript storage/trigger sidecars)
-#include <stdlib.h>          // getenv, atoi
+#include <stdlib.h>          // malloc/free (child-collection scratch)
 #include <string.h>
 #include <ctype.h>
 
@@ -7209,74 +7210,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};
 }
@@ -8084,7 +8086,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";
     }
 }
 

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

tests/test_extraction.c

@@ -5480,6 +5480,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)
  * ═══════════════════════════════════════════════════════════════════ */
@@ -8738,6 +8770,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);