Skip to content

Commit badc527

Browse files
committed
test(go): sweep the dataflow relations over every position, not one
The existing tests pin each property at one hand-picked position; these keep the relations and add the sampling. R1 depth monotonicity, R2 a capped slice is a prefix (not merely a subset) and reports the whole total, R3 flows_to_argument implies flows_to_call, R4 a returned path implies the predicate, R7 every edge endpoint is an id get_source can read -- each on both backends over every addressable position, capped at 60. This is the part of the analyzer's metamorphic design that transfers: it varies the input program and holds the analyzer fixed, which the SDK cannot do because it produces no analysis, so the SDK's dual is to vary the query and hold the analysis fixed. Not generative -- a property-based library over this space found nothing a deterministic sweep did not. The sweeps found a parity gap on their first run: get_source accepted a body-node id on the graph backend and raised on the local one, whose index held only types, callables and fields, though every edge endpoint and SliceNode.ref is one. Fixing it exposed describe diverging the other way -- local accepted a bare id, the graph silently dropped it. Both now accept both forms. Verified by mutation rather than by passing: six behavior-changing mutations in the shapes these relations guard are all caught. R7 anchors on GoBodyNode.id, the analyzer's own string, because comparing the SDK's composed id against the SDK's own lookup let a corrupted join move both sides together. Also tidies prose in the Go docstrings and tests. Refs #436
1 parent b04ec12 commit badc527

7 files changed

Lines changed: 400 additions & 18 deletions

File tree

‎cldk/analysis/go/backend.py‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,19 @@ def get_callees(self, callable_id: str) -> List[GoCallGraphEdge]:
169169
# -----[ source + whole-program ]-----
170170
@abstractmethod
171171
def get_source(self, node_id: str) -> str:
172-
"""The source text of the node ``node_id`` — ``module.source[node.span.bytes]``."""
172+
"""The source text of the node ``node_id`` — ``module.source[node.span.bytes]``.
173+
174+
``node_id`` is a durable id (type, callable, field) **or a body-node id** —
175+
``"<callable can:// id>@<body key>"``, the analyzer's own, round-tripped and never composed
176+
by the caller. The body-node form is what every edge endpoint and every
177+
:class:`~cldk.analysis.commons.results.SliceNode` carries, so accepting it is what lets a
178+
caller read an id off a ``get_ddg`` page or a slice and then read its text.
179+
180+
Raises:
181+
CodeanalyzerExecutionException: nothing in this application owns that id, or the node
182+
exists and carries no span — a parameter-passing vertex is a dataflow position,
183+
not a region of the file, and has no text on either backend.
184+
"""
173185

