Skip to content

Commit 080d67d

Browse files
gadievronclaude
andcommitted
fix(parsers/c): guard macro-alias resolution against cyclic #defines
_resolve_call recursed on a macro alias target with no visited-set, so a cyclic function-like #define pair ({"A":"B","B":"A"}) looped A->B->A->... until RecursionError aborted the entire repository's C call-graph build. Thread an _alias_chain set through the recursion so a cyclic alias resolves to None. Stacked on pr/zig-php-generic-anon, whose C-parser rework carries the same unguarded macro recursion; this layers the cycle guard on top of that signature. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4058a1c commit 080d67d

2 files changed

Lines changed: 273 additions & 19 deletions

File tree

libs/openant-core/parsers/c/call_graph_builder.py

Lines changed: 222 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ def __init__(self, extractor_output: Dict, options: Optional[Dict] = None):
110110
self.macros = extractor_output.get('macros', {})
111111
self.macro_aliases = extractor_output.get('macro_aliases', {})
112112
self.prototypes = extractor_output.get('prototypes', {})
113+
# class_name -> [direct base-class name, ...] for the inheritance walk in
114+
# member dispatch (bug [30]). Defaults to {} when the extractor output
115+
# predates base-class extraction, so resolution degrades to the [51]
116+
# same-type behavior rather than erroring.
117+
self.class_bases: Dict[str, List[str]] = extractor_output.get('class_bases', {})
113118
self.repo_path = extractor_output.get('repository', '')
114119

115120
self.max_depth = options.get('max_depth', 3)
@@ -121,6 +126,10 @@ def __init__(self, extractor_output: Dict, options: Optional[Dict] = None):
121126
# Indexes for faster lookup
122127
self.functions_by_name: Dict[str, List[str]] = {}
123128
self.functions_by_file: Dict[str, List[str]] = {}
129+
# class_name -> {base_method_name -> [func_id, ...]} for member dispatch.
130+
# Scoped per (class, method) so a receiver-typed call resolves only to a
131+
# method actually declared on that class, never a sibling/free function.
132+
self.methods_by_class: Dict[str, Dict[str, List[str]]] = {}
124133

125134
# Include map: file -> set of included header files
126135
self.include_map: Dict[str, Set[str]] = {}
@@ -153,6 +162,13 @@ def _build_indexes(self) -> None:
153162
self.functions_by_file[file_path] = []
154163
self.functions_by_file[file_path].append(func_id)
155164

165+
# Index methods by their declaring class for receiver-type dispatch.
166+
class_name = func_data.get('class_name')
167+
if class_name and name:
168+
method_base = name.split('::')[-1] if '::' in name else name
169+
self.methods_by_class.setdefault(class_name, {}) \
170+
.setdefault(method_base, []).append(func_id)
171+
156172
# Build include map
157173
for file_path, inc_list in self.includes.items():
158174
self.include_map[file_path] = set()
@@ -191,15 +207,25 @@ def _extract_calls_from_code(self, code: str, caller_id: str) -> Set[str]:
191207
except Exception:
192208
return self._extract_calls_regex(code, caller_id)
193209

210+
# Receiver static types inferred from local declarations in this body,
211+
# used to resolve member calls (w.compute() / w->compute()) to the
212+
# method on the receiver's known type.
213+
local_var_types = self._extract_local_var_types(tree.root_node, code_bytes)
214+
194215
stack = [tree.root_node]
195216
while stack:
196217
node = stack.pop()
197218
if node.type == 'call_expression':
198219
func_node = node.child_by_field_name('function')
199220
if func_node:
200-
call_name = self._extract_call_name(func_node, code_bytes)
221+
call_name, receiver = self._extract_call_name_and_receiver(
222+
func_node, code_bytes
223+
)
201224
if call_name:
202-
resolved = self._resolve_call(call_name, caller_file)
225+
receiver_type = local_var_types.get(receiver) if receiver else None
226+
resolved = self._resolve_call(call_name, caller_file,
227+
receiver_type=receiver_type,
228+
is_member=func_node.type == 'field_expression')
203229
if resolved:
204230
calls.add(resolved)
205231
# A function passed by name as an argument (e.g.
@@ -232,6 +258,81 @@ def _extract_callback_args(self, call_node, source: bytes, caller_file: str) ->
232258
found.add(resolved)
233259
return found
234260

