Skip to content

Commit 97517a4

Browse files
committed
fix(python): carry bare-call parameter bindings in the walk
Replaces the cursor ascent from 0b0d143. That commit fixed a real hang -- ts_node_parent() is O(depth), so a parent chain is O(depth^2) per call and O(depth^3) across a file where every level of f(f(f(...))) is itself a bare call, which hung stack_overflow_b's 30,000-deep fixture at 900s. But it fixed it by making each hop cheap rather than by removing the per-call walk, so the cost stayed O(depth) per call and needed a 64-ancestor cap to stay bounded. That cap FAILS OPEN. Past 64 ancestors the guard stopped suppressing, so deep-but-ordinary code silently kept the fabricated edges the guard exists to remove -- and nothing failed when it did. Carry the answer instead. The unified walk binds a def's or lambda's parameters when it opens that scope and unwinds them when it closes, so the guard is an O(1) map lookup. This is the lesson CBMWalkScope already records one struct away: carry walk state, never recompute it per node. - CBMParamSlot is a name -> active-count map, not a set. `def outer(run): def inner(run):` binds one name twice and leaving the inner scope must not unbind the outer. - Bindings are pushed at the end of push_boundary_scopes, after every scope push for that node, so they land in the frame just pushed -- covering function_definition (push_function_scope) and lambda (push_lexical_boundary) alike -- and pop_expired_scopes unwinds to the frame's entry height. - Allocation failure sets py_param_tracking_failed and the lookup answers false forever after: a lost suppression, never a lost edge. The repo's other binding table cannot serve this. CBMLexicalBinding is qsort-ed in cbm_finalize_lexical_usages AFTER the walk and its active_end stays 0 until then, so its binary search is invalid from handle_calls, which runs mid-walk and before handle_usages. extract_python_bare_call_scope_walk_is_bounded pinned the cap's fail-open contract, which no longer exists, and is replaced by extract_python_bare_call_flag_is_depth_independent. It keeps the shallow and 200-ancestor cases -- the deep one now asserts the call IS flagged, so reintroducing a cap fails it -- and adds an unwind case: `handler` as both a parameter and a module-level def, asserting the two shadowed calls are flagged and the sibling call after that scope closed is NOT. A binding outliving its frame would destroy that true edge, the one direction this guard must never fail in. It passed on the first run. extraction 327 passed; pipeline registry parallel complexity 407 passed; stack_overflow_b 6 passed in 38s (46s with the cursor ascent, 900s rc=124 before it), lsp_python_deep_nesting_no_crash PASS. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 63f7936 commit 97517a4

4 files changed

Lines changed: 353 additions & 156 deletions

File tree

‎internal/cbm/extract_calls.c‎

