Skip to content

Commit 0b0d143

Browse files
fix(python): walk bare-call scopes with the walk cursor, bounded
DeusData#1912 hung stack_overflow_b on all seven CI legs: rc=124 at 900s with zero tests completed, on lsp_python_deep_nesting_no_crash. The bare-call scope walk used ts_node_parent(), which is NOT O(1) -- it restarts at the tree root and descends to find the parent (vendored ts_runtime/src/node.c). Each hop therefore costs O(depth), a parent chain is O(depth^2) per call, and since every level of f(f(f(...))) is itself a bare call, O(depth^3) across the file. The fixture nests 30,000 deep, so this is a hang rather than a slowdown. A hop-count cap alone is NOT sufficient, and this was measured rather than assumed: with a 64-hop cap the suite still timed out, because 64 x O(30,000) x 30,000 calls is still ~5.7e10. When the cost is per hop, bounding the number of hops cannot fix it -- the walk itself has to be cheap. Use the unified walk's own cursor. WalkState.current_cursor is already parked on the current node and ts_tree_cursor_goto_parent() IS O(1), because the cursor carries its path stack, so ascending costs nothing per hop. Same current_cursor idiom, including the ts_node_eq identity guard before trusting the shared cursor, as usage_current_field_name in extract_usages.c. This is the same lesson CBMWalkScope already records: carry walk state, never recompute it per node. The 64-hop cap is retained and now does real work, bounding the remaining O(depth) per call. Precedent for the shape: CBM_LSP_PERL_MAX_WALK_DEPTH and LEAN_MAX_PARENT_DEPTH in this file. Both the cap and a missing or mismatched cursor FAIL OPEN -- they can only ever cost a suppression, never a true edge, which is the safe direction for a guard whose justification is precision. lsp_python_deep_nesting_no_crash: 900s timeout, 0 completions -> PASS. stack_overflow_b overall: 6 passed in 46s. The regression test pins the cap's contract deterministically rather than by wall clock: within the cap a shadowed callee is flagged; past 64 ancestors the guard fails open and leaves it unflagged. Raising the cap flips the deep case and fails the test, verified. 826 passed / 0 failed across stack_overflow_a stack_overflow_b stack_overflow_c pipeline registry parallel extraction complexity lsp_resolution_probe. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-authored-by: Joseph-MingEn <125283161+Joseph-MingEn@users.noreply.github.com>
1 parent 8bcfa29 commit 0b0d143

2 files changed

Lines changed: 111 additions & 6 deletions

File tree