261+
def _extract_call_name_and_receiver(self, node, source: bytes):
262+
"""Return (call_name, receiver_identifier) for a call's function child.
263+
264+
receiver_identifier is the bare identifier text of a member-call receiver
265+
(the `w` in `w.compute()` / `w->compute()`) when it is a simple
266+
identifier, else None. The call_name is identical to what
267+
_extract_call_name returns, so non-member calls are unaffected.
268+
"""
269+
if node.type == 'field_expression':
270+
receiver = None
271+
arg = node.child_by_field_name('argument')
272+
if arg is not None and arg.type == 'identifier':
273+
receiver = source[arg.start_byte:arg.end_byte].decode(
274+
'utf-8', errors='replace')
275+
# _extract_call_name declines field_expression (no false free-function
276+
# edges); the member name is recovered here from the `field` child and
277+
# resolved ONLY through typed/same-file member dispatch in _resolve_call.
278+
field = node.child_by_field_name('field')
279+
name = None
280+
if field is not None:
281+
name = source[field.start_byte:field.end_byte].decode(
282+
'utf-8', errors='replace')
283+
if not name.isidentifier():
284+
name = None
285+
return name, receiver
286+
return self._extract_call_name(node, source), None
287+
288+
def _extract_local_var_types(self, root, source: bytes) -> Dict[str, str]:
289+
"""Map local variable name -> declared type name within a function body.
290+
291+
Walks `declaration` nodes and records the (type_identifier, variable)
292+
pairs for both plain declarations (`Widget w;`) and pointer declarations
293+
(`Widget* w = ...;`). Only simple type_identifier types are recorded;
294+
anything else (templates, qualified types, multiple declarators we can't
295+
cleanly attribute) is skipped so callers fall back to base-name
296+
resolution rather than risk a wrong-type edge.
297+
"""
298+
var_types: Dict[str, str] = {}
299+
stack = [root]
300+
while stack:
301+
node = stack.pop()
302+
if node.type == 'declaration':
303+
type_node = node.child_by_field_name('type')
304+
if type_node is not None and type_node.type == 'type_identifier':
305+
type_name = source[type_node.start_byte:type_node.end_byte] \
306+
.decode('utf-8', errors='replace')
307+
# A declaration can hold several declarators (Widget a, b;);
308+
# attribute the type to every variable name we extract.
309+
for child in node.children:
310+
var_name = self._declared_var_name(child, source)
311+
if var_name:
312+
var_types[var_name] = type_name
313+
stack.extend(reversed(node.children))
314+
return var_types
315+
316+
def _declared_var_name(self, node, source: bytes) -> Optional[str]:
317+
"""Extract the declared variable identifier from a declarator subtree.
318+
319+
Handles the plain identifier (`w`), pointer_declarator (`* w`) and
320+
init_declarator (`* w = ...` / `w = ...`) shapes. Returns None for nodes
321+
that are not a variable declarator (e.g. the type node, `;`).
322+
"""
323+
if node.type == 'identifier':
324+
return source[node.start_byte:node.end_byte].decode('utf-8', errors='replace')
325+
if node.type in ('pointer_declarator', 'init_declarator', 'reference_declarator'):
326+
inner = node.child_by_field_name('declarator')
327+
if inner is not None:
328+
return self._declared_var_name(inner, source)
329+
# init_declarator with no declarator field: scan children.
330+
for child in node.children:
331+
name = self._declared_var_name(child, source)
332+
if name:
333+
return name
334+
return None
335+
235336
def _extract_call_name(self, node, source: bytes) -> Optional[str]:
236337
"""Extract the function name from a call_expression's function child."""
237338
text = source[node.start_byte:node.end_byte].decode('utf-8', errors='replace')
@@ -274,27 +375,126 @@ def _is_visible_from(self, func_id: str, caller_file: str) -> bool:
274375
return True
275376
return not func_data.get('is_static', False)
276377