Lines changed: 17 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -2970,121 +2970,34 @@ static bool python_receiver_is_exempt(CBMExtractCtx *ctx, TSNode receiver) {
29702970
* a bare `identifier`, the `name` field of default/typed parameters, and the
29712971
* identifier under a `*args` / `**kwargs` splat. A shape with no identifier (the
29722972
* bare `*` keyword separator) yields NULL and simply matches nothing. */
2973-
static const char *python_parameter_name(CBMExtractCtx *ctx, TSNode param) {
2974-
if (ts_node_is_null(param)) {
2975-
return NULL;
2976-
}
2977-
if (strcmp(ts_node_type(param), "identifier") == 0) {
2978-
return cbm_node_text(ctx->arena, param, ctx->source);
2979-
}
2980-
TSNode name = ts_node_child_by_field_name(param, TS_FIELD("name"));
2981-
if (!ts_node_is_null(name) && strcmp(ts_node_type(name), "identifier") == 0) {
2982-
return cbm_node_text(ctx->arena, name, ctx->source);
2983-
}
2984-
/* `*args` / `**kwargs`, and any typed shape without a `name` field: the
2985-
* bound identifier is the first named child. */
2986-
TSNode first = ts_node_named_child(param, 0);
2987-
if (!ts_node_is_null(first) && strcmp(ts_node_type(first), "identifier") == 0) {
2988-
return cbm_node_text(ctx->arena, first, ctx->source);
2989-
}
2990-
return NULL;
2991-
}
2992-
29932973
/* True when the callee of a BARE Python call `foo()` is bound as a parameter of
2994-
* an enclosing function or lambda — the bare-call counterpart of
2974+
* an enclosing function or lambda -- the bare-call counterpart of
29952975
* python_receiver_is_exempt above.
29962976
*
29972977
* A parameter binding shadows any module-level `foo` for the whole body, so
29982978
* resolving such a call to a project Function/Method by short name alone
29992979
* fabricates the edge BY CONSTRUCTION: `def _run_with_heavy_slot(run): run()`
30002980
* must not bind an unrelated `SatoriLive.run`. Unlike a receiver type this is
30012981
* decidable from the AST outright, with no flow analysis and no list of
3002-
* "generic-looking" callee names — Python forbids `global` on a parameter, and a
3003-
* parameter is in scope for the entire body regardless of position, so there is
3004-
* no ordering subtlety to get wrong.
3005-
*
3006-
* Enclosing scopes are walked to the file root so a closure over an outer
3007-
* parameter counts (`def outer(run): def inner(): return run()`).
2982+
* "generic-looking" callee names -- Python forbids `global` on a parameter, and
2983+
* a parameter is in scope for the entire body regardless of position.
30082984
*
3009-
* LOCAL ASSIGNMENTS are deliberately NOT covered. They are flow- and
3010-
* binding-form-sensitive (`for`, `with ... as`, `except ... as`, `:=`,
3011-
* unpacking, plus `global`/`nonlocal` overrides), so a partial body scan would
3012-
* suppress the wrong edges invisibly — the same failure mode that rules out a
3013-
* hardcoded name list. Parameters alone already cover the Callable-parameter
3014-
* shape that motivated this guard. Cost is O(enclosing depth x params) per bare
3015-
* call, never the corpus.
2985+
* The answer is CARRIED BY THE WALK rather than recomputed here. The unified
2986+
* walk binds a def's or lambda's parameters when it opens that scope and
2987+
* unwinds them when it closes it, so this is an O(1) map lookup. Deciding it
2988+
* by ascending the tree instead -- with ts_node_parent() or with a copied walk
2989+
* cursor -- costs O(depth) per call, and since every level of f(f(f(...))) is
2990+
* itself a bare call, that is quadratic across the file: the 30,000-deep
2991+
* fixture in tests/test_stack_overflow.c turned it into a hang rather than a
2992+
* slowdown. An O(1) lookup needs no hop cap, so unlike a capped walk this can
2993+
* no longer fail open on deep-but-ordinary code.
30162994
*
3017-
* Known and accepted: `def inner(): global run; return run()` nested in a
3018-
* function whose parameter is `run` is still flagged. Detecting it needs exactly
3019-
* the body scan this helper avoids, and it costs one edge in a shape that
3020-
* essentially does not occur. */
3021-
static bool python_callee_is_bound_parameter(CBMExtractCtx *ctx, WalkState *state, TSNode call_node,
2995+
* It still answers false if the walk's own tracking hit an allocation failure,
2996+
* which costs a suppression and never a true edge. */
2997+
static bool python_callee_is_bound_parameter(CBMExtractCtx *ctx, WalkState *state,
30222998
TSNode callee_ident) {
3023-
/* Ancestors are walked with the UNIFIED WALK'S OWN CURSOR, not ts_node_parent().
3024-
*
3025-
* ts_node_parent() is not O(1): it restarts at the tree root and descends to
3026-
* find the parent (vendored ts_runtime/src/node.c), so it costs O(depth) per
3027-
* hop. A parent-chain walk is therefore O(depth^2) per call, and since every
3028-
* level of f(f(f(...))) is ITSELF a bare call, O(depth^3) across the file --
3029-
* tests/test_stack_overflow.c nests 30,000 deep, which is a hang, not a
3030-
* slowdown. Capping the hop COUNT does not fix that, because the cost is per
3031-
* hop; the walk itself has to be cheap.
3032-
*
3033-
* ts_tree_cursor_goto_parent() IS O(1) -- the cursor carries its path stack --
3034-
* so copying the walk cursor and ascending costs nothing per hop. Same reason
3035-
* CBMWalkScope keeps a frame stack instead of recomputing enclosing state, and
3036-
* same current_cursor idiom (including the identity guard) as
3037-
* usage_current_field_name in extract_usages.c.
3038-
*
3039-
* The hop cap then bounds the remaining O(depth) per call. 64 rather than
3040-
* Lean's 20 above: that walk looks for an IMMEDIATE declaration boundary,
3041-
* while this one crosses whatever statement/expression nesting separates a
3042-
* call from its enclosing def, so it needs headroom before it can bite on
3043-
* ordinary code.
3044-
*
3045-
* Both the cap and a missing/mismatched cursor FAIL OPEN -- return false, do
3046-
* NOT suppress. A guard whose whole justification is precision must never
3047-
* destroy an edge it did not actually prove was fabricated, so either can
3048-
* only ever cost a suppression, never a true edge. Pinned in
3049-
* test_extraction.c. */
3050-
enum { PY_MAX_SCOPE_WALK_DEPTH = 64 };
3051-
30522999
const char *callee_name = cbm_node_text(ctx->arena, callee_ident, ctx->source);
3053-
if (!callee_name || !callee_name[0]) {
3054-
return false;
3055-
}
3056-
/* Only trust the shared cursor when it is actually parked on this call. */
3057-
if (!state || !state->current_cursor ||
3058-
!ts_node_eq(ts_tree_cursor_current_node(state->current_cursor), call_node)) {
3059-
return false;
3060-
}
3061-
3062-
TSTreeCursor up = ts_tree_cursor_copy(state->current_cursor);
3063-
bool bound = false;
3064-
for (int walked = 0; walked < PY_MAX_SCOPE_WALK_DEPTH && !bound; walked++) {
3065-
if (!ts_tree_cursor_goto_parent(&up)) {
3066-
break;
3067-
}
3068-
TSNode scope = ts_tree_cursor_current_node(&up);
3069-
const char *kind = ts_node_type(scope);
3070-
if (strcmp(kind, "function_definition") != 0 && strcmp(kind, "lambda") != 0) {
3071-
continue;
3072-
}
3073-
TSNode params = ts_node_child_by_field_name(scope, TS_FIELD("parameters"));
3074-
if (ts_node_is_null(params)) {
3075-
continue;
3076-
}
3077-
uint32_t count = ts_node_named_child_count(params);
3078-
for (uint32_t i = 0; i < count; i++) {
3079-
const char *pname = python_parameter_name(ctx, ts_node_named_child(params, i));
3080-
if (pname && strcmp(pname, callee_name) == 0) {
3081-
bound = true;
3082-
break;
3083-
}
3084-
}
3085-
}
3086-
ts_tree_cursor_delete(&up);
3087-
return bound;
3000+
return cbm_walk_python_param_is_bound(state, callee_name);
30883001
}
30893002

30903003
static bool is_objectscript_language(CBMLanguage language) {
@@ -3722,8 +3635,7 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML
37223635
TSNode obj = ts_node_child_by_field_name(fn, TS_FIELD("object"));
37233636
call.is_method = !python_receiver_is_exempt(ctx, obj);
37243637
} else if (!ts_node_is_null(fn) && strcmp(ts_node_type(fn), "identifier") == 0) {
3725-
call.callee_is_locally_bound =
3726-
python_callee_is_bound_parameter(ctx, state, node, fn);
3638+
call.callee_is_locally_bound = python_callee_is_bound_parameter(ctx, state, fn);
37273639
}
37283640
}
37293641
// TS/JS/TSX receiver-aware guard (#592/#606 direction; same intent

‎internal/cbm/extract_unified.c‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,196 @@ static uint32_t add_lexical_scope(WalkState *state, TSNode node, CBMLexicalScope
119119
return scope->id;
120120
}
121121

122+
/* ── #1912: Python parameter bindings held live by the walk ───────────────────
123+
*
124+
* A bare Python call `run()` cannot honestly resolve to a project function by
125+
* short name when `run` is bound as a parameter of an enclosing def or lambda:
126+
* the parameter shadows any module-level `run` for the whole body, so the match
127+
* is fabricated by construction.
128+
*
129+
* Deciding that per call by ASCENDING the tree is the trap this replaces. Both
130+
* ts_node_parent() (O(depth) per hop) and a walk-cursor ascent (O(1) per hop,
131+
* O(depth) per call) are per-call costs, and every level of f(f(f(...))) is
132+
* itself a bare call, so the file-wide cost is quadratic or worse -- exactly
133+
* what CBMWalkScope's comment records for the state it replaced. Scanning the
134+
* frame stack has the same shape.
135+
*
136+
* So the walk carries the answer instead: a name -> active-count map, pushed
137+
* when a Python function or lambda scope opens and unwound when it closes.
138+
* Lookup is O(1), which is what removes the need for a hop cap.
139+
*
140+
* A COUNT, not a flag: `def outer(run): def inner(run):` binds one name twice,
141+
* and leaving the inner scope must not unbind the outer one.
142+
*
143+
* Every failure path answers "not bound", which can only ever cost a
144+
* suppression -- never a true edge. */
145+
146+
static uint32_t py_param_hash(const char *s) {
147+
uint32_t h = 2166136261u; /* FNV-1a */
148+
for (const unsigned char *p = (const unsigned char *)s; *p; p++) {
149+
h = (h ^ *p) * 16777619u;
150+
}
151+
return h ? h : 1u; /* 0 marks an empty slot */
152+
}
153+
154+
static CBMParamSlot *py_param_slot(WalkState *state, const char *name, uint32_t hash) {
155+
int mask = state->py_param_slot_capacity - 1;
156+
int i = (int)(hash & (uint32_t)mask);
157+
for (;;) {
158+
CBMParamSlot *slot = &state->py_param_slots[i];
159+
if (slot->hash == 0) {
160+
return slot; /* free slot -- caller decides whether to claim it */
161+
}
162+
if (slot->hash == hash && strcmp(slot->name, name) == 0) {
163+
return slot;
164+
}
165+
i = (i + 1) & mask;
166+
}
167+
}
168+
169+
static bool py_param_grow(WalkState *state) {
170+
int new_capacity = state->py_param_slot_capacity * PAIR_LEN;
171+
if (new_capacity <= state->py_param_slot_capacity) {
172+
return false;
173+
}
174+
CBMParamSlot *grown =
175+
(CBMParamSlot *)cbm_arena_alloc(state->arena, (size_t)new_capacity * sizeof(*grown));
176+
if (!grown) {
177+
return false;
178+
}
179+
memset(grown, 0, (size_t)new_capacity * sizeof(*grown));
180+
CBMParamSlot *old = state->py_param_slots;
181+
int old_capacity = state->py_param_slot_capacity;
182+
state->py_param_slots = grown;
183+
state->py_param_slot_capacity = new_capacity;
184+
for (int i = 0; i < old_capacity; i++) {
185+
if (old[i].hash != 0) {
186+
*py_param_slot(state, old[i].name, old[i].hash) = old[i];
187+
}
188+
}
189+
return true;
190+
}
191+
192+
static bool py_param_stack_reserve(WalkState *state) {
193+
if (state->py_param_stack_count < state->py_param_stack_capacity) {
194+
return true;
195+
}
196+
int new_capacity = state->py_param_stack_capacity * PAIR_LEN;
197+
if (new_capacity <= state->py_param_stack_capacity) {
198+
return false;
199+
}
200+
const char **grown =
201+
(const char **)cbm_arena_alloc(state->arena, (size_t)new_capacity * sizeof(*grown));
202+
if (!grown) {
203+
return false;
204+
}
205+
memcpy(grown, state->py_param_stack, (size_t)state->py_param_stack_count * sizeof(*grown));
206+
state->py_param_stack = grown;
207+
state->py_param_stack_capacity = new_capacity;
208+
return true;
209+
}
210+
211+
/* Bind one parameter name for the lifetime of the frame currently on top. */
212+
static void py_param_bind(WalkState *state, const char *name) {
213+
if (state->py_param_tracking_failed || !name || !name[0]) {
214+
return;
215+
}
216+
/* Grow before inserting: the probe below must always find a free slot, and
217+
* a table above ~70% load degrades toward linear probing. */
218+
if ((state->py_param_slot_used + 1) * 10 >= state->py_param_slot_capacity * 7) {
219+
if (!py_param_grow(state)) {
220+
state->py_param_tracking_failed = true;
221+
return;
222+
}
223+
}
224+
if (!py_param_stack_reserve(state)) {
225+
state->py_param_tracking_failed = true;
226+
return;
227+
}
228+
uint32_t hash = py_param_hash(name);
229+
CBMParamSlot *slot = py_param_slot(state, name, hash);
230+
if (slot->hash == 0) {
231+
slot->hash = hash;
232+
slot->name = name;
233+
slot->count = 0;
234+
state->py_param_slot_used++;
235+
}
236+
slot->count++;
237+
state->py_param_stack[state->py_param_stack_count++] = name;
238+
}
239+
240+
/* Unwind to a frame's entry height, undoing exactly what that frame bound. */
241+
static void py_param_unwind_to(WalkState *state, int base) {
242+
if (state->py_param_tracking_failed) {
243+
return;
244+
}
245+
while (state->py_param_stack_count > base) {
246+
const char *name = state->py_param_stack[--state->py_param_stack_count];
247+
CBMParamSlot *slot = py_param_slot(state, name, py_param_hash(name));
248+
if (slot->hash != 0 && slot->count > 0) {
249+
slot->count--;
250+
}
251+
}
252+
}
253+
254+
bool cbm_walk_python_param_is_bound(const WalkState *state, const char *name) {
255+
if (!state || !name || !name[0] || state->py_param_tracking_failed ||
256+
state->py_param_slot_used == 0) {
257+
return false;
258+
}
259+
const CBMParamSlot *slot = py_param_slot((WalkState *)state, name, py_param_hash(name));
260+
return slot->hash != 0 && slot->count > 0;
261+
}
262+
263+
/* The identifier a Python parameter node binds. Handles the bare, typed,
264+
* defaulted, keyword-only, *args and **kwargs shapes. */
265+
static const char *py_parameter_name(CBMExtractCtx *ctx, TSNode param) {
266+
if (ts_node_is_null(param)) {
267+
return NULL;
268+
}
269+
if (strcmp(ts_node_type(param), "identifier") == 0) {
270+
return cbm_node_text(ctx->arena, param, ctx->source);
271+
}
272+
TSNode name = ts_node_child_by_field_name(param, TS_FIELD("name"));
273+
if (!ts_node_is_null(name) && strcmp(ts_node_type(name), "identifier") == 0) {
274+
return cbm_node_text(ctx->arena, name, ctx->source);
275+
}
276+
/* `*args` / `**kwargs`, and any typed shape without a `name` field: the
277+
* bound identifier is the first named child. */
278+
TSNode first = ts_node_named_child(param, 0);
279+
if (!ts_node_is_null(first) && strcmp(ts_node_type(first), "identifier") == 0) {
280+
return cbm_node_text(ctx->arena, first, ctx->source);
281+
}
282+
return NULL;
283+
}
284+
285+
/* Bind this node's parameters into the frame just pushed for it. Called after
286+
* every push in push_boundary_scopes, so the top frame is this node's own and
287+
* pop_expired_scopes unwinds them at exactly the right depth. */
288+
static void py_bind_scope_parameters(CBMExtractCtx *ctx, TSNode node, WalkState *state) {
289+
if (ctx->language != CBM_LANG_PYTHON || state->scope_top == 0) {
290+
return;
291+
}
292+
const char *kind = ts_node_type(node);
293+
if (strcmp(kind, "function_definition") != 0 && strcmp(kind, "lambda") != 0) {
294+
return;
295+
}
296+
TSNode params = ts_node_child_by_field_name(node, TS_FIELD("parameters"));
297+
if (ts_node_is_null(params)) {
298+
return;
299+
}
300+
uint32_t count = ts_node_named_child_count(params);
301+
for (uint32_t i = 0; i < count; i++) {
302+
py_param_bind(state, py_parameter_name(ctx, ts_node_named_child(params, i)));
303+
}
304+
}
305+
122306
static bool push_scope(WalkState *state, uint8_t kind, uint32_t depth, const char *qn) {
123307
if (!ensure_scope_capacity(state)) {
124308
return false;
125309
}
126310
CBMWalkScope *f = &state->scopes[state->scope_top];
311+
f->prev_py_param_stack_count = state->py_param_stack_count;
127312
f->kind = kind;
128313
f->depth = depth;
129314
f->qn = qn;
@@ -260,6 +445,7 @@ static void pop_expired_scopes(WalkState *state, uint32_t cur_depth) {
260445
state->inside_import = f->prev_inside_import;
261446
state->loop_depth = f->prev_loop_depth;
262447
state->branch_depth = f->prev_branch_depth;
448+
py_param_unwind_to(state, f->prev_py_param_stack_count);
263449
}
264450
}
265451

@@ -2354,6 +2540,7 @@ static void push_boundary_scopes(CBMExtractCtx *ctx, TSNode node, const CBMLangS
23542540
}
23552541

23562542
push_lexical_boundary(node, state, depth);
2543+
py_bind_scope_parameters(ctx, node, state);
23572544
push_call_scope(state, depth, invocation);
23582545
if (is_actual_import_boundary(ctx, node, spec) && !is_export_of_declaration(node)) {
23592546
push_scope(state, SCOPE_IMPORT, depth, NULL);
@@ -2387,6 +2574,14 @@ void cbm_extract_unified(CBMExtractCtx *ctx) {
23872574
state.scope_capacity = MAX_SCOPES;
23882575
state.lexical_scopes = state.inline_lexical_scopes;
23892576
state.lexical_scope_capacity = INLINE_LEXICAL_SCOPES;
2577+
state.py_param_slots = state.inline_py_param_slots;
2578+
state.py_param_slot_capacity = INLINE_PY_PARAM_SLOTS;
2579+
state.py_param_slot_used = 0;
2580+
memset(state.inline_py_param_slots, 0, sizeof(state.inline_py_param_slots));
2581+
state.py_param_stack = state.inline_py_param_stack;
2582+
state.py_param_stack_capacity = INLINE_PY_PARAM_STACK;
2583+
state.py_param_stack_count = 0;
2584+
state.py_param_tracking_failed = false;
23902585
state.usage_start_index = ctx->result->usages.count;
23912586
state.root_lexical_scope_id = add_lexical_scope(&state, ctx->root, CBM_LEXICAL_SCOPE_MODULE);
23922587
/* Base walk-state tuple (previously established by the first

0 commit comments

Comments
 (0)