‎internal/cbm/extract_calls.c‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2853,14 +2853,54 @@ static const char *python_parameter_name(CBMExtractCtx *ctx, TSNode param) {
28532853
* function whose parameter is `run` is still flagged. Detecting it needs exactly
28542854
* the body scan this helper avoids, and it costs one edge in a shape that
28552855
* essentially does not occur. */
2856-
static bool python_callee_is_bound_parameter(CBMExtractCtx *ctx, TSNode call_node,
2856+
static bool python_callee_is_bound_parameter(CBMExtractCtx *ctx, WalkState *state, TSNode call_node,
28572857
TSNode callee_ident) {
2858+
/* Ancestors are walked with the UNIFIED WALK'S OWN CURSOR, not ts_node_parent().
2859+
*
2860+
* ts_node_parent() is not O(1): it restarts at the tree root and descends to
2861+
* find the parent (vendored ts_runtime/src/node.c), so it costs O(depth) per
2862+
* hop. A parent-chain walk is therefore O(depth^2) per call, and since every
2863+
* level of f(f(f(...))) is ITSELF a bare call, O(depth^3) across the file --
2864+
* tests/test_stack_overflow.c nests 30,000 deep, which is a hang, not a
2865+
* slowdown. Capping the hop COUNT does not fix that, because the cost is per
2866+
* hop; the walk itself has to be cheap.
2867+
*
2868+
* ts_tree_cursor_goto_parent() IS O(1) -- the cursor carries its path stack --
2869+
* so copying the walk cursor and ascending costs nothing per hop. Same reason
2870+
* CBMWalkScope keeps a frame stack instead of recomputing enclosing state, and
2871+
* same current_cursor idiom (including the identity guard) as
2872+
* usage_current_field_name in extract_usages.c.
2873+
*
2874+
* The hop cap then bounds the remaining O(depth) per call. 64 rather than
2875+
* Lean's 20 above: that walk looks for an IMMEDIATE declaration boundary,
2876+
* while this one crosses whatever statement/expression nesting separates a
2877+
* call from its enclosing def, so it needs headroom before it can bite on
2878+
* ordinary code.
2879+
*
2880+
* Both the cap and a missing/mismatched cursor FAIL OPEN -- return false, do
2881+
* NOT suppress. A guard whose whole justification is precision must never
2882+
* destroy an edge it did not actually prove was fabricated, so either can
2883+
* only ever cost a suppression, never a true edge. Pinned in
2884+
* test_extraction.c. */
2885+
enum { PY_MAX_SCOPE_WALK_DEPTH = 64 };
2886+
28582887
const char *callee_name = cbm_node_text(ctx->arena, callee_ident, ctx->source);
28592888
if (!callee_name || !callee_name[0]) {
28602889
return false;
28612890
}
2862-
for (TSNode scope = ts_node_parent(call_node); !ts_node_is_null(scope);
2863-
scope = ts_node_parent(scope)) {
2891+
/* Only trust the shared cursor when it is actually parked on this call. */
2892+
if (!state || !state->current_cursor ||
2893+
!ts_node_eq(ts_tree_cursor_current_node(state->current_cursor), call_node)) {
2894+
return false;
2895+
}
2896+
2897+
TSTreeCursor up = ts_tree_cursor_copy(state->current_cursor);
2898+
bool bound = false;
2899+
for (int walked = 0; walked < PY_MAX_SCOPE_WALK_DEPTH && !bound; walked++) {
2900+
if (!ts_tree_cursor_goto_parent(&up)) {
2901+
break;
2902+
}
2903+
TSNode scope = ts_tree_cursor_current_node(&up);
28642904
const char *kind = ts_node_type(scope);
28652905
if (strcmp(kind, "function_definition") != 0 && strcmp(kind, "lambda") != 0) {
28662906
continue;
@@ -2873,11 +2913,13 @@ static bool python_callee_is_bound_parameter(CBMExtractCtx *ctx, TSNode call_nod
28732913
for (uint32_t i = 0; i < count; i++) {
28742914
const char *pname = python_parameter_name(ctx, ts_node_named_child(params, i));
28752915
if (pname && strcmp(pname, callee_name) == 0) {
2876-
return true;
2916+
bound = true;
2917+
break;
28772918
}
28782919
}
28792920
}
2880-
return false;
2921+
ts_tree_cursor_delete(&up);
2922+
return bound;
28812923
}
28822924

28832925
static bool is_objectscript_language(CBMLanguage language) {
@@ -3515,7 +3557,8 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML
35153557
TSNode obj = ts_node_child_by_field_name(fn, TS_FIELD("object"));
35163558
call.is_method = !python_receiver_is_exempt(ctx, obj);
35173559
} else if (!ts_node_is_null(fn) && strcmp(ts_node_type(fn), "identifier") == 0) {
3518-
call.callee_is_locally_bound = python_callee_is_bound_parameter(ctx, node, fn);
3560+
call.callee_is_locally_bound =
3561+
python_callee_is_bound_parameter(ctx, state, node, fn);
35193562
}
35203563
}
35213564
// TS/JS/TSX receiver-aware guard (#592/#606 direction; same intent

‎tests/test_extraction.c‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4700,6 +4700,67 @@ TEST(extract_python_bare_call_flags_locally_bound_callee) {
47004700
PASS();
47014701
}
47024702

4703+
/* The scope walk behind the bare-call flag is BOUNDED (64 ancestors). An
4704+
* unbounded walk is QUADRATIC in a file's nesting depth -- every level of
4705+
* f(f(f(...))) is itself a bare call re-walking its own chain -- which hung
4706+
* stack_overflow_b's 30,000-deep fixture rather than merely slowing it.
4707+
*
4708+
* This pins the cap's CONTRACT deterministically rather than by wall clock (a
4709+
* timing assertion would be a lottery, not a gate): within the cap the shadowed
4710+
* callee is flagged; past it the guard FAILS OPEN and leaves the call alone, so
4711+
* the cap can only ever cost a suppression, never a true edge. Removing the cap
4712+
* flips the deep case to flagged and fails this test. */
4713+
TEST(extract_python_bare_call_scope_walk_is_bounded) {
4714+
/* Shallow: return_statement / block / function_definition — 3 ancestors. */
4715+
CBMFileResult *shallow = extract("def shallow(handler):\n"
4716+
" return handler()\n",
4717+
CBM_LANG_PYTHON, "t", "s.py");
4718+
ASSERT_NOT_NULL(shallow);
4719+
ASSERT_FALSE(shallow->has_error);
4720+
int shallow_seen = 0;
4721+
for (int i = 0; i < shallow->calls.count; i++) {
4722+
const char *cn = shallow->calls.items[i].callee_name;
4723+
if (cn && strcmp(cn, "handler") == 0) {
4724+
shallow_seen++;
4725+
ASSERT_TRUE(shallow->calls.items[i].callee_is_locally_bound);
4726+
}
4727+
}
4728+
ASSERT_EQ(shallow_seen, 1);
4729+
cbm_free_result(shallow);
4730+
4731+
/* Deep: 200 parenthesized_expression ancestors put the SAME call well past
4732+
* the cap, so the enclosing parameter is never reached and the call stays
4733+
* unflagged. */
4734+
const int PARENS = 200;
4735+
size_t sz = (size_t)PARENS * 2 + 128;
4736+
char *src = malloc(sz);
4737+
ASSERT_NOT_NULL(src);
4738+
char *w = src;
4739+
w += snprintf(w, sz, "def deep(handler):\n return ");
4740+
memset(w, '(', (size_t)PARENS);
4741+
w += PARENS;
4742+
w += snprintf(w, sz - (size_t)(w - src), "handler()");
4743+
memset(w, ')', (size_t)PARENS);
4744+
w += PARENS;
4745+
snprintf(w, sz - (size_t)(w - src), "\n");
4746+
4747+
CBMFileResult *deep = extract(src, CBM_LANG_PYTHON, "t", "d.py");
4748+
ASSERT_NOT_NULL(deep);
4749+
ASSERT_FALSE(deep->has_error);
4750+
int deep_seen = 0;
4751+
for (int i = 0; i < deep->calls.count; i++) {
4752+
const char *cn = deep->calls.items[i].callee_name;
4753+
if (cn && strcmp(cn, "handler") == 0) {
4754+
deep_seen++;
4755+
ASSERT_FALSE(deep->calls.items[i].callee_is_locally_bound);
4756+
}
4757+
}
4758+
ASSERT_EQ(deep_seen, 1);
4759+
cbm_free_result(deep);
4760+
free(src);
4761+
PASS();
4762+
}
4763+
47034764
/* TS/JS/TSX receiver-aware flag (#592/#606; same intent as the Perl flag above).
47044765
* A member call x.foo() with a non-this/super receiver is flagged is_method so
47054766
* the resolver can suppress a weak short-name match (`re.test()` must not bind a
@@ -6380,6 +6441,7 @@ SUITE(extraction) {
63806441
RUN_TEST(extract_flag_exempt_method_call_not_flagged_is_method);
63816442
RUN_TEST(extract_python_member_call_flags_is_method);
63826443
RUN_TEST(extract_python_bare_call_flags_locally_bound_callee);
6444+
RUN_TEST(extract_python_bare_call_scope_walk_is_bounded);
63836445
RUN_TEST(extract_ts_member_call_flags_is_method);
63846446
RUN_TEST(extract_ts_this_super_receiver_not_flagged);
63856447
RUN_TEST(extract_js_member_call_flags_is_method);

0 commit comments

Comments
 (0)