Skip to content

fix(extract): AST traversal stacks no longer outlive the file they walk #2013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
shafty023 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(extract): AST traversal stacks no longer outlive the file they walk #2013

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 47 additions & 5 deletions internal/cbm/cbm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/* Full declaration set for the same CBMArena, and it must precede cbm.h:
* internal/cbm/arena.h declares a subset and the two share the CBM_ARENA_H
* guard, so whichever is included first is the one this file sees. */
#include "foundation/arena.h" // cbm_arena_init_sized
#include "cbm.h"
#include "arena.h" // CBMArena, cbm_arena_init/alloc/strdup/destroy
#include "helpers.h"
Expand Down Expand Up @@ -1162,11 +1166,27 @@ CBMFileResult *cbm_extract_file(const char *source, int source_len, CBMLanguage
return r;
}

CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLanguage language,
const char *project, const char *rel_path,
int64_t timeout_micros, const char **extra_defines,
const char **include_paths, const CBMMacroTable *macro_table,
const CBMReturnTypeTable *return_type_table) {
/* Initial block for the per-file traversal scratch arena, chosen by measuring
* arena_grow on a 14k-file TypeScript tree: it fires on one file in 12,000 at
* both this size and at 1 MB, and on most files at 256 KB, where the two
* channel walks alone are exactly 262144 bytes. 512 KB therefore buys the same
* growth behaviour as 1 MB for half the resident block per worker. It is also
* exactly MI_LARGE_MAX_OBJ_SIZE in the vendored mimalloc
* (vendored/mimalloc/include/mimalloc/types.h:426, MI_LARGE_PAGE_SIZE/8 with
* MI_ENABLE_LARGE_PAGES defaulting to 1 at :115 and not overridden here), so
* the block is still bin-allocated from a large page. Growth is not free at
* this size for the same reason: arena_grow doubles to 1 MiB, which is above
* that bound and so a singleton OS allocation. One file in twelve thousand
* pays it, which is why the cost is accepted. */
enum { CBM_EXTRACT_SCRATCH_BLOCK = CBM_SZ_512 * CBM_SZ_1K };

static CBMFileResult *extract_file_ex_body(const char *source, int source_len, CBMLanguage language,
const char *project, const char *rel_path,
int64_t timeout_micros, const char **extra_defines,
const char **include_paths,
const CBMMacroTable *macro_table,
const CBMReturnTypeTable *return_type_table,
CBMArena *scratch) {
// Allocate result on heap (arena inside for all string data)
enum { SINGLE = 1 };
CBMFileResult *result = (CBMFileResult *)calloc(SINGLE, sizeof(CBMFileResult));
Expand Down Expand Up @@ -1277,6 +1297,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
// Build extraction context
CBMExtractCtx ctx = {
.arena = a,
.scratch = scratch,
.result = result,
.source = source,
.source_len = source_len,
Expand Down Expand Up @@ -1398,6 +1419,7 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
// Build context for expanded source — extract only calls via unified extractor
CBMExtractCtx pp_ctx = {
.arena = a,
.scratch = scratch,
.result = result,
.source = expanded,
.source_len = expanded_len,
Expand Down Expand Up @@ -1634,6 +1656,26 @@ CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLangua
return result;
}

/* Public entry. Owns the traversal scratch arena for the whole of one file's
* extraction: created here, handed to the body as ctx->scratch, destroyed on
* the way out. The body has seven early returns, so bracketing it in a wrapper
* is what keeps that to one create and one destroy. If the arena cannot be
* created, the body is handed NULL and the traversal stacks fall back to the
* result arena, which is what shipped before #1997. */
CBMFileResult *cbm_extract_file_ex(const char *source, int source_len, CBMLanguage language,
const char *project, const char *rel_path,
int64_t timeout_micros, const char **extra_defines,
const char **include_paths, const CBMMacroTable *macro_table,
const CBMReturnTypeTable *return_type_table) {
CBMArena scratch;
cbm_arena_init_sized(&scratch, CBM_EXTRACT_SCRATCH_BLOCK);
CBMFileResult *result = extract_file_ex_body(
source, source_len, language, project, rel_path, timeout_micros, extra_defines,
include_paths, macro_table, return_type_table, scratch.nblocks > 0 ? &scratch : NULL);
cbm_arena_destroy(&scratch);
return result;
}

void cbm_free_result(CBMFileResult *result) {
if (!result) {
return;
Expand Down
7 changes: 7 additions & 0 deletions internal/cbm/cbm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,13 @@ typedef struct {

typedef struct {
CBMArena *arena;
/* Scratch for AST traversal, owned by the cbm_extract_file_ex call that
* built this context and destroyed when it returns. Nothing a
* CBMFileResult points at may be allocated here: `arena` is the result's
* own, and it outlives extraction by the whole pipeline (#1997). NULL in a
* context built without one, in which case the stacks fall back to
* `arena`. */
CBMArena *scratch;
CBMFileResult *result;
const char *source;
int source_len;
Expand Down
60 changes: 30 additions & 30 deletions internal/cbm/extract_channels.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ static const char *literal_from_first_child(CBMExtractCtx *ctx, TSNode node) {
* const table per file is sufficient for the common Socket.IO pattern). */
static void scan_string_consts_js(CBMExtractCtx *ctx, chan_const_table_t *tbl) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0 && tbl->count < CHAN_CONST_CAP) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -128,15 +128,15 @@ static void scan_string_consts_js(CBMExtractCtx *ctx, chan_const_table_t *tbl) {
}
}

ts_nstack_push_children(&stack, ctx->arena, node);
ts_nstack_push_children(&stack, node);
}
}

/* Python constant resolution: NAME = "value" (assignment node). */
static void scan_string_consts_python(CBMExtractCtx *ctx, chan_const_table_t *tbl) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0 && tbl->count < CHAN_CONST_CAP) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -163,7 +163,7 @@ static void scan_string_consts_python(CBMExtractCtx *ctx, chan_const_table_t *tb

uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -373,15 +373,15 @@ static void extract_channels_js(CBMExtractCtx *ctx) {

/* Second pass: walk the tree looking for call_expression nodes. */
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
if (strcmp(ts_node_type(node), "call_expression") == 0) {
js_process_call(ctx, node, &consts);
}
ts_nstack_push_children(&stack, ctx->arena, node);
ts_nstack_push_children(&stack, node);
}
}

Expand Down Expand Up @@ -549,8 +549,8 @@ static void extract_channels_python(CBMExtractCtx *ctx) {
scan_string_consts_python(ctx, &consts);

TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -562,7 +562,7 @@ static void extract_channels_python(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -627,8 +627,8 @@ static void go_process_call(CBMExtractCtx *ctx, TSNode call) {

static void extract_channels_go(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -637,7 +637,7 @@ static void extract_channels_go(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -724,8 +724,8 @@ static void java_process_annotation(CBMExtractCtx *ctx, TSNode annotation) {

static void extract_channels_java(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -737,7 +737,7 @@ static void extract_channels_java(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -793,8 +793,8 @@ static void csharp_process_call(CBMExtractCtx *ctx, TSNode call) {

static void extract_channels_csharp(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -803,7 +803,7 @@ static void extract_channels_csharp(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -856,8 +856,8 @@ static void ruby_process_call(CBMExtractCtx *ctx, TSNode call) {

static void extract_channels_ruby(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -866,7 +866,7 @@ static void extract_channels_ruby(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -958,8 +958,8 @@ static void elixir_process_function_def(CBMExtractCtx *ctx, TSNode func_def) {

static void extract_channels_elixir(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -971,7 +971,7 @@ static void extract_channels_elixir(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down Expand Up @@ -1034,8 +1034,8 @@ static void rust_process_call(CBMExtractCtx *ctx, TSNode call) {

static void extract_channels_rust(CBMExtractCtx *ctx) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->arena, ctx->root);
ts_nstack_init(&stack, ctx, CHAN_STACK_CAP);
ts_nstack_push(&stack, ctx->root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -1044,7 +1044,7 @@ static void extract_channels_rust(CBMExtractCtx *ctx) {
}
uint32_t count = ts_node_child_count(node);
for (int i = (int)count - SKIP_ONE; i >= 0; i--) {
ts_nstack_push(&stack, ctx->arena, ts_node_child(node, (uint32_t)i));
ts_nstack_push(&stack, ts_node_child(node, (uint32_t)i));
}
}
}
Expand Down
26 changes: 13 additions & 13 deletions internal/cbm/extract_defs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5054,8 +5054,8 @@ static TSNode emit_elixir_module_class(CBMExtractCtx *ctx, TSNode cur) {
static void extract_elixir_call(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec *spec) {
(void)spec;
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CBM_SZ_64);
ts_nstack_push(&stack, ctx->arena, node);
ts_nstack_init(&stack, ctx, CBM_SZ_64);
ts_nstack_push(&stack, node);

while (stack.count > 0) {
TSNode cur = ts_nstack_pop(&stack);
Expand Down Expand Up @@ -5083,7 +5083,7 @@ static void extract_elixir_call(CBMExtractCtx *ctx, TSNode node, const CBMLangSp
for (int di = (int)dbc - SKIP_CHAR; di >= 0; di--) {
TSNode dchild = ts_node_child(do_block, (uint32_t)di);
if (!ts_node_is_null(dchild) && strcmp(ts_node_type(dchild), "call") == 0) {
ts_nstack_push(&stack, ctx->arena, dchild);
ts_nstack_push(&stack, dchild);
}
}
}
Expand Down Expand Up @@ -6021,8 +6021,8 @@ static void extract_var_names(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec
// Used by YAML, TOML, INI, JSON.
static void walk_variables_iter(CBMExtractCtx *ctx, TSNode root, const CBMLangSpec *spec) {
TSNodeStack stack;
ts_nstack_init(&stack, ctx->arena, CBM_SZ_256);
ts_nstack_push(&stack, ctx->arena, root);
ts_nstack_init(&stack, ctx, CBM_SZ_256);
ts_nstack_push(&stack, root);

while (stack.count > 0) {
TSNode node = ts_nstack_pop(&stack);
Expand All @@ -6046,7 +6046,7 @@ static void walk_variables_iter(CBMExtractCtx *ctx, TSNode root, const CBMLangSp
strcmp(ck, "section") == 0 || strcmp(ck, "object") == 0 ||
strcmp(ck, "array") == 0 || strcmp(ck, "pair") == 0 || strcmp(ck, "element") == 0 ||
strcmp(ck, "content") == 0) {
ts_nstack_push(&stack, ctx->arena, child);
ts_nstack_push(&stack, child);
}
}
}
Expand Down Expand Up @@ -6749,10 +6749,10 @@ static void wd_push_children_reverse(wd_stack_t *s, TSNode node, const char *enc
// 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, wd_stack_t *s,
const char *enclosing_qn, CBMArena *arena) {
const char *enclosing_qn, const CBMExtractCtx *ctx) {
TSNodeStack nc_stack;
ts_nstack_init(&nc_stack, arena, NESTED_CLASS_STACK_CAP);
ts_nstack_push(&nc_stack, arena, body);
ts_nstack_init(&nc_stack, ctx, NESTED_CLASS_STACK_CAP);
ts_nstack_push(&nc_stack, body);

while (nc_stack.count > 0) {
TSNode cur = ts_nstack_pop(&nc_stack);
Expand All @@ -6767,7 +6767,7 @@ static void push_nested_class_nodes(TSNode body, const CBMLangSpec *spec, wd_sta
const char *ck = ts_node_type(child);
if (strcmp(ck, "field_declaration") == 0 ||
strcmp(ck, "template_declaration") == 0 || strcmp(ck, "declaration") == 0) {
ts_nstack_push(&nc_stack, arena, child);
ts_nstack_push(&nc_stack, child);
}
}
}
Expand Down Expand Up @@ -6883,7 +6883,7 @@ static void extract_typescript_namespace_def(CBMExtractCtx *ctx, TSNode node,

// Push nested class children from a class body container onto the walk stack.
static void push_class_body_children(TSNode node, const CBMLangSpec *spec, wd_stack_t *s,
const char *new_enclosing, CBMArena *arena) {
const char *new_enclosing, const CBMExtractCtx *ctx) {
/* Use the same language-aware body selection as method extraction. The old
* independent spelling list omitted valid containers such as Scala's
* `template_body` and Solidity's contract body. Methods were extracted
Expand All @@ -6899,7 +6899,7 @@ static void push_class_body_children(TSNode node, const CBMLangSpec *spec, wd_st
body = find_class_member_body(node, spec->language);
}
if (!ts_node_is_null(body)) {
push_nested_class_nodes(body, spec, s, new_enclosing, arena);
push_nested_class_nodes(body, spec, s, new_enclosing, ctx);
return;
}

Expand Down Expand Up @@ -7423,7 +7423,7 @@ 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, &s, new_enclosing, ctx->arena);
push_class_body_children(node, spec, &s, new_enclosing, ctx);
continue;
}

Expand Down
Loading
Loading