174186
@abstractmethod
175187
def get_call_graph_json(self) -> str:
@@ -463,7 +475,7 @@ def get_degraded(self) -> List[GoDegraded]:
463475
**Read this before trusting an absence.** A non-empty list means some L4 step fell back to
464476
a coarser result (``range_func``, ``unmapped_ssa``, ``no_vta``, …), so a path that *is*
465477
returned is still a real may-flow, but "no path" is not a proof over the affected packages
466-
and callables. :meth:`taint` reports this on its own ledger rather than making the judgement
478+
and callables. :meth:`taint` reports this on its own ledger rather than making the judgment
467479
for the caller; this accessor is how a caller inspects it directly.
468480
"""
469481

@@ -509,7 +521,7 @@ def taint(
509521
``unresolved``.
510522
511523
**Sources, sinks and sanitizers are the caller's to supply.** This SDK ships no Go framework
512-
catalogue and derives no default set: a per-language vocabulary of taint sources is policy
524+
catalog and derives no default set: a per-language vocabulary of taint sources is policy
513525
that rots, and this accessor is the mechanism.
514526
515527
**A sanitizer is two mechanisms wearing one word**, told apart by shape. A bare ``str`` cuts

‎cldk/analysis/go/codeanalyzer/codeanalyzer.py‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@
6868
VIA,
6969
GoAnalysisBackend,
7070
)
71-
from cldk.analysis.go.ids import body_node_id, callable_match_names, external_signature, go_value_names, pick_declaration
71+
from cldk.analysis.go.ids import body_node_id, callable_match_names, external_signature, go_value_names, pick_declaration, split_body_node_id
7272
from cldk.models.go import (
7373
GoAnalysis,
7474
GoApplication,
@@ -339,17 +339,27 @@ def get_functions(self) -> Dict[str, GoCallable]:
339339
return self._functions
340340

341341
def get_source(self, node_id: str) -> str:
342-
"""The source text of ``node_id`` — ``module.source`` sliced by ``node.span.bytes`` — for a
343-
durable-id node (type, callable, field).
342+
"""The source text of ``node_id`` — ``module.source`` sliced by ``node.span.bytes``.
343+
344+
``node_id`` is a durable-id node (type, callable, field) **or a body-node id**: a statement,
345+
a call site or a vertex, named by the analyzer's own
346+
``"<callable can:// id>@<body key>"``. The body-node form is what every edge endpoint and
347+
every :class:`~cldk.analysis.commons.results.SliceNode` carries, so without it a caller
348+
could read an id off a ``get_ddg`` page and then not use it — and the Neo4j backend accepts
349+
it (its ``:GoCanNode`` lookup does not care which kind of node it found), which made this
350+
a parity gap rather than a shared limitation. Python's ``get_source`` names the same two
351+
forms in its own contract.
344352
345353
``span.bytes`` are **UTF-8 byte** offsets, so a non-ASCII module is sliced on the encoded
346354
bytes and decoded back (an ASCII module is indexed directly, paying nothing), the same way
347355
the TS ``_Spanned.code`` property does. Slicing the ``str`` by character index would drift
348356
on any file with a multibyte character before the span.
349357
"""
350-
entry = self._source_of.get(node_id)
358+
entry = self._source_of.get(node_id) or self._body_source_of(node_id)
351359
if entry is None:
352-
raise CodeanalyzerExecutionException(f"no source for {node_id!r}: not a type, callable or field of application " f"{self.application_name!r}")
360+
raise CodeanalyzerExecutionException(
361+
f"no source for {node_id!r}: not a type, callable, field or body node of application {self.application_name!r}"
362+
)
353363
mod, node = entry
354364
if node.span is None:
355365
raise CodeanalyzerExecutionException(f"node {node_id!r} carries no span, so it has no source text")
@@ -362,6 +372,31 @@ def get_source(self, node_id: str) -> str:
362372
self._module_bytes[mod.id] = encoded
363373
return encoded[b0:b1].decode("utf-8")
364374

375+
def _body_source_of(self, node_id: str) -> "tuple | None":
376+
"""``(module, body node)`` for a body-node id, or ``None`` if nothing owns it.
377+
378+
The owner is found with :func:`~cldk.analysis.go.ids.split_body_node_id` rather than by
379+
splitting on ``@`` — trap 1: a library stand-in's own id holds ``/@external/`` before the
380+
vertex ``@``. A stand-in has no module and so no text, which this reports as "not found"
381+
rather than as a module lookup that would fail further on.
382+
383+
Not indexed eagerly: the body-node ids outnumber the durable ones by a wide margin (3,324
384+
against 319 on the analyzer's own corpus), and ``get_source`` on one is the uncommon case,
385+
so the prefix walk is paid per call instead of the index being paid per application.
386+
"""
387+
hit = split_body_node_id(node_id, self._dataflow_owners)
388+
if hit is None:
389+
return None
390+
cid, key = hit
391+
owner, _tid, path = self._dataflow_owners[cid]
392+
if not path: # a stand-in: synthesized, so there is no source file behind it
393+
return None
394+
node = (owner.body or {}).get(key) or (owner.body or {}).get(key.lstrip("@"))
395+
if node is None:
396+
return None
397+
entry = self._source_of.get(cid)
398+
return (entry[0], node) if entry else None
399+
365400
# =====================================================================================
366401
# L3/L4 DATAFLOW. See :class:`~cldk.analysis.go.backend.GoAnalysisBackend` for the contract and
367402
# for the four Go-specific traps; the id/name grammar those traps need is in
@@ -392,7 +427,7 @@ def _require_dataflow(self) -> None:
392427
393428
One guard for the per-callable graphs and the slices: they come from the same analyzer pass
394429
and go dark together. It **raises** rather than returning empty because at a shallower level
395-
an empty answer would mean "not analysed" while looking exactly like "no dependence" (D7).
430+
an empty answer would mean "not analyzed" while looking exactly like "no dependence" (D7).
396431
"""
397432
if self._level < self._DATAFLOW_LEVEL:
398433
raise CodeanalyzerUsageException(

‎cldk/analysis/go/go_analysis.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ def taint(
257257
m sources against n sinks in one traversal, reporting per pair: the witnesses in ``paths``,
258258
the pairs an absence claim can be built on in ``exhausted``, and everything else in
259259
``unresolved``. The source/sink/sanitizer vocabulary is the caller's — no Go framework
260-
catalogue ships. A non-empty :meth:`get_degraded` voids every ``exhausted`` certification,
260+
catalog ships. A non-empty :meth:`get_degraded` voids every ``exhausted`` certification,
261261
because absence of a path is not a proof over an approximated graph.
262262
"""
263263
return self.backend.taint(sources, sinks, sanitizers, depth=depth, max_paths=max_paths)