277-
def _resolve_call(self, call_name: str, caller_file: str) -> Optional[str]:
278-
"""Resolve a function call name to a function ID."""
378+
def _resolve_same_file(self, call_name: str, caller_file: str) -> Optional[str]:
379+
"""Resolve a call to a user-defined function in the same file, if any."""
380+
same_file_funcs = self.functions_by_file.get(caller_file, [])
381+
for func_id in same_file_funcs:
382+
func_data = self.functions.get(func_id, {})
383+
fname = func_data.get('name', '')
384+
base_name = fname.split('::')[-1] if '::' in fname else fname
385+
if base_name == call_name:
386+
return func_id
387+
return None
388+
389+
def _resolve_method_on_class(self, class_name: str, call_name: str,
390+
caller_file: str) -> Optional[str]:
391+
"""Resolve call_name to a method DIRECTLY declared on class_name (same file).
392+
393+
Returns the func_id of a method named call_name declared on class_name and
394+
defined in caller_file, else None. No inheritance — this is the single-hop
395+
lookup the walk in _resolve_member_call composes over the base chain.
396+
"""
397+
by_method = self.methods_by_class.get(class_name)
398+
if not by_method:
399+
return None
400+
for func_id in by_method.get(call_name, []):
401+
func_data = self.functions.get(func_id, {})
402+
if func_data.get('file_path', '') == caller_file:
403+
return func_id
404+
return None
405+
406+
def _resolve_member_call(self, call_name: str, caller_file: str,
407+
receiver_type: str) -> Optional[str]:
408+
"""Resolve a member call to the method on the receiver's STATIC type,
409+
walking UP the base-class chain to the first ancestor that defines it.
410+
411+
Sound static-type floor (bug [30]): start at the receiver's declared type
412+
and return its own method if it defines call_name; otherwise walk up its
413+
direct base classes (BFS, cycle-guarded) and resolve to the FIRST ancestor
414+
that declares call_name in the same file. The walk STOPS at the first
415+
definer, so a derived override resolves to itself, never an ancestor.
416+
417+
Deliberately does NOT link derived overrides of an ancestor's virtual
418+
method (a documented non-goal that would create false edges): a call via a
419+
Base* receiver resolves to Base's method only — the static-type floor.
420+
421+
Same-file only: if no class on the chain defines call_name in this
422+
translation unit, returns None so the caller falls back to base-name
423+
resolution (never a wrong-type / unrelated-free-function edge).
424+
"""
425+
visited: Set[str] = set()
426+
queue: List[str] = [receiver_type]
427+
while queue:
428+
cls = queue.pop(0)
429+
if cls in visited:
430+
continue
431+
visited.add(cls)
432+
# First definer on the chain wins (own type before ancestors).
433+
match = self._resolve_method_on_class(cls, call_name, caller_file)
434+
if match:
435+
return match
436+
for base in self.class_bases.get(cls, []):
437+
if base not in visited:
438+
queue.append(base)
439+
return None
440+
441+
def _resolve_call(self, call_name: str, caller_file: str,
442+
receiver_type: Optional[str] = None,
443+
is_member: bool = False,
444+
_alias_chain: Optional[Set[str]] = None) -> Optional[str]:
445+
"""Resolve a function call name to a function ID.
446+
447+
When receiver_type is given (a member call like w.compute() whose receiver
448+
w has a known same-file type), resolve to that type's method FIRST. If
449+
that fails, fall through to the unchanged base-name resolution below.
450+
"""
451+
if receiver_type:
452+
member_match = self._resolve_member_call(call_name, caller_file,
453+
receiver_type)
454+
if member_match:
455+
return member_match
456+
457+
# A user-defined function in the SAME FILE shadows any stdlib/builtin
458+
# of the same name, so it must be checked BEFORE the stdlib filter.
459+
# Scope is deliberately same-file only: a genuine stdlib call (no
460+
# same-file definition) still falls through to _is_stdlib below, so we
461+
# never wrongly link a real stdlib call (e.g. printf/open) to an
462+
# unrelated same-named user function in another file.
463+
same_file_user_func = self._resolve_same_file(call_name, caller_file)
464+
if same_file_user_func:
465+
return same_file_user_func
466+
467+
# A member call (obj->m() / obj.m()) whose receiver type is unknown or
468+
# whose chain defines no such method resolves same-file only: declining
469+
# here keeps the field-expression precision guarantee (never an edge to
470+
# an unrelated cross-file free function of the same name).
471+
if is_member:
472+
return None
473+
279474
if self._is_stdlib(call_name):
280475
return None
281476

