From 228f270417c1aad25513d91992c9d1d951c5bc3e Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 17:28:02 -0700 Subject: [PATCH 1/7] fix(l3): define a callable's formals at @entry so dataflow can root at a parameter --- .../syntactic_analysis/CallableBuilder.java | 5 +- .../cldk/syntactic_analysis/L3Overlays.java | 13 +++- .../dataflow/DdgBuilder.java | 17 +++- .../dataflow/DdgBuilderEntryDefsTest.java | 77 +++++++++++++++++++ 4 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/CallableBuilder.java b/src/main/java/com/ibm/cldk/syntactic_analysis/CallableBuilder.java index b3f35ab5..f8430acb 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/CallableBuilder.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/CallableBuilder.java @@ -116,7 +116,10 @@ public JCallable build( // cfg/cdg/ddg overlays. The BlockStmt and symbol solver are live only here. // Skipped under the wala engine — L3WalaOverlays handles that path post-build. if (ctx.getAnalysisLevel() >= 3 && "ast".equals(ctx.getL3Engine())) { - L3Overlays.L3Result l3 = L3Overlays.build(b, callable.getBody(), ctx, ctx.getGraphFieldDepth()); + L3Overlays.L3Result l3 = L3Overlays.build(b, callable.getBody(), ctx, + ctx.getGraphFieldDepth(), + callable.getParameters().stream().map(JParameter::getName) + .collect(Collectors.toList())); callable.setBody(l3.body()); callable.setCfg(l3.cfg()); callable.setCdg(l3.cdg()); diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/L3Overlays.java b/src/main/java/com/ibm/cldk/syntactic_analysis/L3Overlays.java index 32124fab..b966a58a 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/L3Overlays.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/L3Overlays.java @@ -56,11 +56,22 @@ public List ddg() { } } + /** Existing callers keep working; a callable with no declared formals behaves exactly as before. */ public static L3Result build(BlockStmt body, Map existingBody, L1BuildContext ctx, int fieldDepth) { + return build(body, existingBody, ctx, fieldDepth, List.of()); + } + + /** + * @param formals the callable's declared parameter names, in declaration order — passed through to + * {@link DdgBuilder#build(ControlFlowGraph, int, List)} so a formal is defined at {@code @entry} + * and its own dataflow can root a dependence at a parameter. + */ + public static L3Result build(BlockStmt body, Map existingBody, L1BuildContext ctx, + int fieldDepth, List formals) { ControlFlowGraph cfg = CfgBuilder.build(body, existingBody, ctx); List cdg = CdgBuilder.build(cfg); - List ddg = DdgBuilder.build(cfg, fieldDepth); + List ddg = DdgBuilder.build(cfg, fieldDepth, formals); return new L3Result(cfg.nodes(), cfg.toCfgEdges(), cdg, ddg); } } diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilder.java b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilder.java index ecea0cbd..82ec8c33 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilder.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilder.java @@ -71,6 +71,11 @@ public int hashCode() { } } + /** Existing callers keep working; a callable with no declared formals behaves exactly as before. */ + public static List build(ControlFlowGraph cfg, int fieldDepth) { + return build(cfg, fieldDepth, List.of()); + } + /** * Compute the data-dependence edges for one callable, in three phases: * @@ -85,8 +90,13 @@ public int hashCode() { * * Edges are deduped and sorted for determinism; each is one def→use pair for a {@code fieldDepth}- * limited access path. + * + * @param formals the callable's declared parameter names, in declaration order. They are defined + * at {@code @entry}: a formal has no defining statement, but it is live on entry, and without + * that def no dependence can root at a parameter — which is what forced downstream consumers + * to recover parameter flow by matching source text. */ - public static List build(ControlFlowGraph cfg, int fieldDepth) { + public static List build(ControlFlowGraph cfg, int fieldDepth, List formals) { // Phase 1: per-node gen sets (defs) and the paths each node reads (uses). List nodes = reachableFromEntry(cfg); Map> defs = new HashMap<>(); @@ -95,6 +105,11 @@ public static List build(ControlFlowGraph cfg, int fieldDepth) { Set d = new LinkedHashSet<>(); Set u = new LinkedHashSet<>(); collect(cfg.astNode(n), d, u, fieldDepth); + if (ControlFlowGraph.ENTRY.equals(n)) { + // A formal's access path is its bare name — a base segment, which AccessPath never + // truncates, so this is k-independent by construction. + d.addAll(formals); + } defs.put(n, d); uses.put(n, u); } diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java new file mode 100644 index 00000000..f00bb2c2 --- /dev/null +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java @@ -0,0 +1,77 @@ +package com.ibm.cldk.syntactic_analysis.dataflow; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.body.MethodDeclaration; +import com.github.javaparser.ast.stmt.BlockStmt; +import com.ibm.cldk.schema.JDdgEdge; +import com.ibm.cldk.syntactic_analysis.L1BuildContext; +import com.ibm.cldk.syntactic_analysis.L3Overlays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; + +/** + * A callable's formals are defined at {@code @entry}, so its own dataflow can see them. Before this, + * {@code @entry} had no AST node and therefore no defs, and no ddg edge could root at a parameter. + */ +class DdgBuilderEntryDefsTest { + + /** Build the AST-engine L3 overlays for one method's body, with its formals declared. */ + private static List ddgOf(String source, String methodName) { + MethodDeclaration md = StaticJavaParser.parse(source) + .findAll(MethodDeclaration.class).stream() + .filter(m -> m.getNameAsString().equals(methodName)) + .findFirst().orElseThrow(); + BlockStmt body = md.getBody().orElseThrow(); + List formals = md.getParameters().stream() + .map(p -> p.getNameAsString()).collect(Collectors.toList()); + L1BuildContext ctx = new L1BuildContext("can://java/t", "T.java", source, 3, 3, "ast"); + return L3Overlays.build(body, new LinkedHashMap<>(), ctx, 3, formals).ddg(); + } + + private static boolean hasEntryEdge(List ddg, String var) { + return ddg.stream().anyMatch(e -> "@entry".equals(e.getSrc()) && var.equals(e.getVar())); + } + + @Test + void aParameterUsedInTheReturnGetsAnEntryRootedEdge() { + List ddg = ddgOf("class T { int m(int q) { return q; } }", "m"); + assertTrue(hasEntryEdge(ddg, "q"), + "the formal is defined at @entry and used by the return: " + ddg); + assertEquals(1, ddg.stream().filter(e -> "@entry".equals(e.getSrc())).count(), + "exactly one entry-rooted edge for one used formal: " + ddg); + } + + @Test + void anUnusedParameterProducesNoEdge() { + List ddg = ddgOf("class T { int m(int q) { return 1; } }", "m"); + assertTrue(ddg.stream().noneMatch(e -> "@entry".equals(e.getSrc())), + "a formal nothing reads yields no edge — a def with no use is not a dependence: " + ddg); + } + + @Test + void aLocalShadowingTheFormalKillsTheEntryDefinition() { + // `q` is reassigned before the read, so the read depends on the assignment, not on @entry. + List ddg = ddgOf("class T { int m(int q) { q = 5; return q; } }", "m"); + assertTrue(ddg.stream().noneMatch(e -> "@entry".equals(e.getSrc())), + "the reassignment kills the entry def before any use reaches it: " + ddg); + } + + @Test + void twoFormalsBothUsedEachGetTheirOwnEdge() { + List ddg = ddgOf("class T { int m(int a, int b) { return a + b; } }", "m"); + assertTrue(hasEntryEdge(ddg, "a") && hasEntryEdge(ddg, "b"), ddg.toString()); + } + + @Test + void everyEntryEdgeCarriesSsaProvenance() { + List ddg = ddgOf("class T { int m(int q) { return q; } }", "m"); + ddg.stream().filter(e -> "@entry".equals(e.getSrc())) + .forEach(e -> assertEquals(List.of("ssa"), e.getProv(), + "an entry def is syntactic, not points-to derived")); + } +} From e3ebccf55d6116632a62ae4f8c1ef4bfd93ce4b7 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 18:10:38 -0700 Subject: [PATCH 2/7] docs(l3): record what entry defs buy the summary pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measurement only; no analyzer behaviour changes and no scratch edits land. L4GateTest's pinned counts did not move and the file is untouched: param_in 6, param_out 6, summary 5 on l4-sdg-test, before and after. That is not evidence of neutrality, though — every callable in that fixture has arity 0 or 1, and the mechanism that does move summary edges needs two parameters in one callable to fire. On the fixture: 11 ddg edges added, all @entry-rooted with prov ["ssa"], none lost. On daytrader8: ddg 3829 -> 5475 (+1646, 0 lost), points-to half unchanged at 1231 on both sides, wall clock +1.0% (median 29.98s -> 30.28s), so entry defs do not blow up the reaching-definitions fixpoint on real code. Seeding-rule redundancy: rules 2 and 3 lose zero edges when disabled on either corpus, and rule 1 alone reproduces the whole set (5 on the fixture, 329 on daytrader8). Against the parent commit, rule 1 alone produced zero — the ddg-rooted rule was inert for parameter seeding. Rule 1 is not yet a safe replacement, however. It seeds e.getSrc(), which for an entry-rooted edge is the shared @entry node, so BFS follows every formal's out-edges and one parameter's reach credits them all. That manufactured 43 false summary edges on daytrader8 — six on KeySequenceDirect.getNextID alone. Seeding the use site instead restores the exact pre-change set on both corpora. The recommendation is therefore: fix the seed and pin it with a multi-parameter fixture case first, retire the text rules second. --- docs/design/plans/2026-08-28-l3-entry-defs.md | 252 +++++++++++++++++- 1 file changed, 247 insertions(+), 5 deletions(-) diff --git a/docs/design/plans/2026-08-28-l3-entry-defs.md b/docs/design/plans/2026-08-28-l3-entry-defs.md index b4cb9a3d..2e7eb0f0 100644 --- a/docs/design/plans/2026-08-28-l3-entry-defs.md +++ b/docs/design/plans/2026-08-28-l3-entry-defs.md @@ -286,23 +286,265 @@ git commit -m "docs(spec): correct D7 — the reference analyzer has no hammock ## Measurement -*Task 2 fills this in. Leave the headings; replace the placeholder text.* +Measured at `228f270` ("after") against its parent `53a4029` ("before"). The "before" jar was +built from a throwaway worktree rather than by stashing, so the working branch was never mutated: + +```bash +mkdir -p /tmp/m +git worktree add /tmp/l3-before 228f270~1 +(cd /tmp/l3-before && ./gradlew fatJar -x test) # before jar +./gradlew fatJar -x test # after jar +# note: build/libs also holds a stale codeanalyzer-2.4.1.jar, so name the jar explicitly +# rather than globbing codeanalyzer-*.jar +java -jar /tmp/l3-before/build/libs/codeanalyzer-3.0.0.jar \ + -i src/test/resources/test-applications/l4-sdg-test -a 4 --no-build -o /tmp/m/before +java -jar build/libs/codeanalyzer-3.0.0.jar \ + -i src/test/resources/test-applications/l4-sdg-test -a 4 --no-build -o /tmp/m/after +git worktree remove /tmp/l3-before # when finished +``` + +**What this measurement does not cover.** There is no `gradle` on PATH in this environment, so +every CLI run needs `--no-build` and the WALA half degrades wherever the target has no compiled +classes. On `l4-sdg-test` that is total: both documents carry **zero** `prov:["points-to"]` edges, +so nothing below says anything about the WALA engine on that fixture. On `daytrader8` it happens +*not* to degrade — that project ships a `target/classes` (155 `.class` files, untracked, left by an +earlier local run), so WALA did run there and emitted 1231 `points-to` edges, **identically on both +sides** (1231 before, 1231 after). That identity is the only positive evidence here that the change +is AST-engine-only as the Global Constraints require; a clean clone without a built `daytrader8` +will not reproduce it and will see the points-to half absent on both sides instead. + +Also uncovered: the shadowing risk this plan's own self-review flags (a catch or lambda parameter +sharing a formal's spelling). Nothing below tests it. The 43 spurious `summary` edges found on +daytrader8 are all attributable to a different mechanism, established by construction — restoring +per-parameter seeding removes exactly those 43 and nothing else — but no claim is made here about +whether any of the 1646 new `ddg` edges is itself a shadowing artifact, because that was not +measured. ### Fixture deltas, per callable -*(new `ddg` edges as full tuples; `summary` edges gained)* +Eleven `ddg` edges added, **none lost, none changed**. Every one is rooted at `@entry` with +`prov:["ssa"]`, as the additivity constraint requires. `Heap.get()` takes no parameters and is +correctly untouched. + +| Callable | new `ddg` edge | `dst` node kind | +| --- | --- | --- | +| `Chain.a(int)` | `{src:"@entry", dst:"5:9", var:"x", prov:["ssa"]}` | `return` | +| `Chain.b(int)` | `{src:"@entry", dst:"9:9", var:"y", prov:["ssa"]}` | `return` | +| `Chain.c(int)` | `{src:"@entry", dst:"13:9", var:"z", prov:["ssa"]}` | `return` | +| `Heap.put(int)` | `{src:"@entry", dst:"7:9", var:"v", prov:["ssa"]}` | `statement` (`this.box = v;`) | +| `Heap.roundTrip(int)` | `{src:"@entry", dst:"15:9", var:"v", prov:["ssa"]}` | `call` (`put(v);`) | +| `Loops.callFirst(int[])` | `{src:"@entry", dst:"15:9", var:"a", prov:["ssa"]}` | `return` | +| `Loops.first(int[])` | `{src:"@entry", dst:"8:9", var:"q", prov:["ssa"]}` | `loop` (the for-each container) | +| `Mutual.even(int)` | `{src:"@entry", dst:"5:9", var:"n", prov:["ssa"]}` | `branch` (`if (n == 0)`) | +| `Mutual.even(int)` | `{src:"@entry", dst:"8:9", var:"n", prov:["ssa"]}` | `return` (`return odd(n - 1);`) | +| `Mutual.odd(int)` | `{src:"@entry", dst:"12:9", var:"n", prov:["ssa"]}` | `branch` | +| `Mutual.odd(int)` | `{src:"@entry", dst:"15:9", var:"n", prov:["ssa"]}` | `return` | + +Aggregate: `ddg` 2 → 13; `summary` 5 → 5; `param_in` 6 → 6; `param_out` 6 → 6. + +**`summary` edges: no change at all.** The two documents carry the identical five-edge set — +`Chain.a` `5:16/actual_in:0 → 5:16/actual_out`, `Chain.b` `9:16/actual_in:0 → 9:16/actual_out`, +`Loops.callFirst` `15:16/actual_in:0 → 15:16/actual_out`, `Mutual.even` +`8:16/actual_in:0 → 8:16/actual_out`, `Mutual.odd` `15:16/actual_in:0 → 15:16/actual_out`. Nothing +gained, nothing lost. + +**Therefore `L4GateTest`'s pinned counts did not move** and the file is unmodified by this task: +`param_in` 6, `param_out` 6, `summary` 5, exactly as pinned at #203. On this fixture semantic +seeding reproduces the textual result precisely. `Loops.first` is the interesting case: `L4GateTest`'s +javadoc singles it out as the one summary edge that *needs* container-node text seeding, because +`q` is named nowhere but the for-each header. It no longer needs it — `{@entry → 8:9, var q}` +composes with the pre-existing `{8:9 → 9:13, var x}` to reach the `return` sink semantically. + +Read that "no change" carefully, though. **Every callable in `l4-sdg-test` has arity 0 or 1** — the +fixture's maximum parameter count is one. The daytrader8 run below shows the change *does* move +`summary` edges, by a mechanism that needs two parameters in one callable to fire. So the stable +count here is not evidence that the change is summary-neutral; it is evidence that this fixture +cannot see the difference. That is a gap in the fixture, and the follow-on should close it. ### Seeding-rule redundancy -*(per rule: what disappears when it is disabled)* +Each rule was put behind a system property in a scratch worktree +(`git worktree add /tmp/l3-scratch 228f270`) so one jar covers every combination: +`-Dseed.rule1=0` / `-Dseed.rule2=0` / `-Dseed.rule3=0` disable `seedsFor`'s ddg-rooted, call-argument +text, and whole-body span text rules respectively. With no property set the instrumented jar emits a +**set-identical** `summary` payload to the clean jar on both corpora, so the harness is +behaviour-neutral. The scratch edits live only in `/tmp/l3-scratch`; nothing in this repo changed. + +```bash +java -Dseed.rule2=0 -Dseed.rule3=0 -jar /tmp/l3-scratch/build/libs/codeanalyzer-3.0.0.jar \ + -i src/test/resources/test-applications/daytrader8 -a 4 --no-build -o /tmp/m/dt-R1only +``` + +Summary-edge totals (`l4-sdg-test` / `daytrader8`): + +| variant | fixture | daytrader8 | vs. all-rules-on | +| --- | --- | --- | --- | +| all three rules (= shipped) | 5 | 329 | — | +| rule 1 disabled | 5 | 283 | loses 46 on daytrader8 | +| rule 2 disabled | 5 | 329 | loses nothing | +| rule 3 disabled | 5 | 329 | loses nothing | +| **rule 1 only** | **5** | **329** | **set-identical to all three** | +| rule 1 only, *before* the change | **0** | — | — | + +Read straight, that is the answer the follow-on wants: **rules 2 and 3 are dead on both corpora. +Disabling either loses zero edges, and rule 1 alone reproduces the entire emitted set — all 5 on the +fixture and all 329 on daytrader8.** And the last row is what the change bought: at `53a4029`, rule 1 +alone produced **zero** summary edges on the fixture, because no `ddg` edge could be rooted at a +parameter, so the ddg-rooted rule had nothing to match. It was inert for parameter seeding; the +textual rules were carrying the pass entirely. + +Rule 1 was not *entirely* inert before, though. On daytrader8, `dt-noR1` emits 283 edges against +`dt-before`'s 286, so three edges came from rule 1 matching an ordinary (non-entry) `ddg` edge whose +access-path base is a parameter name — e.g. a parameter reassigned in the body, whose reassignment +node then roots an edge. Those three are: + +- `TradeDirect.buy(String, String, double, int)` — `338:11/actual_in:1 → 338:11/actual_out` +- `TradeDirect.completeOrder(Integer, boolean)` — `524:19/actual_in:1 → 524:19/actual_out` +- `TradeDirect.sell(String, Integer, int)` — `445:11/actual_in:1 → 445:11/actual_out` + +#### The 46-edge gap is not a win — rule 1 now over-approximates across parameters + +Rule 1 disabled loses 46 edges but only 3 of those existed before the change, so the change *added* +43 summary edges on daytrader8 (286 → 329, none lost). **All 43 are spurious.** The mechanism: + +```java +for (JDdgEdge e : ddg) { + if (name.equals(base(e.getVar()))) { + seeds.add(e.getSrc()); // SummaryPass.java:244 + } +} +``` + +For an ordinary edge, `src` is the node that *defines* the variable, and seeding there is right. For +an entry-rooted edge `src` is `@entry` — the node that defines **every** formal. So the seed set for +*any* parameter collapses to the single shared node `@entry`, and `reaches()`'s BFS then follows +`@entry`'s out-edges for *all* the other parameters too. If one parameter reaches the return, every +parameter with an entry-rooted edge is credited with reaching it. + +Confirmed on a purpose-built discriminator rather than inferred. Drop this in a throwaway project +(a `build.gradle` copied from `l4-sdg-test` plus `src/main/java/com/x/T.java`) and run both jars over +it at `-a 4 --no-build`. The layout is verbatim — the node ids cited below are line:column, so +reformatting it renumbers them: + +```java +package com.x; + +public class T { + // Only `p` reaches the return. `q` is used by a void call and goes nowhere. + public int leak(int p, int q) { + int t = p; + sink(q); + return t; + } + + public void sink(int z) { + } + + public int caller(int m, int n) { + return leak(m, n); + } +} +``` + +`leak`'s `ddg` after the change is `{@entry → 6:9, var p}`, `{@entry → 7:9, var q}`, +`{6:9 → 8:9, var t}`. Seeding `q` at `@entry` walks the `p` edge to `6:9`, then to the `8:9` return +sink, so `flows(leak)` becomes `{0,1}` and `caller` gains +`{src:"15:16/actual_in:1", dst:"15:16/actual_out"}` — a claim that `n` comes back out of `leak`, +which it cannot: `q` is only handed to a `void` callee. Before the change this edge did not exist, +because the textual rules seeded `q` at `7:9` and at `7:9/actual_in:0`, both dead ends (`sink` is +`void`, so that site has no `actual_out` to bridge to). + +`KeySequenceDirect.getNextID` is the same shape in real code. It gains exactly six edges — +`39:13/actual_in:{1,2,3}` and `45:19/actual_in:{1,2,3}`, each `→ actual_out` — at its two +`allocNewBlock(conn, keyName, inSession, inGlobalTxn)` sites. Argument 0 is credited on *both* sides, +so `conn` already flowed to `allocNewBlock`'s return before the change; arguments 1–3 appear only +after it. Reading the callee, `conn` reaches the return through +`stmt = conn.prepareStatement(…)` → `rs = stmt.executeQuery()` → `keyVal = rs.getInt(…)` → +`block = new KeyBlock(keyVal, …)` → `return block`, and `keyName`, `inSession` and `inGlobalTxn` +are swept along with it at the shared `@entry` seed. The first sentence of that is observed; the +def-use chain is read off the source, not extracted from the document. + +This is over-approximation, which the L4 weak-update posture explicitly accepts, and no real flow is +lost — so it is not a defect against the plan's constraints. It is a **precision regression in +`SummaryPass`**, not in `DdgBuilder`, and it matters here because it lands on exactly the axis the +retirement decision turns on. + +#### A one-line correction removes it + +Seeding the *use* site instead of the shared def when the source is `@entry` restores per-parameter +resolution. Tested as a fourth toggle (`-Dseed.entryfix=1`) over +`seeds.add(ENTRYFIX && "@entry".equals(e.getSrc()) ? e.getDst() : e.getSrc())`: + +| variant | fixture | daytrader8 | +| --- | --- | --- | +| corrected rule 1 + rules 2,3 | 5 | 286 — **set-identical to `dt-before`** | +| **corrected rule 1 alone** | **5** | **286 — set-identical to `dt-before`** | + +All 43 spurious edges disappear, the discriminator's `caller` drops back to one summary edge, and +corrected rule 1 *on its own* reproduces the exact pre-change edge set across 1229 callables. This is +observed, not argued. The reasoning for why it should hold in general — reach from `@entry` along the +`var == name` edges is exactly `{dst} ∪ reach(dst)`, so seeding `dst` is the same set minus the other +parameters' edges — is inference, and the follow-on should re-derive it rather than take it from here. ### Real-application aggregates -*(edge deltas and wall-clock, on a fixture of realistic size)* +`daytrader8`, 1229 callables, 141 source files. Same two jars, `-a 4 --no-build`. + +| | before (`53a4029`) | after (`228f270`) | delta | +| --- | --- | --- | --- | +| `ddg` total | 3829 | 5475 | **+1646, 0 lost** | +| — of which `prov:["ssa"]` | 2598 | 4244 | +1646 | +| — of which `prov:["points-to"]` | 1231 | 1231 | **0** | +| — of which rooted at `@entry` | 0 | 1646 | +1646 | +| `summary` total | 286 | 329 | +43, 0 lost (all 43 spurious — above) | +| `param_in` | 1943 | 1943 | 0 | +| `param_out` | 914 | 914 | 0 | + +Every added edge is `prov:["ssa"]` and `@entry`-rooted; the WALA-derived half is untouched, which is +the additivity and engine-scope constraint holding on real code. + +**Wall clock — no measurable cost.** Three alternating warm runs per side, `/usr/bin/time -p`: + +| | run 1 | run 2 | run 3 | median real | median user | +| --- | --- | --- | --- | --- | --- | +| before | 29.98 | 29.98 | 30.14 | **29.98 s** | 62.45 s | +| after | 31.07 | 29.66 | 30.28 | **30.28 s** | 61.59 s | + ++0.30 s (+1.0%) on median wall clock, inside the 1.4 s spread of the "after" arm, and median *user* +CPU is slightly **lower** after. A first, discarded pair read 92.99 s (before) against 52.81 s +(after) — that ordering is backwards and both numbers are cold-cache artifacts of first-touching a +35 MB jar and a 20 MB output file; the gap closes entirely once warm. The check this run exists to +make — that seeding `@entry` with every formal does not blow up the reaching-definitions fixpoint on +real code — passes: 43% more `ddg` edges for ~1% more wall clock. ### Recommendation for the follow-on -*(which rules to retire, and what must stay)* +**Retire rules 2 and 3, but only together with a one-line correction to rule 1 — not before it.** +The redundancy evidence is unambiguous: on both corpora, disabling the call-argument-text rule +(`SummaryPass.java:248-257`) or the whole-body-span-text rule (`:258-262`) loses zero summary edges, +and rule 1 alone reproduces the entire emitted set. That is the retirement case, and entry defs are +what created it — the same experiment run against `53a4029` gives rule 1 alone **zero** edges. But +rule 1 as currently written seeds the shared `@entry` node, which conflates all of a callable's +formals and manufactured 43 false summary edges on daytrader8; retiring the textual rules on top of +that would trade a documented, bounded textual over-approximation for an undocumented semantic one +that is *worse* on exactly the multi-parameter callables the summary pass is most used on. With +`seeds.add(e.getDst())` for entry-rooted edges, corrected rule 1 alone reproduces `daytrader8`'s +pre-change 286-edge set and the fixture's 5-edge set exactly — so the follow-on's order of work +should be: fix the seed, pin the corrected behaviour with a regression test built on the +two-parameter discriminator above, and only then delete `:248-262`. Nothing in the suite can +currently distinguish the two: every test that runs `SummaryPass` — `SummaryPassTest.analyzed()`, +`L4GateTest`, `V2Neo4jSchemaConformanceTest` — runs it over `l4-sdg-test`, and every callable in that +fixture has arity 0 or 1. The fixture needs a multi-parameter callable before the assertion is worth +writing. + +Three gaps bound this recommendation. First, both text rules are exercised here only where rule 1 +already succeeds; neither corpus contains a case where source text is unavailable to `SummaryPass` +(a module with a null `source` would silently disable rule 3 wholesale — `index()` handles that +case, so it is reachable), so "dead" means dead on the 1243 callables measured, not proven +unreachable. Second, `l4-sdg-test` produced no `points-to` edges at all, so nothing here says how the +rules interact with WALA-derived `ddg` edges beyond the observation that daytrader8's 1231 of them +are unchanged on both sides. Third, the corrected-rule-1 result is measured on two corpora, not +proven; the follow-on should re-derive the argument before relying on it. --- From 61e433635322a43c1b6b23c1a4c2c19eaf94709c Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 18:14:51 -0700 Subject: [PATCH 3/7] =?UTF-8?q?docs(spec):=20correct=20D7=20=E2=80=94=20th?= =?UTF-8?q?e=20reference=20analyzer=20has=20no=20hammock=20regions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/SCHEMA_DECISIONS.md | 11 ++++++----- docs/design/specs/schema-v2-l3-l4-design.md | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.claude/SCHEMA_DECISIONS.md b/.claude/SCHEMA_DECISIONS.md index 1ad68e4d..8b4852bd 100644 --- a/.claude/SCHEMA_DECISIONS.md +++ b/.claude/SCHEMA_DECISIONS.md @@ -60,11 +60,12 @@ found not to). Expose `--precision {rta,0-cfa,0-1-cfa}`. Coarse heap precision conservative but sound-leaning semantic `ddg`. ### D7 — L4 summary edges: own summary pass -Compute `summary` (actual_in→actual_out) edges via a dedicated pass — hammock regions -composed bottom-up over the SCC-condensation DAG (Tarjan), k-limited to a monotone -fixpoint — mirroring `codeanalyzer-python` (`summaries.py`/`scc.py`). WALA's HRB -summaries are lazily computed inside its Slicer and not cleanly exposable. Heaviest -L4 unit; lands last. +Compute `summary` (actual_in→actual_out) edges via a dedicated pass — composed +bottom-up over the SCC-condensation DAG (Tarjan), k-limited to a monotone fixpoint. +The reference analyzer (`codeanalyzer-python`) operates at statement granularity, +not via region decomposition; region decomposition remains an open refinement for +either analyzer. WALA's HRB summaries are lazily computed inside its Slicer and not +cleanly exposable. Heaviest L4 unit; lands last. ### D8 — Identity: `can://java////` Java analog of the pilot's `can://python/…`; built from the existing `signatureOf()`. diff --git a/docs/design/specs/schema-v2-l3-l4-design.md b/docs/design/specs/schema-v2-l3-l4-design.md index 33f0b0b5..8ebdc9a9 100644 --- a/docs/design/specs/schema-v2-l3-l4-design.md +++ b/docs/design/specs/schema-v2-l3-l4-design.md @@ -77,7 +77,7 @@ Recorded in [`.claude/SCHEMA_DECISIONS.md`](../../../.claude/SCHEMA_DECISIONS.md | D4 | Type kinds | **Single `kind`** ∈ `class\|interface\|enum\|record\|annotation` + `nesting:{parent?,is_local?}` | Replaces the `is_interface/is_enum/is_record/is_nested/...` boolean pile. | | D5 | L3 CFG engine & granularity | **WALA engine → project to source-statement `line:col` nodes** | WALA computes SSACFG/dominance/def-use (heap-ready for L4); project each SSA instruction to its enclosing source statement via `IMethod.getSourcePosition` + JavaParser statement spans. **Fallback:** if source-fidelity proves unresolvable, revisit hand-building the CFG from the JavaParser AST (how Python/TS/Go do it). | | D6 | L4 points-to precision | **RTA default + `--precision {rta,0-cfa,0-1-cfa}`** | RTA is proven to scale (0-1-CFA was found not to); coarse heap precision ⇒ conservative semantic `ddg`. Precision tunable per project. | -| D7 | L4 summary edges | **Own summary pass** (region/bottom-up over the SCC condensation, k-limited) | Parity with `codeanalyzer-python` (`summaries.py`/`scc.py`); keystone-conformant. Heaviest L4 unit; lands last. | +| D7 | L4 summary edges | **Own summary pass** (bottom-up over the SCC condensation, k-limited, monotone fixpoint) | Bottom-up composition with monotone fixpoint over SCC-condensation DAG, mirroring the reference analyzer's approach (which operates at statement granularity, not region decomposition); keystone-conformant. Heaviest L4 unit; lands last. | | D8 | `can://` scheme for Java | `can://java////` | Java analog of the pilot's `can://python/…`; built from the existing `signatureOf()`. | | D9 | Neo4j namespace | Keep the **`J_`** relationship prefix | Existing convention (`J_CALLS`, …); dual-label `JSymbol` merge pattern already present. | @@ -148,7 +148,7 @@ Global/static state modeled as **extra** formal/actual vertices (rides the same **Semantic DDG:** `DataDependenceOptions.FULL` + `ModRef` yields alias/heap-derived def-use, emitted as **additional** `ddg` edges tagged `prov:["points-to"]`. L3's `prov:["ssa"]` edges are untouched — this preserves `L3 ⊆ L4` (weak-update / over-approximate posture; no strong updates that would remove an edge). -**Summary pass (D7):** a dedicated pass mirroring `codeanalyzer-python` — hammock-region summaries composed bottom-up over the SCC-condensation DAG (Tarjan), k-limited, iterated to a monotone fixpoint within each SCC. Produces the `summary` (actual_in→actual_out) edges that make later SDK slicing/taint context-sensitive without re-descending into callees. Heaviest unit; sequenced last. +**Summary pass (D7):** a dedicated pass composing bottom-up over the SCC-condensation DAG (Tarjan), k-limited, iterated to a monotone fixpoint within each SCC. The reference analyzer (`codeanalyzer-python`) reaches its transfer relation at statement granularity rather than via region decomposition; region decomposition remains an open refinement for either analyzer. This analyzer is better positioned than the reference for a future region pass: it persists both `cfg` and `cdg` on the callable and has Cooper–Harvey–Kennedy post-dominators in `CdgBuilder`. Produces the `summary` (actual_in→actual_out) edges that make later SDK slicing/taint context-sensitive without re-descending into callees. Heaviest unit; sequenced last. **Cost controls:** flag-gated (nothing at L4 runs unless `-a 4`); k-limiting mandatory for termination; summaries content-hashed/cached with recorded dependency metadata (incremental re-analysis aspirational); parallel-by-construction wavefront over the SCC DAG, `-j N` byte-identical to `-j 1`. From 0ce7c4401f43452a3139f12a6961baa8db4c0930 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 18:25:29 -0700 Subject: [PATCH 4/7] =?UTF-8?q?docs:=20correct=20D7=20=E2=80=94=20remove?= =?UTF-8?q?=20false=20comparative=20claim,=20align=20terminology=20to=20Py?= =?UTF-8?q?thon=20pilot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/SCHEMA_DECISIONS.md | 9 +++++---- docs/design/plans/2026-08-28-l3-entry-defs.md | 2 +- docs/design/specs/schema-v2-l3-l4-design.md | 4 ++-- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.claude/SCHEMA_DECISIONS.md b/.claude/SCHEMA_DECISIONS.md index 8b4852bd..3037da0b 100644 --- a/.claude/SCHEMA_DECISIONS.md +++ b/.claude/SCHEMA_DECISIONS.md @@ -62,10 +62,11 @@ conservative but sound-leaning semantic `ddg`. ### D7 — L4 summary edges: own summary pass Compute `summary` (actual_in→actual_out) edges via a dedicated pass — composed bottom-up over the SCC-condensation DAG (Tarjan), k-limited to a monotone fixpoint. -The reference analyzer (`codeanalyzer-python`) operates at statement granularity, -not via region decomposition; region decomposition remains an open refinement for -either analyzer. WALA's HRB summaries are lazily computed inside its Slicer and not -cleanly exposable. Heaviest L4 unit; lands last. +The Python pilot operates at statement granularity, not via region decomposition; +region decomposition remains an open refinement for either analyzer. Both persist +`cfg` and `cdg` on the callable and compute post-dominators; what remains is the +region decomposition itself. WALA's HRB summaries are lazily computed inside its +Slicer and not cleanly exposable. Heaviest L4 unit; lands last. ### D8 — Identity: `can://java////` Java analog of the pilot's `can://python/…`; built from the existing `signatureOf()`. diff --git a/docs/design/plans/2026-08-28-l3-entry-defs.md b/docs/design/plans/2026-08-28-l3-entry-defs.md index 2e7eb0f0..0783fc1d 100644 --- a/docs/design/plans/2026-08-28-l3-entry-defs.md +++ b/docs/design/plans/2026-08-28-l3-entry-defs.md @@ -268,7 +268,7 @@ D7 currently reads, in both places, that the summary pass mirrors `codeanalyzer- Rewrite both mentions to say what is true: the pass composes bottom-up over the SCC condensation with a monotone fixpoint (which both analyzers do), that `codeanalyzer-python` reaches its transfer relation at statement granularity rather than by region decomposition, and that region decomposition remains an open refinement for either analyzer rather than an existing precedent. Keep the rest of D7 — SCC condensation, k-limiting, fixpoint, "heaviest unit, sequenced last" — as written; only the parity claim is wrong. -Add one sentence recording that Java is better positioned than the reference for a future region pass, since it already persists both `cfg` and `cdg` on the callable and has Cooper–Harvey–Kennedy post-dominators in `CdgBuilder`. +Add one sentence stating that both analyzers persist `cfg` and `cdg` on the callable and compute post-dominators, so what remains to implement is the region decomposition itself. - [ ] **Step 2: Verify no other document repeats the claim** diff --git a/docs/design/specs/schema-v2-l3-l4-design.md b/docs/design/specs/schema-v2-l3-l4-design.md index 8ebdc9a9..f38602ef 100644 --- a/docs/design/specs/schema-v2-l3-l4-design.md +++ b/docs/design/specs/schema-v2-l3-l4-design.md @@ -77,7 +77,7 @@ Recorded in [`.claude/SCHEMA_DECISIONS.md`](../../../.claude/SCHEMA_DECISIONS.md | D4 | Type kinds | **Single `kind`** ∈ `class\|interface\|enum\|record\|annotation` + `nesting:{parent?,is_local?}` | Replaces the `is_interface/is_enum/is_record/is_nested/...` boolean pile. | | D5 | L3 CFG engine & granularity | **WALA engine → project to source-statement `line:col` nodes** | WALA computes SSACFG/dominance/def-use (heap-ready for L4); project each SSA instruction to its enclosing source statement via `IMethod.getSourcePosition` + JavaParser statement spans. **Fallback:** if source-fidelity proves unresolvable, revisit hand-building the CFG from the JavaParser AST (how Python/TS/Go do it). | | D6 | L4 points-to precision | **RTA default + `--precision {rta,0-cfa,0-1-cfa}`** | RTA is proven to scale (0-1-CFA was found not to); coarse heap precision ⇒ conservative semantic `ddg`. Precision tunable per project. | -| D7 | L4 summary edges | **Own summary pass** (bottom-up over the SCC condensation, k-limited, monotone fixpoint) | Bottom-up composition with monotone fixpoint over SCC-condensation DAG, mirroring the reference analyzer's approach (which operates at statement granularity, not region decomposition); keystone-conformant. Heaviest L4 unit; lands last. | +| D7 | L4 summary edges | **Own summary pass** (bottom-up over the SCC condensation, k-limited, monotone fixpoint) | Bottom-up composition with monotone fixpoint over SCC-condensation DAG, mirroring the Python pilot's approach (which operates at statement granularity, not region decomposition); keystone-conformant. Heaviest L4 unit; lands last. | | D8 | `can://` scheme for Java | `can://java////` | Java analog of the pilot's `can://python/…`; built from the existing `signatureOf()`. | | D9 | Neo4j namespace | Keep the **`J_`** relationship prefix | Existing convention (`J_CALLS`, …); dual-label `JSymbol` merge pattern already present. | @@ -148,7 +148,7 @@ Global/static state modeled as **extra** formal/actual vertices (rides the same **Semantic DDG:** `DataDependenceOptions.FULL` + `ModRef` yields alias/heap-derived def-use, emitted as **additional** `ddg` edges tagged `prov:["points-to"]`. L3's `prov:["ssa"]` edges are untouched — this preserves `L3 ⊆ L4` (weak-update / over-approximate posture; no strong updates that would remove an edge). -**Summary pass (D7):** a dedicated pass composing bottom-up over the SCC-condensation DAG (Tarjan), k-limited, iterated to a monotone fixpoint within each SCC. The reference analyzer (`codeanalyzer-python`) reaches its transfer relation at statement granularity rather than via region decomposition; region decomposition remains an open refinement for either analyzer. This analyzer is better positioned than the reference for a future region pass: it persists both `cfg` and `cdg` on the callable and has Cooper–Harvey–Kennedy post-dominators in `CdgBuilder`. Produces the `summary` (actual_in→actual_out) edges that make later SDK slicing/taint context-sensitive without re-descending into callees. Heaviest unit; sequenced last. +**Summary pass (D7):** a dedicated pass composing bottom-up over the SCC-condensation DAG (Tarjan), k-limited, iterated to a monotone fixpoint within each SCC. The Python pilot reaches its transfer relation at statement granularity rather than via region decomposition; region decomposition remains an open refinement for either analyzer. Both analyzers persist `cfg` and `cdg` on the callable and compute post-dominators; what remains to implement is the region decomposition itself. Produces the `summary` (actual_in→actual_out) edges that make later SDK slicing/taint context-sensitive without re-descending into callees. Heaviest unit; sequenced last. **Cost controls:** flag-gated (nothing at L4 runs unless `-a 4`); k-limiting mandatory for termination; summaries content-hashed/cached with recorded dependency metadata (incremental re-analysis aspirational); parallel-by-construction wavefront over the SCC DAG, `-j N` byte-identical to `-j 1`. From 6a8590e4c6f1bc7d872d4481e8be482843d3339c Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 18:45:32 -0700 Subject: [PATCH 5/7] fix(l4): seed a parameter at the use end of its @entry ddg edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `seedsFor`'s ddg rule filtered edges by `var` per parameter and then threw that precision away. It seeded `e.getSrc()`, which for an entry-rooted edge is the single `@entry` node where `DdgBuilder` defines every formal, and the reachability graph `facts` builds discards `var` entirely — it is keyed on node identity alone. So one parameter reaching the return credited every formal with reaching it. On daytrader8 that manufactured 43 summary edges (286 -> 329, none lost), all on callables of arity greater than one, which is why nothing in the fixture could see it. Seeding `e.getDst()` for entry-rooted edges restores the exact pre-change set: sorted (callable, src, dst) listings of all 286 daytrader8 summary edges are byte-identical against a jar built from 228f270~1 (sha256 be6b8302..d198a on both sides), and the fixture's 6 likewise. With both text rules disabled, corrected rule 1 alone reproduces the same 286-edge set — the result the follow-on retirement of those rules rests on. Evidence recorded in the plan's Measurement section, whose Recommendation paragraph also regains the "on the fixture" qualifier it dropped. Scoped to `@entry` deliberately. Generalising it to every rule-1 edge loses three daytrader8 edges, all at TradeDirect.completeOrder(Connection, Integer) argument 1, where `orderID` reaches the return only through a WALA points-to edge rooted at the node defining the returned `orderData`. Dropping a may-flow is the one direction the L4 posture forbids. Fixture: com/l4/Arity.java adds the first callable with two parameters — `leak(p, q)` returns a copy of `p` and hands `q` to a void callee — reached through `caller(m, n)` so the assertion lands on a summary edge. Without the fix `caller` carries both actual_in:0 and actual_in:1; with it, only actual_in:0. L4GateTest's pinned counts move with that fixture addition: - param_in 6 -> 9: `caller -> leak` contributes two arguments, `leak -> sink` one. - param_out 6 -> 7: `caller -> leak` returns a value; `leak -> sink` is void. - summary 5 -> 6: `Arity.caller` gains a shortcut; `Arity.leak`'s only site is void. SummaryPassTest's whole-fixture endpoint count moves 5 -> 6 for that same edge. --- docs/design/plans/2026-08-28-l3-entry-defs.md | 89 ++++++++++++++++++- .../dataflow/SummaryPass.java | 46 +++++++--- .../java/com/ibm/cldk/schema/L4GateTest.java | 35 ++++---- .../dataflow/SummaryPassTest.java | 32 ++++++- .../src/main/java/com/l4/Arity.java | 24 +++++ 5 files changed, 192 insertions(+), 34 deletions(-) create mode 100644 src/test/resources/test-applications/l4-sdg-test/src/main/java/com/l4/Arity.java diff --git a/docs/design/plans/2026-08-28-l3-entry-defs.md b/docs/design/plans/2026-08-28-l3-entry-defs.md index 0783fc1d..4c75b104 100644 --- a/docs/design/plans/2026-08-28-l3-entry-defs.md +++ b/docs/design/plans/2026-08-28-l3-entry-defs.md @@ -359,7 +359,9 @@ Read that "no change" carefully, though. **Every callable in `l4-sdg-test` has a fixture's maximum parameter count is one. The daytrader8 run below shows the change *does* move `summary` edges, by a mechanism that needs two parameters in one callable to fire. So the stable count here is not evidence that the change is summary-neutral; it is evidence that this fixture -cannot see the difference. That is a gap in the fixture, and the follow-on should close it. +cannot see the difference. That is a gap in the fixture, and the follow-on should close it. (Closed: +`com/l4/Arity.java` adds a two-parameter callable of which only one parameter flows out, which moves +the fixture's totals to `param_in` 9, `param_out` 7, `summary` 6.) ### Seeding-rule redundancy @@ -486,6 +488,88 @@ observed, not argued. The reasoning for why it should hold in general — reach `var == name` edges is exactly `{dst} ∪ reach(dst)`, so seeding `dst` is the same set minus the other parameters' edges — is inference, and the follow-on should re-derive it rather than take it from here. +#### Exact-set restoration, shown rather than asserted + +The claim above was originally recorded as counts. Re-derived end to end while shipping the fix, as +**sets** — the extractor emits one `callable-id ⇥ src ⇥ dst` line per summary edge, sorted, so an +empty `diff` is set (indeed sorted-multiset) identity and the matching digest is independent +corroboration. Run from the repo root: + +```bash +git worktree add /tmp/l3-seedfix 228f270~1 +(cd /tmp/l3-seedfix && ./gradlew fatJar -x test -q) +./gradlew fatJar -x test -q + +mkdir -p /tmp/m4 +java -jar /tmp/l3-seedfix/build/libs/codeanalyzer-3.0.0.jar \ + -i src/test/resources/test-applications/daytrader8 -a 4 --no-build -o /tmp/m4/before +java -jar build/libs/codeanalyzer-3.0.0.jar \ + -i src/test/resources/test-applications/daytrader8 -a 4 --no-build -o /tmp/m4/after + +cat > /tmp/m4/summaries.py <<'PY' +import json, sys +def walk(types, out): + for t in types.values(): + walk(t.get("types") or {}, out) + for c in (t.get("callables") or {}).values(): + walk(c.get("types") or {}, out) + for e in c.get("summary") or []: + out.append("%s\t%s\t%s" % (c["id"], e["src"], e["dst"])) +d, out = json.load(open(sys.argv[1])), [] +for f in d["application"]["symbol_table"].values(): + walk(f.get("types") or {}, out) +print("\n".join(sorted(out))) +PY +python3 /tmp/m4/summaries.py /tmp/m4/before/analysis.json > /tmp/m4/before.txt +python3 /tmp/m4/summaries.py /tmp/m4/after/analysis.json > /tmp/m4/after.txt +wc -l /tmp/m4/before.txt /tmp/m4/after.txt +diff /tmp/m4/before.txt /tmp/m4/after.txt && echo "EXACT SET MATCH" +shasum -a 256 /tmp/m4/before.txt /tmp/m4/after.txt +git worktree remove /tmp/l3-seedfix +``` + +``` +Preparing worktree (detached HEAD 53a4029) +HEAD is now at 53a4029 feat(l4): interprocedural SDG — param_in/param_out, semantic DDG, summary edges, graph 2.1.0 (#203) + 286 /tmp/m4/before.txt + 286 /tmp/m4/after.txt + 572 total +EXACT SET MATCH +be6b830272fee1fc2af9125ccc1b9bec6dbdffa718d93065d23978f3926d198a /tmp/m4/before.txt +be6b830272fee1fc2af9125ccc1b9bec6dbdffa718d93065d23978f3926d198a /tmp/m4/after.txt +``` + +The same comparison on `l4-sdg-test` (now carrying the two-parameter `Arity` discriminator) is 6 +edges on both sides with an empty `diff`. And with rules 2 and 3 disabled in a scratch worktree — +the toggles re-applied on top of the corrected rule 1 — **corrected rule 1 alone** is also 286 and +also `diff`-empty against `before.txt`, which is the row the retirement decision actually rests on: + +```bash +java -Dseed.rule2=0 -Dseed.rule3=0 -jar /tmp/l3-r1only/build/libs/codeanalyzer-3.0.0.jar \ + -i "$PWD/src/test/resources/test-applications/daytrader8" -a 4 --no-build -o /tmp/m4/dt-R1only +python3 /tmp/m4/summaries.py /tmp/m4/dt-R1only/analysis.json > /tmp/m4/dt-R1only.txt +wc -l /tmp/m4/before.txt /tmp/m4/dt-R1only.txt +diff /tmp/m4/before.txt /tmp/m4/dt-R1only.txt && echo IDENTICAL +``` + +``` + 286 /tmp/m4/before.txt + 286 /tmp/m4/dt-R1only.txt + 572 total +IDENTICAL +``` + +**The correction must stay scoped to `@entry`.** Generalising it — `seeds.add(e.getDst())` for +*every* rule-1 edge, which the inference above invites — was measured and **loses three edges**, all +at `TradeDirect.completeOrder(Connection, Integer)` argument 1 (`buy` `338:11`, `completeOrder(Integer, +boolean)` `524:19`, `sell` `445:11`): 283 against `before.txt`'s 286. The mechanism is the *same* +conflation, arriving through a WALA edge instead of a synthetic one — `completeOrder`'s `ddg` carries +`{566:5 → …, var orderID, prov:["points-to"]}`, and `566:5` is the node that defines the returned +`orderData`, so seeding `orderID` there borrows `orderData`'s reach to the `return`. Losing it is +nonetheless an under-approximation, which the L4 posture does not permit, and the flow is arguably +real (the returned row is selected by `orderID` through the database). So `@entry` gets the +correction and ordinary def sites keep `src`. + ### Real-application aggregates `daytrader8`, 1229 callables, 141 source files. Same two jars, `-a 4 --no-build`. @@ -523,7 +607,8 @@ real code — passes: 43% more `ddg` edges for ~1% more wall clock. The redundancy evidence is unambiguous: on both corpora, disabling the call-argument-text rule (`SummaryPass.java:248-257`) or the whole-body-span-text rule (`:258-262`) loses zero summary edges, and rule 1 alone reproduces the entire emitted set. That is the retirement case, and entry defs are -what created it — the same experiment run against `53a4029` gives rule 1 alone **zero** edges. But +what created it — the same experiment run against `53a4029` gives rule 1 alone **zero** edges on the +fixture. But rule 1 as currently written seeds the shared `@entry` node, which conflates all of a callable's formals and manufactured 43 false summary edges on daytrader8; retiring the textual rules on top of that would trade a documented, bounded textual over-approximation for an undocumented semantic one diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java index 310427ba..8986a109 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java @@ -9,6 +9,7 @@ import com.ibm.cldk.schema.JParameter; import com.ibm.cldk.schema.JType; import com.ibm.cldk.schema.Span; +import com.ibm.cldk.syntactic_analysis.controlflow.ControlFlowGraph; import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.ArrayList; @@ -36,13 +37,15 @@ *

The relation is derived syntactically, in the weak-update (may-flow, never-drop) * posture the L4 design takes: a parameter reaches the return if the callable's {@code ddg} carries * it there, if a call site passes it on and that callee's own summary returns it, or if the - * parameter is simply named in a body node's source text. That last rule is what makes the - * pass useful at all on ordinary code, because {@link DdgBuilder} gives a parameter no defining node - * — nothing in the {@code ddg} is ever rooted at one. Without it {@code int c(int z) { return z - 3; - * }} has no def-use to follow at all, and {@code int m(int q) { int t = q; return t; }} has only - * {@code (decl → ret, var "t")}, which no seed reaches. The cost is counting a parameter merely + * parameter is simply named in a body node's source text. That last rule is what made the + * pass useful at all on ordinary code back when {@link DdgBuilder} gave a parameter no defining node, + * so nothing in the {@code ddg} could be rooted at one: {@code int c(int z) { return z - 3; }} had no + * def-use to follow at all, and {@code int m(int q) { int t = q; return t; }} had only + * {@code (decl → ret, var "t")}, which no seed reached. The cost is counting a parameter merely * mentioned in a statement as flowing out of it. Over-approximating there is the accepted - * trade. + * trade. {@code DdgBuilder} now defines the formals at {@code @entry}, so the ddg rule reaches these + * shapes on its own and the two text rules have been measured redundant on both corpora; retiring + * them is a separate change. */ public final class SummaryPass { @@ -204,11 +207,12 @@ private static Fn facts(JCallable c, byte[] source) { } /** - * Where a parameter's value is visible, syntactically: the def end of any {@code ddg} edge whose - * access path is rooted at it, any call-site {@code actual_in} vertex whose argument text names - * it, and every body node whose source text names it (see the class javadoc — a parameter - * has no defining node in the {@code ddg}, so text is the only thing that can put it on the map at - * all). + * Where a parameter's value is visible, syntactically: an end of any {@code ddg} edge whose + * access path is rooted at it — the def end for an ordinary edge, the use end for one + * rooted at {@code @entry} (see the comment on that rule) — any call-site {@code actual_in} + * vertex whose argument text names it, and every body node whose source text names it + * (see the class javadoc for why the two text rules exist and why they now have nothing left to + * add). * *

Every kind with a span is seedable, containers included. A {@code branch}/{@code loop}/ * {@code switch} span swallows the statements nested inside it, so seeding one on a name mentioned @@ -241,7 +245,25 @@ private static Set seedsFor(Fn fn, String name, Map span if (ddg != null) { for (JDdgEdge e : ddg) { if (name.equals(base(e.getVar()))) { - seeds.add(e.getSrc()); + // For an entry-rooted edge, the *use* end — the obvious `e.getSrc()` is wrong + // there. A seed is a node id, and the graph `facts` builds is keyed on node + // identity alone (it discards `var`), so seeding a def node also hands this + // parameter the out-edges of every *other* variable defined at that node. + // `@entry` is where that bites hardest: `DdgBuilder` defines all of a callable's + // formals at that one synthetic node, so `getSrc()` collapses every parameter's + // seed set to `{@entry}` and lets one parameter's reach credit them all — 43 + // spurious summary edges on daytrader8, every one of them on a callable of arity + // greater than 1. The dst is where *this* parameter's value is observed, which + // is exactly what the `var`-filtered edge licenses and no more. + // + // A real def site keeps its `src`. The same conflation happens there in + // miniature, but it is the bounded may-flow over-approximation this pass already + // accepts, and narrowing it *drops* flows: doing this unconditionally costs + // three daytrader8 edges at `TradeDirect.completeOrder(Connection, Integer)`, + // where `orderID` reaches the return only through a WALA `points-to` edge rooted + // at the node defining the returned `orderData`. Under-approximation is the one + // direction the L4 posture does not allow, so the correction stops at `@entry`. + seeds.add(ControlFlowGraph.ENTRY.equals(e.getSrc()) ? e.getDst() : e.getSrc()); } } } diff --git a/src/test/java/com/ibm/cldk/schema/L4GateTest.java b/src/test/java/com/ibm/cldk/schema/L4GateTest.java index 9d28876a..6ba4504f 100644 --- a/src/test/java/com/ibm/cldk/schema/L4GateTest.java +++ b/src/test/java/com/ibm/cldk/schema/L4GateTest.java @@ -69,37 +69,40 @@ void paramEdgeAritiesMatchAndNothingDangles() { } /** - * §14's "arity matches" as an actual count. Hand-derived from the four fixture files and + * §14's "arity matches" as an actual count. Hand-derived from the five fixture files and * confirmed against this run: * *

    - *
  • {@code param_in} = 6 — one per argument at each of the six in-project call sites with - * arguments: {@code a→b}, {@code b→c}, {@code even→odd}, {@code odd→even}, - * {@code roundTrip→put}, {@code callFirst→first}. {@code roundTrip→get()} passes none, so - * it contributes none. - *
  • {@code param_out} = 6 — one per site whose callee returns a value: the same four - * {@code Chain}/{@code Mutual} sites, plus {@code callFirst→first} and - * {@code roundTrip→get()}; {@code put} is {@code void}, so that site has no - * {@code actual_out} to reach. - *
  • {@code summary} = 5 — {@code Chain.a}, {@code Chain.b}, {@code Mutual.even}, - * {@code Mutual.odd} and {@code Loops.callFirst}, one shortcut each. + *
  • {@code param_in} = 9 — one per argument at each in-project call site that passes any: + * {@code a→b}, {@code b→c}, {@code even→odd}, {@code odd→even}, {@code roundTrip→put}, + * {@code callFirst→first} and {@code leak→sink} contribute one each; {@code caller→leak} + * contributes two. {@code roundTrip→get()} passes none, so it contributes none. + *
  • {@code param_out} = 7 — one per site whose callee returns a value: the same four + * {@code Chain}/{@code Mutual} sites, plus {@code callFirst→first}, + * {@code roundTrip→get()} and {@code caller→leak}; {@code put} and {@code sink} are + * {@code void}, so those sites have no {@code actual_out} to reach. + *
  • {@code summary} = 6 — {@code Chain.a}, {@code Chain.b}, {@code Mutual.even}, + * {@code Mutual.odd}, {@code Loops.callFirst} and {@code Arity.caller}, one shortcut each. * {@code Heap.roundTrip}'s two sites are a void callee and a no-arg callee, so neither can - * carry one. {@code Loops.callFirst}'s is the one that needs container nodes to be - * seedable: {@code first}'s parameter is named only in its {@code for}-each header. + * carry one, and neither can {@code Arity.leak}'s single void site. {@code Loops.callFirst}'s + * is the one that needs container nodes to be seedable: {@code first}'s parameter is named + * only in its {@code for}-each header. {@code Arity.caller}'s is the one that needs + * per-parameter seeding: its site passes two arguments and only the first comes back, so a + * seed shared across formals would make it two ({@code SummaryPassTest} pins which). *
*/ @Test void overlayCountsAreExactlyWhatTheFixtureImplies() { JsonObject app = root.getAsJsonObject("application"); - assertEquals(6, app.getAsJsonArray("param_in").size(), "param_in: one per argument at a resolved site"); - assertEquals(6, app.getAsJsonArray("param_out").size(), "param_out: one per value-returning site"); + assertEquals(9, app.getAsJsonArray("param_in").size(), "param_in: one per argument at a resolved site"); + assertEquals(7, app.getAsJsonArray("param_out").size(), "param_out: one per value-returning site"); int summaries = 0; for (JsonObject c : callablesById(root).values()) { JsonArray summary = c.getAsJsonArray("summary"); summaries += summary == null ? 0 : summary.size(); } - assertEquals(5, summaries, "summary: one shortcut per pass-through call site"); + assertEquals(6, summaries, "summary: one shortcut per pass-through call site"); } @Test diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java index cb63dd51..b11c3b63 100644 --- a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java @@ -9,6 +9,7 @@ import com.ibm.cldk.schema.JCallEdge; import com.ibm.cldk.schema.JCallable; import com.ibm.cldk.schema.JDdgEdge; +import com.ibm.cldk.schema.JIdEdge; import com.ibm.cldk.schema.JModule; import com.ibm.cldk.schema.JParameter; import com.ibm.cldk.schema.JType; @@ -167,6 +168,29 @@ void aParameterUsedOnlyInALoopHeaderStillReachesTheReturn() throws Exception { assertTrue(callFirst.getSummary().get(0).getDst().endsWith("/actual_out")); } + /** + * Two parameters in one callable, only one of which flows out — the discriminator no other + * fixture callable can be, since every other has arity 0 or 1. {@code Arity.leak(p, q)} returns a + * copy of {@code p} and hands {@code q} to a {@code void} callee, so {@code caller} may shortcut + * its {@code actual_in:0} and must not shortcut its {@code actual_in:1}. + * + *

The negative half is the one with teeth. Seeding a parameter at the def end of its + * {@code ddg} edge seeds every formal at the one shared {@code @entry} node — the reachability + * graph is keyed on node identity, not on {@code var} — so the search follows {@code p}'s edge to + * the return on {@code q}'s behalf and {@code caller} carries two summary edges instead of one. + */ + @Test + void onlyTheParameterThatActuallyFlowsIsShortcut() throws Exception { + Map modules = analyzed(); + JCallable caller = callable(modules, "/Arity/caller(int, int)"); + assertNotNull(caller.getSummary(), "p flows out of leak, so caller shortcuts that argument"); + List shortcut = new ArrayList<>(); + for (JIdEdge e : caller.getSummary()) { + shortcut.add(e.getSrc().substring(e.getSrc().lastIndexOf('/') + 1)); + } + assertEquals(List.of("actual_in:0"), shortcut, "q reaches only a void callee — argument 1 is not a flow"); + } + /** * A parameter that reaches the return through a local — {@code int m(int q) { int t = q; * return t; }} — the commonest Java shape after {@code return q;} itself. Nothing in the ddg is @@ -384,9 +408,9 @@ void summaryEndpointsAreExistingLocalBodyNodes() throws Exception { edges[0]++; })); // Without this the endpoint check passes vacuously on a regression that empties every - // summary. Five is the fixture's whole set: Chain.a→b, Chain.b→c, Mutual.even→odd, - // Mutual.odd→even, Loops.callFirst→first. Heap's two sites cannot carry one (void callee, - // no-arg callee). - assertEquals(5, edges[0], "every fixture summary edge is checked, and there are five of them"); + // summary. Six is the fixture's whole set: Chain.a→b, Chain.b→c, Mutual.even→odd, + // Mutual.odd→even, Loops.callFirst→first and Arity.caller→leak. Heap's two sites cannot + // carry one (void callee, no-arg callee), and neither can Arity.leak→sink (void callee). + assertEquals(6, edges[0], "every fixture summary edge is checked, and there are six of them"); } } diff --git a/src/test/resources/test-applications/l4-sdg-test/src/main/java/com/l4/Arity.java b/src/test/resources/test-applications/l4-sdg-test/src/main/java/com/l4/Arity.java new file mode 100644 index 00000000..ea82e9b2 --- /dev/null +++ b/src/test/resources/test-applications/l4-sdg-test/src/main/java/com/l4/Arity.java @@ -0,0 +1,24 @@ +package com.l4; + +/** + * The fixture's only callable with two parameters, and the only shape that can tell per-parameter + * seeding apart from seeding the shared node every formal is defined at. Only `p` reaches `leak`'s + * return; `q` is handed to a void callee and goes nowhere. Seed a parameter at the def end of its + * ddg edge and both collapse onto `@entry`, `q` walks `p`'s edges to the return, and `caller` gains + * a second summary edge claiming `n` comes back out of `leak`. + */ +public class Arity { + + public int leak(int p, int q) { + int t = p; + sink(q); + return t; + } + + public void sink(int z) { + } + + public int caller(int m, int n) { + return leak(m, n); + } +} From f318e3bfdb65205f831c6777371dd792e57d727b Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 19:41:23 -0700 Subject: [PATCH 6/7] docs(l4): correct @entry-scoping rationale's false counterexample MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A later review established that the three daytrader8 edges cited as the reason the src/dst correction stays scoped to @entry were never a real flow. Their `orderID` var label names the field OrderDataBean.orderID, read off the bean TradeDirect.completeOrder(Connection, Integer) returns (OrderDataBean.java:68 declares it; TradeDirect.java:597,610, 623,626,634 are all `orderData.getOrderID()` call sites) — not that callable's own `orderID` parameter, which is bound into a JDBC call instead and reaches the return, if at all, only through database semantics neither engine models. So generalising the correction would not have dropped a real flow. It would have removed a same-named-field conflation of exactly the class the @entry fix already removes, arriving by name collision with a field rather than a shared synthetic node. The correction still stays scoped to @entry, but for a narrower reason: this is a regression-fix branch restoring the 53a4029 baseline it perturbed, and the L4 posture's bar for tolerating under-approximation isn't met by evidence from two corpora — not because these three edges are real flows. Corrects SummaryPass.java's seedsFor comment and the plan's "must stay scoped to @entry" section to say this; the mechanism explanation in both — why a shared @entry node conflates formals, why ordinary def sites keep `src` — is unchanged. --- docs/design/plans/2026-08-28-l3-entry-defs.md | 28 +++++++++++++++---- .../dataflow/SummaryPass.java | 22 +++++++++++---- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/docs/design/plans/2026-08-28-l3-entry-defs.md b/docs/design/plans/2026-08-28-l3-entry-defs.md index 4c75b104..c0610c05 100644 --- a/docs/design/plans/2026-08-28-l3-entry-defs.md +++ b/docs/design/plans/2026-08-28-l3-entry-defs.md @@ -565,10 +565,28 @@ at `TradeDirect.completeOrder(Connection, Integer)` argument 1 (`buy` `338:11`, boolean)` `524:19`, `sell` `445:11`): 283 against `before.txt`'s 286. The mechanism is the *same* conflation, arriving through a WALA edge instead of a synthetic one — `completeOrder`'s `ddg` carries `{566:5 → …, var orderID, prov:["points-to"]}`, and `566:5` is the node that defines the returned -`orderData`, so seeding `orderID` there borrows `orderData`'s reach to the `return`. Losing it is -nonetheless an under-approximation, which the L4 posture does not permit, and the flow is arguably -real (the returned row is selected by `orderID` through the database). So `@entry` gets the -correction and ordinary def sites keep `src`. +`orderData`, so seeding `orderID` there borrows `orderData`'s reach to the `return`. + +**A later read of these three edges found they are not a real flow.** The `var` match is nominal, not +semantic: `orderID` here names the field `OrderDataBean.orderID` (`OrderDataBean.java:68`), read off +the returned bean at the `orderData.getOrderID()` call sites this edge's `dst` reaches +(`TradeDirect.java:597,610,623,626,634`) — not `completeOrder(Connection, Integer)`'s own parameter of +the same spelling. That parameter is bound into a JDBC call instead (`stmt.setInt(1, +orderID.intValue())` at `:557`), and the row the query returns only *happens* to carry a column also +named `orderID`; neither engine models that connection. So generalising the correction would not have +dropped a real flow here — it would have removed a same-named-field conflation of exactly the class +the `@entry` fix exists to remove, arriving by name collision with an unrelated field rather than by a +shared synthetic node. + +That changes *why* the correction stays scoped to `@entry`, not whether it should. This branch is a +regression fix: its job is restoring the `53a4029` baseline it perturbed — the 286-edge set these +three already belonged to — not auditing every other instance of the same over-seeding. And the L4 +posture's asymmetry still applies on its own terms: it forbids under-approximation, and "measured safe +on `l4-sdg-test` and daytrader8" is not the same claim as "safe everywhere" — widening `e.getDst()` to +every rule-1 edge, not just entry-rooted ones, is a precision reform whose burden of proof (that it +drops no real flow on some corpus neither of these two happens to contain) belongs to a follow-on that +can gather it, not to this fix riding in on two corpora's worth of evidence. So `@entry` gets the +correction and ordinary def sites keep `src`, for now. ### Real-application aggregates @@ -638,4 +656,4 @@ proven; the follow-on should re-derive the argument before relying on it. - **Spec coverage:** #204's three goals map to Tasks 1 (entry defs + k-limit parity), 2 (measurement), 3 (spec correction). Its fourth DoD item — `L3 ⊆ L4` still holding and the L4 gate's counts updated — is Task 2 Step 5. - **Placeholder scan:** the Measurement section is deliberately a template Task 2 fills; every other step carries runnable commands or complete code. Task 2 Step 2's per-rule findings cannot be pre-written because they are the experiment's output, but the procedure and the stop condition (a lost summary edge is a defect) are concrete. - **Type consistency:** `build(cfg, fieldDepth, formals)` and `L3Overlays.build(..., formals)` use `List` in both the interface block and the code; `JParameter::getName` matches the model; `ControlFlowGraph.ENTRY` is the existing constant, not a new literal. -- **Known risk not designed away:** Task 1 seeds defs from parameter *names*, which collides with a local of the same name in an inner scope — the reaching-definitions kill at the reassignment handles the common case (covered by `aLocalShadowingTheFormalKillsTheEntryDefinition`), but a catch parameter or a lambda parameter shadowing a formal is not covered by any test here and is called out in #204's caveats. If Task 2's measurement shows spurious edges from shadowing, that is a finding for the follow-on, not a reason to hold Task 1. +- **Known risk not designed away:** Task 1 seeds defs from parameter *names*, which collides with a local of the same name in an inner scope — the reaching-definitions kill at the reassignment handles the common case (covered by `aReassignmentKillsTheEntryDefinition`), but a catch parameter or a lambda parameter shadowing a formal is not covered by any test here and is called out in #204's caveats. If Task 2's measurement shows spurious edges from shadowing, that is a finding for the follow-on, not a reason to hold Task 1. diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java index 8986a109..164ac181 100644 --- a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPass.java @@ -258,11 +258,23 @@ private static Set seedsFor(Fn fn, String name, Map span // // A real def site keeps its `src`. The same conflation happens there in // miniature, but it is the bounded may-flow over-approximation this pass already - // accepts, and narrowing it *drops* flows: doing this unconditionally costs - // three daytrader8 edges at `TradeDirect.completeOrder(Connection, Integer)`, - // where `orderID` reaches the return only through a WALA `points-to` edge rooted - // at the node defining the returned `orderData`. Under-approximation is the one - // direction the L4 posture does not allow, so the correction stops at `@entry`. + // accepts. Doing this unconditionally was measured against the pre-change + // baseline and found to cost three daytrader8 edges at + // `TradeDirect.completeOrder(Connection, Integer)`, all under the `var` label + // `orderID` — but that label is nominal, not semantic: those edges are rooted at + // the node defining `orderData` and reach a `getOrderID()` call on it, so `var` + // names the *field* `OrderDataBean.orderID`, not this callable's own parameter of + // the same spelling. The parameter reaches the return, if at all, only through + // JDBC that neither this engine nor WALA models, so the three are a conflation of + // the same class this fix removes, not a real flow it would drop. + // + // The scoping still stops at `@entry`, though — for a narrower reason than "it + // would cost something real". This branch is a regression fix: its job is + // restoring the baseline it perturbed, not auditing every other instance of this + // same over-seeding. And the L4 posture forbids under-approximation on evidence + // this thin — widening `src` to `dst` for every rule-1 edge, not just entry-rooted + // ones, is measured safe on only two corpora, not enough to rule out dropping a + // real flow somewhere neither has been run. That burden belongs to a follow-on. seeds.add(ControlFlowGraph.ENTRY.equals(e.getSrc()) ? e.getDst() : e.getSrc()); } } From 6d203bf5a48d8c88b9f8bf3a1bb3dfd2b66431b6 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Fri, 28 Aug 2026 19:41:35 -0700 Subject: [PATCH 7/7] test(l3): rebuild differential gate's AST oracle with formals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L3DifferentialGateTest built its AST-engine reference oracle with the legacy two-argument DdgBuilder.build(astG, 3), which passes no formals. Every TARGET_METHODS callable takes a parameter, so the oracle never carried the @entry-rooted edges the shipped engine now emits — exactly the divergence entry defs created — leaving the gate blind to it. Extracts each method's parameter names the same way DdgBuilderEntryDefsTest.ddgOf does and calls the three-argument overload instead. The DDG section is a report, not an equality assertion, so the widened delta (7 more AST-only edges across the four target methods, all @entry-rooted) is expected, not a failure. Also corrects two SummaryPassTest javadocs left over from before @entry defined the formals: a parameter does now have a ddg def site, and Loops.first's summary edge no longer depends solely on textual seeding (the ddg-rooted rule reaches it too, redundantly). The hand-built fixtures the comments sit above are still valid regression coverage for the textual rules and are unchanged. Renames DdgBuilderEntryDefsTest's aLocalShadowingTheFormalKillsTheEntryDefinition to aReassignmentKillsTheEntryDefinition: its case is `q = 5;`, a reassignment, not shadowing — a local can't shadow a formal in Java, since redeclaring it doesn't compile. --- .../cldk/schema/L3DifferentialGateTest.java | 15 ++++++++++++++- .../dataflow/DdgBuilderEntryDefsTest.java | 4 +++- .../dataflow/SummaryPassTest.java | 18 ++++++++++++------ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/ibm/cldk/schema/L3DifferentialGateTest.java b/src/test/java/com/ibm/cldk/schema/L3DifferentialGateTest.java index db663b5c..71dacc2b 100644 --- a/src/test/java/com/ibm/cldk/schema/L3DifferentialGateTest.java +++ b/src/test/java/com/ibm/cldk/schema/L3DifferentialGateTest.java @@ -16,6 +16,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.github.javaparser.StaticJavaParser; +import com.github.javaparser.ast.body.MethodDeclaration; import com.github.javaparser.ast.stmt.BlockStmt; import com.github.javaparser.ast.stmt.Statement; import com.ibm.cldk.CodeAnalyzer; @@ -214,7 +216,18 @@ void differentialGateCfgCdgAgreeDdgDeltaReported(@TempDir Path tmp) throws Excep // ---- AST engine (reference oracle) -------------------------------------------------- ControlFlowGraph astG = CfgBuilder.build(body, new LinkedHashMap<>(), ctx); List astCdg = CdgBuilder.build(astG); - List astDdg = DdgBuilder.build(astG, 3); + // Every TARGET_METHODS entry takes at least one parameter, so building the reference + // oracle's ddg without formals (the legacy two-arg overload) would leave it blind to the + // @entry-rooted edges the shipped engine now emits — exactly the divergence this gate + // exists to compare against WALA. Extract the formals the same way + // DdgBuilderEntryDefsTest.ddgOf does and use the three-arg overload instead. + MethodDeclaration astMethod = StaticJavaParser.parse(FIXTURE) + .findAll(MethodDeclaration.class).stream() + .filter(m -> m.getNameAsString().equals(methodName)) + .findFirst().orElseThrow(); + List formals = astMethod.getParameters().stream() + .map(p -> p.getNameAsString()).collect(Collectors.toList()); + List astDdg = DdgBuilder.build(astG, 3, formals); // ---- WALA engine ------------------------------------------------------------------- WalaAnalysis.MethodIr mir = findMethod(wala, methodName); diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java index f00bb2c2..88fd2970 100644 --- a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/DdgBuilderEntryDefsTest.java @@ -54,8 +54,10 @@ void anUnusedParameterProducesNoEdge() { } @Test - void aLocalShadowingTheFormalKillsTheEntryDefinition() { + void aReassignmentKillsTheEntryDefinition() { // `q` is reassigned before the read, so the read depends on the assignment, not on @entry. + // (A local can't shadow a formal — redeclaring `q` in its own method body doesn't compile — + // this is a plain reassignment, which is a distinct kill case worth its own regression test.) List ddg = ddgOf("class T { int m(int q) { q = 5; return q; } }", "m"); assertTrue(ddg.stream().noneMatch(e -> "@entry".equals(e.getSrc())), "the reassignment kills the entry def before any use reaches it: " + ddg); diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java index b11c3b63..fd6989cf 100644 --- a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SummaryPassTest.java @@ -153,10 +153,14 @@ void anUnresolvedWrapperDoesNotBreakComposition() { * the real pipeline, because that is the point: {@link com.ibm.cldk.syntactic_analysis.controlflow.BodyNodeBuilder} * gives a for-each exactly one {@code loop} node spanning the whole statement (the header never * gets a node of its own), and {@link DdgBuilder} attributes the iterated expression and the loop - * variable to that node. So the loop node has a real outgoing ddg edge to {@code return x;}, but - * the only thing that can ever seed it for {@code q} is its own span text — a parameter has no ddg - * def site. With container kinds excluded from seeding, {@code flows(first)} was empty and the - * caller lost a summary edge for a flow that genuinely exists. + * variable to that node. So the loop node has a real outgoing ddg edge to {@code return x;}, and + * {@code q} is now defined at {@code @entry} too, so {@code DdgBuilder} roots a second edge there + * — {@code @entry → loop node} — which the ddg-rooted seeding rule alone now rides to the return. + * Before {@code @entry} defined the formals, a parameter had no ddg def site at all and the loop + * node's own span text was the only thing that could ever seed it; that text route still finds it + * too, redundantly. With container kinds excluded from seeding altogether, both routes vanish and + * {@code flows(first)} would again be empty — the caller would lose a summary edge for a flow that + * genuinely exists. */ @Test void aParameterUsedOnlyInALoopHeaderStillReachesTheReturn() throws Exception { @@ -193,8 +197,10 @@ void onlyTheParameterThatActuallyFlowsIsShortcut() throws Exception { /** * A parameter that reaches the return through a local — {@code int m(int q) { int t = q; - * return t; }} — the commonest Java shape after {@code return q;} itself. Nothing in the ddg is - * rooted at {@code q}: {@code DdgBuilder} gives a parameter no defining node, so the only edge is + * return t; }} — the commonest Java shape after {@code return q;} itself. The fixture is built by + * hand with no edge rooted at {@code q}: {@code DdgBuilder} now defines every formal at + * {@code @entry}, but this test exists to pin the textual fallback in isolation, so it + * constructs the pre-{@code @entry} shape on purpose — the only edge is * {@code (decl → ret, var "t")}; there is no call site; and the return text names {@code t}, not * {@code q}. The seed therefore has to come from the declaration statement's own text, * which is what the widened word-boundary rule supplies — the existing {@code decl → ret} edge