‎cldk/analysis/go/neo4j/neo4j_backend.py‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ def _slice_node(row: Dict[str, Any], module_key: Callable[[str], str]) -> SliceN
102102
103103
The local twin is ``GoCodeanalyzer._slice_node``; the two share
104104
:func:`~cldk.analysis.commons.resolve.body_node_kind`, so a vertex a caller addressed through
105-
``resolve_value`` as a ``global`` comes back from a slice labelled a ``global`` on either
105+
``resolve_value`` as a ``global`` comes back from a slice labeled a ``global`` on either
106106
backend, and they differ only in where the fields are read from.
107107
108108
``row["var"]`` is the graph's name for the JSON's ``of`` (see
@@ -480,7 +480,7 @@ def get_source(self, node_id: str) -> str:
480480
# the value a vertex stands for arrives under the name the shared helpers expect.
481481
#
482482
# A STAND-IN IS A FIRST-CLASS VERTEX. ``:GoExternal`` nodes carry ``GO_HAS_BODY_NODE`` body
483-
# nodes at L4, so the traversals must not be labelled ``:GoCallable``-only on the interior or
483+
# nodes at L4, so the traversals must not be labeled ``:GoCallable``-only on the interior or
484484
# a flow *through* ``database/sql.DB.Query`` would be invisible — which is the very flow the
485485
# httpsql fixture gates. ``:GoCallable|GoExternal`` is therefore deliberate everywhere below.
486486
# =====================================================================================
@@ -1051,20 +1051,41 @@ def get_degraded(self) -> List[GoDegraded]:
10511051
def describe(self, nodes: Sequence[object]) -> List[SliceNode]:
10521052
"""Fill in ``source`` for these positions (see :meth:`GoAnalysisBackend.describe`).
10531053
1054-
One query for the batch rather than one per node. A vertex with no span gets ``None``: it is
1055-
a dataflow position, not a region of the file, and there is nothing to read on either
1056-
backend.
1054+
A vertex with no span gets ``None``: it is a dataflow position, not a region of the file,
1055+
and there is nothing to read on either backend.
1056+
1057+
**A bare ``can://`` id is accepted as well as a resolved node**, because the local backend
1058+
accepts one and silently *dropping* it here was the worse half of that divergence: a caller
1059+
passing ids got a short list back with no signal, where the local backend answered. The
1060+
documented argument is a :class:`~cldk.analysis.commons.results.SliceNode`; an id is
1061+
resolved to one first rather than refused, so neither backend is the lenient outlier.
10571062
"""
10581063
out: List[SliceNode] = []
10591064
for n in nodes:
10601065
if not isinstance(n, SliceNode):
1061-
continue
1066+
n = self._describe_id(str(n))
1067+
if n is None:
1068+
continue
10621069
try:
10631070
out.append(n.model_copy(update={"source": self.get_source(n.ref)}))
10641071
except CodeanalyzerExecutionException:
10651072
out.append(n.model_copy(update={"source": None}))
10661073
return out
10671074

1075+
def _describe_id(self, node_id: str) -> "SliceNode | None":
1076+
"""One body-node id as a :class:`SliceNode`, or ``None`` if this graph holds no such node.
1077+
1078+
The local twin is ``GoCodeanalyzer._slice_node``; both route the node's ``kind``/``var``
1079+
through :func:`~cldk.analysis.commons.resolve.body_node_kind`, so an id described here and
1080+
one described in process carry the same caller-facing vocabulary.
1081+
"""
1082+
rows = self._run(
1083+
f"MATCH (b:GoBodyNode {{id:$id}}) OPTIONAL MATCH (c:{self._OWNER})-[:GO_HAS_BODY_NODE]->(b) "
1084+
"RETURN b.id AS ref, b.kind AS kind, b.var AS var, b.span_json AS span, c.signature AS callable, c.span_json AS c_span",
1085+
id=node_id,
1086+
)
1087+
return _slice_node(self._span_lines(rows[0]), self._module_key) if rows else None
1088+
10681089
# -----[ artifact / dependency / config layer — reserved-but-unemitted on this projection ]-----
10691090
def get_artifacts(self) -> Dict[str, PyArtifact]:
10701091
raise CodeanalyzerExecutionException(ARTIFACT_LAYER_UNAVAILABLE.format(app=self.application_name))

‎docs/agent-api-reference.md‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,21 @@ Five Go-specific facts, each one a thing that will mislead you if you don't know
555555
`exhausted` certification in a `taint` result and file an `analysis_degraded` diagnostic saying
556556
why. `complete` goes `False` with it.
557557

558+
**Reading text back off a position.** `get_source(node_id)` takes a durable id (type, callable,
559+
field) **or a body-node id** — `"<callable can:// id>@<body key>"`, which is what every edge
560+
endpoint and every `SliceNode.ref` carries. So an id read off a `get_ddg` page or a slice can be
561+
turned into source directly:
562+
563+
```python
564+
page = go.get_ddg("sqlRepo.Find")
565+
go.get_source(page.edges[0].src) # 'return s.db.Query("SELECT …")'
566+
```
567+
568+
A parameter-passing vertex has no span — it is a dataflow position, not a region of the file — so
569+
it raises rather than returning `""`, on both backends. `describe(nodes)` is the batch form: it
570+
takes the `SliceNode`s a slice or path handed you, and also accepts bare ids, returning `source=None`
571+
for a position that has no text rather than raising.
572+
558573
**The port-connectivity gate.** `slice_*`, `paths_between`, `flows_to_*` and `taint` refuse — naming
559574
the gap rather than returning a false empty — when no `formal_in` vertex in the attached analysis has
560575
an outgoing dependence edge. Same rule as Java's, and the probe reads the *data*, never a version

‎tests/analysis/go/test_go_dataflow.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
gated analyzer-side by ``TestGateL4_HTTPToSQLPath``; ``four_type_kinds.json`` is one declaration of
66
each ``GoType.kind`` at ``-a 1``.
77
8-
**What is asserted and what is not.** These tests pin *behaviour a caller depends on* — that the
8+
**What is asserted and what is not.** These tests pin *behavior a caller depends on* — that the
99
gated flow is found, that the four Go id/name traps are handled, that a refutation is only made
1010
where one is warranted. They deliberately do not pin edge counts: those move with every analyzer
1111
release and a test that fails on an improvement teaches nothing. Where a count appears it is
@@ -329,7 +329,7 @@ def test_paging_is_a_prefix_of_one_total_order(httpsql):
329329

330330
def test_a_shallow_analysis_refuses_rather_than_answering_empty(tmp_path, monkeypatch):
331331
"""At a level below the dataflow pass the three lists are absent, and an empty page there would
332-
mean "not analysed" while looking exactly like "no dependence" (D7). So it raises, naming the
332+
mean "not analyzed" while looking exactly like "no dependence" (D7). So it raises, naming the
333333
level to rebuild at."""
334334
shallow = _facade(tmp_path, monkeypatch, FOUR_KINDS, app="kinds", level=AnalysisLevel.symbol_table)
335335
with pytest.raises(CodeanalyzerUsageException, match="level 3"):

0 commit comments

Comments
 (0)