282477
# Check for macro aliases
283478
resolved_name = self.macro_aliases.get(call_name, call_name)
284479
if resolved_name != call_name:
285-
# Try resolving the aliased name instead
286-
result = self._resolve_call(resolved_name, caller_file)
287-
if result:
288-
return result
480+
# Guard against cyclic macro aliases (e.g. ``#define A B`` /
481+
# ``#define B A`` -> {"A": "B", "B": "A"}). Without a visited-set
482+
# the recursion below would loop A->B->A->... until RecursionError
483+
# aborted the whole repo's call-graph build.
484+
if _alias_chain is None:
485+
_alias_chain = {call_name}
486+
if resolved_name not in _alias_chain:
487+
_alias_chain.add(resolved_name)
488+
# Try resolving the aliased name instead
489+
result = self._resolve_call(resolved_name, caller_file,
490+
_alias_chain=_alias_chain)
491+
if result:
492+
return result
289493

290494
# 1. Same-file functions
291-
same_file_funcs = self.functions_by_file.get(caller_file, [])
292-
for func_id in same_file_funcs:
293-
func_data = self.functions.get(func_id, {})
294-
fname = func_data.get('name', '')
295-
base_name = fname.split('::')[-1] if '::' in fname else fname
296-
if base_name == call_name:
297-
return func_id
495+
same_file_match = self._resolve_same_file(call_name, caller_file)
496+
if same_file_match:
497+
return same_file_match
298498

299499
# 2. Functions in included headers
300500
included_files = self.include_map.get(caller_file, set())
@@ -392,10 +592,13 @@ def _extract_calls_regex(self, code: str, caller_id: str) -> Set[str]:
392592
if func_name in ('if', 'while', 'for', 'switch', 'return', 'sizeof',
393593
'typeof', 'alignof', 'offsetof', 'case', 'else'):
394594
continue
395-
if not self._is_stdlib(func_name):
396-
resolved = self._resolve_call(func_name, caller_file)
397-
if resolved:
398-
calls.add(resolved)
595+
# No _is_stdlib gate here: _resolve_call applies the same-file-first
596+
# rule and the stdlib filter internally, so a user function whose
597+
# name collides with a builtin still resolves (same leak as the
598+
# tree-sitter path otherwise).
599+
resolved = self._resolve_call(func_name, caller_file)
600+
if resolved:
601+
calls.add(resolved)
399602

400603
return calls
401604

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
"""Regression test for F4: C call-graph builder must not crash on cyclic macro aliases.
2+
3+
A function-like ``#define`` pair such as::
4+
5+
#define A(x) B(x)
6+
#define B(x) A(x)
7+
8+
produces ``macro_aliases = {"A": "B", "B": "A"}``. Before the fix,
9+
``_resolve_call`` recursed on the aliased name with no cycle guard, so
10+
resolving ``A`` recursed A->B->A->... until ``RecursionError`` aborted the
11+
entire repository's C call-graph build.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
import pytest
17+
18+
tree_sitter_c = pytest.importorskip("tree_sitter_c")
19+
20+
from parsers.c.call_graph_builder import CallGraphBuilder
21+
22+
23+
def _builder(macro_aliases):
24+
return CallGraphBuilder(
25+
{
26+
"functions": {},
27+
"includes": {},
28+
"macros": {},
29+
"macro_aliases": macro_aliases,
30+
}
31+
)
32+
33+
34+
def test_cyclic_macro_alias_does_not_recurse():
35+
"""Two-node alias cycle must resolve to None, not raise RecursionError."""
36+
builder = _builder({"A": "B", "B": "A"})
37+
# Must not raise; unresolved cyclic alias returns None.
38+
assert builder._resolve_call("A", "foo.c") is None
39+
assert builder._resolve_call("B", "foo.c") is None
40+
41+
42+
def test_longer_macro_alias_cycle_terminates():
43+
"""A 3-node alias cycle must also terminate."""
44+
builder = _builder({"A": "B", "B": "C", "C": "A"})
45+
assert builder._resolve_call("A", "foo.c") is None
46+
47+
48+
def test_self_macro_alias_does_not_recurse():
49+
"""A self-alias is short-circuited by the != guard but must stay safe."""
50+
builder = _builder({"A": "A"})
51+
assert builder._resolve_call("A", "foo.c") is None

0 commit comments

Comments
 (0)