From 55c96e59099416ea94e2d02e40c7f4346c940fe3 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 19 Oct 2026 19:52:24 +0200 Subject: [PATCH 1/2] fix(summaries): list external calls in source order A summary listed its external calls in the order the rounds met them, a caller copying its callee's in the callee's order. Around a call cycle of three functions or more, that order rotated forever while the content had settled, so the order-sensitive comparison ending the module's fixpoint never held and the analysis never returned. Summaries now sort their external calls on the key record merges them on. The cache format moves to 17. --- src/coretrace_python/cache.py | 2 +- .../interprocedural/summaries.py | 18 +- tests/test_summary_fixpoint.py | 184 ++++++++++++++++++ 3 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 tests/test_summary_fixpoint.py diff --git a/src/coretrace_python/cache.py b/src/coretrace_python/cache.py index a76a4c9..b8d7b54 100644 --- a/src/coretrace_python/cache.py +++ b/src/coretrace_python/cache.py @@ -42,7 +42,7 @@ from coretrace_python.semantic.symbols import SymbolId from coretrace_python.source import FileLocation, Location, SourceId, SourceSpan -CACHE_FORMAT = 16 +CACHE_FORMAT = 17 @dataclass(frozen=True) diff --git a/src/coretrace_python/interprocedural/summaries.py b/src/coretrace_python/interprocedural/summaries.py index 2250c6d..1acf400 100644 --- a/src/coretrace_python/interprocedural/summaries.py +++ b/src/coretrace_python/interprocedural/summaries.py @@ -791,6 +791,22 @@ def _aligned(dependencies: tuple[_T, ...], cleared: tuple[Cleared, ...]) -> Iter return zip(dependencies, (*cleared, *(() for _ in range(missing))), strict=True) +def _span_order(span: SourceSpan | None) -> tuple[object, ...]: + """Source spans in source order, an absent one first.""" + + if span is None: + return () + return (span.source_id, span.start_line, span.start_column, span.end_line or 0, span.end_column or 0) + + +def _record_order(call: ExternalCall) -> tuple[object, ...]: + """External calls in source order, by the key ``record`` merges them on. A summary + compared in its fixpoint must not depend on the order in which the rounds met its + calls: around a call cycle, that order can rotate forever (#261).""" + + return (_span_order(call.location), _span_order(call.call_site), _span_order(call.invoked_at), call.symbol) + + def summarize( name: str, function: FunctionIR, @@ -826,7 +842,7 @@ def summarize( name, len(function.parameters), problem.returns.parameters, - tuple(problem.external.values()), + tuple(sorted(problem.external.values(), key=_record_order)), return_externals=problem.returns.externals, mutations=tuple(mutations), side_effects=frozenset(problem.mutated_globals), diff --git a/tests/test_summary_fixpoint.py b/tests/test_summary_fixpoint.py new file mode 100644 index 0000000..5ef402a --- /dev/null +++ b/tests/test_summary_fixpoint.py @@ -0,0 +1,184 @@ +"""Acceptance tests for issue #261: the summary fixpoint of a module terminates. + +A function's summary lists the external calls it reaches, its own and those of the +project functions it calls. Those of a callee used to be copied in the callee's order, so +around a cycle of three functions or more each function's order depended on the order +its callee had in the previous round: the content settled, the order rotated, and the +order-sensitive comparison that ends the fixpoint never held. The analysis then never +returned. A summary now lists its external calls in source order, whatever the order in +which the rounds met them. +""" + +from __future__ import annotations + +import dataclasses +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.cfg import CFGAnalysis +from coretrace_python.frontend import build_hir +from coretrace_python.interprocedural import CallGraphAnalysis, SummaryAnalysis +from coretrace_python.interprocedural.summaries import summarize +from coretrace_python.ir.ssa import SSAAnalysis +from coretrace_python.source import SourceManager + +# A converging analysis of these files takes about a second: a run past this is one that +# would not end. +TIMEOUT = 60 + +THREE_FUNCTIONS = """\ +import os + + +def a(x): + b(x) + os.system(x) + + +def b(x): + c(x) + print(x) + + +def c(x): + a(x) + + +def run(): + a(input()) +""" + +FOUR_FUNCTIONS = """\ +import os + + +def a(x): + b(x) + os.system(x) + + +def b(x): + c(x) + print(x) + + +def c(x): + d(x) + + +def d(x): + a(x) + + +def run(): + a(input()) +""" + +# The shape of ansible's ``Role.load``, ``_load_role_data`` and ``_load_dependencies``. +EXCEPTION_HANDLERS = """\ +import os + + +def load(include): + try: + load_data(include) + except RuntimeError: + raise ValueError(include.ds) + + +def load_data(include): + load_dependencies(include) + try: + os.system(include) + except AssertionError: + raise ValueError(include.name) + + +def load_dependencies(include): + load(include) + + +def run(): + load(input()) +""" + + +def checked(tmp_path: Path, source: str) -> list[tuple[str, int, str | None]]: + """The findings of ``--check`` on a project of one module, failing the test instead of + waiting when the analysis does not end.""" + + root = tmp_path / "project" + root.mkdir() + (root / "app.py").write_text(source, encoding="utf-8") + try: + completed = subprocess.run( + [sys.executable, "-m", "coretrace_python", "--check", str(root), "--format", "json"], + capture_output=True, + text=True, + check=False, + timeout=TIMEOUT, + ) + except subprocess.TimeoutExpired: + pytest.fail(f"the analysis did not end within {TIMEOUT} s") + report = json.loads(completed.stdout) + return [(f["rule_id"], f["location"]["line"], f["metadata"].get("verdict")) for f in report["findings"]] + + +@pytest.mark.parametrize( + ("source", "line"), + [(THREE_FUNCTIONS, 19), (FOUR_FUNCTIONS, 23), (EXCEPTION_HANDLERS, 24)], + ids=["three-functions", "four-functions", "exception-handlers"], +) +def test_a_call_cycle_with_external_calls_after_the_recursive_call_is_checked( + tmp_path: Path, source: str, line: int +) -> None: + assert checked(tmp_path, source) == [("command-injection", line, "vulnerability")] + + +CALLEE_FIRST = """\ +import os + + +def run(cmd): + helper(cmd) + os.system(cmd) + + +def helper(cmd): + print(cmd) + os.remove(cmd) +""" + + +def test_a_summary_lists_its_external_calls_in_source_order() -> None: + # ``run`` meets ``helper``'s calls first, at line 5, then its own at line 6. + table = engine.build_manager(build_hir(SourceManager().add_source("app.py", CALLEE_FIRST))).get(SummaryAnalysis) + + assert [(str(c.symbol), c.location.start_line) for c in table.summary("run").external_calls] == [ + ("python.os.system", 6), + ("python.builtins.print", 10), + ("python.os.remove", 11), + ] + + +def test_a_summary_does_not_depend_on_the_order_of_its_callee_s_external_calls() -> None: + manager = engine.build_manager(build_hir(SourceManager().add_source("app.py", CALLEE_FIRST))) + graph = manager.get(CallGraphAnalysis) + summaries = manager.get(SummaryAnalysis) + table = {name: summaries.summary(name) for name in summaries.names} + helper = table["helper"] + reversed_helper = dataclasses.replace(helper, external_calls=helper.external_calls[::-1]) + run = graph.definitions["run"] + + def run_summary(callee): + return summarize( + "run", manager.get(SSAAnalysis, run), manager.get(CFGAnalysis, run), graph, {**table, "helper": callee} + ) + + assert helper.external_calls != reversed_helper.external_calls + assert run_summary(reversed_helper) == run_summary(helper) From 7fb01173e54f4bf535a12a7ce1f5f6a3c782d081 Mon Sep 17 00:00:00 2001 From: Hugo Date: Mon, 19 Oct 2026 19:52:24 +0200 Subject: [PATCH 2/2] fix(engine): start an import cycle's fixpoint from empty summaries A call between two modules of an import cycle was a call to an unknown symbol on the first iteration and a project call on the next, so around a cycle of three modules the iteration shifted by one module each time and ran to MAX_PROJECT_ITERATIONS. The members' functions now start from the summaries SummaryAnalysis starts from, shared as starting_summaries. Closes #261. --- src/coretrace_python/engine.py | 16 ++++++- .../interprocedural/__init__.py | 2 + .../interprocedural/summaries.py | 40 ++++++++++------ tests/test_summary_fixpoint.py | 46 ++++++++++++++++++- 4 files changed, 87 insertions(+), 17 deletions(-) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index c7f1648..3291d45 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -96,6 +96,7 @@ discover_sources, name_modules, project_symbol, + starting_summaries, ) from coretrace_python.ir import PYIR_SCHEMA_VERSION from coretrace_python.ir.defuse import DefUseAnalysis @@ -831,7 +832,20 @@ def _analyse_managers( counts = Counter(manager.module.name for manager in managers.values()) names = {name: manager.module.name for name, manager in managers.items() if counts[manager.module.name] == 1} - index = seed + # The members' functions start from the summaries ``SummaryAnalysis`` starts from. + # Left out, a call from one member to another is a call to an unknown symbol on the + # first iteration and a project call on the next: around a cycle of modules, the + # iteration then shifts by one module each time and never settles (#261). + index = SummaryIndex( + { + **seed.summaries, + **{ + project_symbol(module, function): summary + for name, module in names.items() + for function, summary in starting_summaries(managers[name].get(CallGraphAnalysis)).items() + }, + } + ) for manager in managers.values(): manager.provide(ProjectSummaries, index) for _ in range(MAX_PROJECT_ITERATIONS): diff --git a/src/coretrace_python/interprocedural/__init__.py b/src/coretrace_python/interprocedural/__init__.py index fc053b2..e27a800 100644 --- a/src/coretrace_python/interprocedural/__init__.py +++ b/src/coretrace_python/interprocedural/__init__.py @@ -40,6 +40,7 @@ TemplateCallsAnalysis, TemplateFilter, cleared_by, + starting_summaries, ) __all__ = [ @@ -78,4 +79,5 @@ "discover_sources", "name_modules", "project_symbol", + "starting_summaries", ] diff --git a/src/coretrace_python/interprocedural/summaries.py b/src/coretrace_python/interprocedural/summaries.py index 1acf400..a5a5c33 100644 --- a/src/coretrace_python/interprocedural/summaries.py +++ b/src/coretrace_python/interprocedural/summaries.py @@ -855,6 +855,21 @@ def summarize( ) +def starting_summaries(graph: CallGraph) -> dict[str, FunctionSummary]: + """The summary each function of ``graph`` starts from in a fixpoint: a supported + function reaches nothing yet, the bottom its summary grows from; an unsupported one + passes every parameter to its return value and keeps that summary.""" + + table: dict[str, FunctionSummary] = {} + for name, function in graph.definitions.items(): + count = len(function.parameters) + if name in graph.unsupported: + table[name] = FunctionSummary(name, count, frozenset(range(count)), (), True) + else: + table[name] = FunctionSummary(name, count, NONE, ()) + return table + + class SummaryAnalysis(Analysis[SummaryTable]): name: ClassVar[str] = "interprocedural.summaries" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset( @@ -876,20 +891,17 @@ def compute(cls, ctx: AnalysisContext) -> SummaryTable: project = ctx.get(ProjectSummaries).summaries clearing = ctx.get(ClearingAnalysis) templates = ctx.get(TemplateCallsAnalysis) - table: dict[str, FunctionSummary] = {} - supported: dict[str, tuple[FunctionIR, CFG, HeapFacts, Callable[[BlockId], bool]]] = {} - for name, function in graph.definitions.items(): - if name in graph.unsupported: - count = len(function.parameters) - table[name] = FunctionSummary(name, count, frozenset(range(count)), (), True) - else: - supported[name] = ( - ctx.get(SSAAnalysis, function), - ctx.get(CFGAnalysis, function), - ctx.get(HeapAnalysis, function), - ctx.get(ConstantPropagation, function).reachable, - ) - table[name] = FunctionSummary(name, len(function.parameters), NONE, ()) + table = starting_summaries(graph) + supported: dict[str, tuple[FunctionIR, CFG, HeapFacts, Callable[[BlockId], bool]]] = { + name: ( + ctx.get(SSAAnalysis, function), + ctx.get(CFGAnalysis, function), + ctx.get(HeapAnalysis, function), + ctx.get(ConstantPropagation, function).reachable, + ) + for name, function in graph.definitions.items() + if name not in graph.unsupported + } changed = True while changed: changed = False diff --git a/tests/test_summary_fixpoint.py b/tests/test_summary_fixpoint.py index 5ef402a..7ed97b9 100644 --- a/tests/test_summary_fixpoint.py +++ b/tests/test_summary_fixpoint.py @@ -1,4 +1,5 @@ -"""Acceptance tests for issue #261: the summary fixpoint of a module terminates. +"""Acceptance tests for issue #261: the summary fixpoints terminate, a module's and a +project's. A function's summary lists the external calls it reaches, its own and those of the project functions it calls. Those of a callee used to be copied in the callee's order, so @@ -7,6 +8,10 @@ order-sensitive comparison that ends the fixpoint never held. The analysis then never returned. A summary now lists its external calls in source order, whatever the order in which the rounds met them. + +The modules of an import cycle iterate their summaries together. Their own functions +now start from the summaries a module's fixpoint starts from, so the iteration grows to +its fixpoint instead of running to its bound. """ from __future__ import annotations @@ -15,14 +20,16 @@ import json import subprocess import sys +from collections import Counter from pathlib import Path import pytest from coretrace_python import engine +from coretrace_python.analysis import AnalysisManager from coretrace_python.cfg import CFGAnalysis from coretrace_python.frontend import build_hir -from coretrace_python.interprocedural import CallGraphAnalysis, SummaryAnalysis +from coretrace_python.interprocedural import CallGraphAnalysis, FunctionSummary, SummaryAnalysis from coretrace_python.interprocedural.summaries import summarize from coretrace_python.ir.ssa import SSAAnalysis from coretrace_python.source import SourceManager @@ -182,3 +189,38 @@ def run_summary(callee): assert helper.external_calls != reversed_helper.external_calls assert run_summary(reversed_helper) == run_summary(helper) + + +# The modules of an import cycle are analysed together, their summaries iterated until +# the index of the project's summaries stops changing. ``THREE_FUNCTIONS`` across three +# modules: a call from one member to another used to be a call to an unknown symbol on +# the first iteration and a project call on the next, and the iteration never settled +# before its bound. +CYCLE_OF_MODULES = { + "pkg/__init__.py": "", + "pkg/a.py": "import os\n\nfrom pkg import b\n\n\ndef a(x):\n b.b(x)\n os.system(x)\n\n\ndef run():\n a(input())\n", + "pkg/b.py": "from pkg import c\n\n\ndef b(x):\n c.c(x)\n print(x)\n", + "pkg/c.py": "from pkg import a\n\n\ndef c(x):\n a.a(x)\n", +} + + +def test_a_call_cycle_across_modules_converges_before_the_bound(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + root = tmp_path / "project" + for path, text in CYCLE_OF_MODULES.items(): + (root / path).parent.mkdir(parents=True, exist_ok=True) + (root / path).write_text(text, encoding="utf-8") + extractions: Counter[str] = Counter() + summaries_of = engine._summaries_of + + def counted(manager: AnalysisManager) -> dict[str, FunctionSummary]: + extractions[manager.module.name] += 1 + return summaries_of(manager) + + monkeypatch.setattr(engine, "_summaries_of", counted) + analysis = engine.analyze_project(root, [engine.BUNDLED_PLUGINS]) + + # Once per iteration, and once more for the module's results. + assert max(extractions[name] for name in ("pkg.a", "pkg.b", "pkg.c")) <= engine.MAX_PROJECT_ITERATIONS + assert [ + (f.rule_id, Path(str(f.span.source_id)).name, f.span.start_line, f.metadata["verdict"]) for f in analysis.findings + ] == [("command-injection", "a.py", 12, "vulnerability")]