From 5d449174daa89c98265b913a7c8f838a43592ce0 Mon Sep 17 00:00:00 2001 From: Rahul Krishna Date: Mon, 7 Sep 2026 00:14:58 -0400 Subject: [PATCH] fix(l4): join the SDG port lattice to the statement ddg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The L4 ports and the param_in/param_out/summary edges between them were emitted, but no dependence edge ever touched a port: a formal_in had out-degree zero, so the SDG was two disjoint graphs. Every interprocedural value question then degenerated to an empty answer indistinguishable from 'no flow exists', which is why the SDK refuses slice_forward, paths_between, flows_to_call and flows_to_argument on a Java graph today. SdgPortEdges adds the four missing edge classes, each a re-rooting of something the statement graph already states, so the pass stays pure and engine-free: @formal_in:k -> use mirrors the @entry definition of parameter k return -> @formal_out the statements that hand a value back statement -> actual_in:i the statement that evaluates the arguments actual_out -> statement the statement that consumes the result A call's own body node carries no dependence edges, so the actuals attach to the innermost statement containing the call — or to the call itself for a bare-call statement, where the two share an addressing anchor. That attachment is deliberately coarse: L4 may add reach, never drop it, and the per-argument binding is recoverable from the statement's own incoming edges. Runs after SummaryPass, so summaries are still computed from the statement-level ddg alone. No schema surface changes: no new node kind, edge type or field, and prov stays the closed {ssa, points-to} enum, so the graph contract holds at 2.0.0. On daytrader8 at -a 4: 0 -> 5,108 ddg edges touching a port, and formal_in ports with out-degree zero fall from 1,176 to 140 — all 140 parameters that no statement reads (methods whose body is a single throw, and one unread argument). Output validates against analysis.v2.schema.json and two runs are byte-identical. Closes #227 --- src/main/java/com/ibm/cldk/CodeAnalyzer.java | 4 + .../dataflow/SdgPortEdges.java | 268 ++++++++++++++++++ .../dataflow/SdgPortEdgesTest.java | 151 ++++++++++ 3 files changed, 423 insertions(+) create mode 100644 src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdges.java create mode 100644 src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdgesTest.java diff --git a/src/main/java/com/ibm/cldk/CodeAnalyzer.java b/src/main/java/com/ibm/cldk/CodeAnalyzer.java index 60722375..9e968cea 100644 --- a/src/main/java/com/ibm/cldk/CodeAnalyzer.java +++ b/src/main/java/com/ibm/cldk/CodeAnalyzer.java @@ -36,6 +36,7 @@ import com.ibm.cldk.syntactic_analysis.L1Cache; import com.ibm.cldk.syntactic_analysis.L1Extractor; import com.ibm.cldk.syntactic_analysis.L2CallGraph; +import com.ibm.cldk.syntactic_analysis.dataflow.SdgPortEdges; import com.ibm.cldk.syntactic_analysis.dataflow.SdgVertices; import com.ibm.cldk.syntactic_analysis.dataflow.SummaryPass; import com.ibm.cldk.utils.BuildProject; @@ -569,6 +570,9 @@ private void analyzeV2() throws Exception { sdg = SdgVertices.apply(modules); // Summaries read the vertices SdgVertices just added, so this must follow it. SummaryPass.apply(modules, l2.callGraph(), graphFieldDepth); + // Joins those vertices to the statement ddg (#227). Strictly last: summaries are + // computed from the statement-level ddg alone, as they always were. + SdgPortEdges.apply(modules); } analysis = V2Emitter.emit(application, analysisLevel, modules, version, l2.callGraph(), l2.externalSymbols(), diff --git a/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdges.java b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdges.java new file mode 100644 index 00000000..027ac535 --- /dev/null +++ b/src/main/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdges.java @@ -0,0 +1,268 @@ +package com.ibm.cldk.syntactic_analysis.dataflow; + +import com.ibm.cldk.schema.JBodyNode; +import com.ibm.cldk.schema.JCallable; +import com.ibm.cldk.schema.JDdgEdge; +import com.ibm.cldk.schema.JModule; +import com.ibm.cldk.schema.JParameter; +import com.ibm.cldk.schema.JType; +import com.ibm.cldk.schema.Span; +import com.ibm.cldk.utils.Log; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Comparator; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +/** + * L4 stage 8, derived: the {@code ddg} edges that join the SDG port lattice to the statement graph. + * + *

{@link SdgVertices} mints the ports and the cross-function {@code param_in}/{@code param_out} + * edges, and {@link SummaryPass} shortcuts each call site — but nothing ever flowed into or + * out of a port, so the two halves were disjoint graphs: a statement-level {@code ddg} and + * a port lattice reachable from it in neither direction. A {@code formal_in} — the natural seed for + * "what does this argument reach" — had out-degree zero, which makes every interprocedural value + * question degenerate to an empty answer indistinguishable from "no flow exists". + * + *

Four edge classes close the gap, each a re-rooting of something the statement graph already + * says, so this pass is pure and engine-free: + *

+ * + *

The argument attachment is deliberately coarse: every actual of a call site is fed by the one + * statement containing it, rather than by the reaching definition of that particular argument + * expression. L4 is over-approximate — it may add reach, never drop it — and the precise binding + * is recoverable from the statement's own incoming edges. + * + *

Runs after {@link SummaryPass} so summaries are computed from the statement-level {@code ddg} + * alone, exactly as before. + */ +public final class SdgPortEdges { + + private SdgPortEdges() {} + + /** Body-node kinds that carry dependence edges, i.e. can host a port attachment. */ + private static final Set STATEMENT_KINDS = new HashSet<>( + Arrays.asList("statement", "return", "branch", "loop", "switch", "expression", "block")); + + private static final String ENTRY = "@entry"; + private static final String FORMAL_OUT = "@formal_out"; + private static final String RETURN_VALUE = "$ret"; + + /** Mutates each callable's {@code ddg} in place. */ + public static void apply(Map modules) { + int added = 0; + int touched = 0; + for (JCallable c : callables(modules)) { + int n = connect(c); + if (n > 0) { + added += n; + touched++; + } + } + Log.info("L4 SDG port edges: " + added + " ddg edge(s) added across " + touched + " callable(s)"); + } + + /** @return the number of edges added to {@code c}. */ + private static int connect(JCallable c) { + Map body = c.getBody(); + if (body == null || body.isEmpty() || c.getDdg() == null) { + return 0; + } + + List produced = new ArrayList<>(); + formalInEdges(c, body, produced); + formalOutEdges(c, body, produced); + actualEdges(c, body, produced); + return merge(c, produced); + } + + /** + * {@code @formal_in:k → use}, mirroring the {@code @entry} definition of parameter {@code k}. + * + *

Only the AST engine roots parameter flow at {@code @entry}; the WALA L3 engine keeps + * {@code NormalStatement} endpoints only and so has no such edge to mirror, leaving its + * {@code formal_in} ports unattached. + */ + private static void formalInEdges(JCallable c, Map body, List out) { + List params = c.getParameters(); + for (JDdgEdge e : c.getDdg()) { + if (!ENTRY.equals(e.getSrc()) || e.getVar() == null) { + continue; + } + String base = baseOf(e.getVar()); + for (int k = 0; k < params.size(); k++) { + if (!base.equals(params.get(k).getName())) { + continue; + } + String port = "@formal_in:" + k; + if (body.containsKey(port)) { + out.add(edge(port, e.getDst(), e.getVar(), e.getProv())); + } + break; + } + } + } + + /** {@code return → @formal_out} for every returning statement of a value-returning callable. */ + private static void formalOutEdges(JCallable c, Map body, List out) { + if (!body.containsKey(FORMAL_OUT)) { + return; // void, or a constructor: no value leaves through a port + } + for (Map.Entry entry : body.entrySet()) { + if ("return".equals(entry.getValue().getKind())) { + out.add(edge(entry.getKey(), FORMAL_OUT, RETURN_VALUE, ssa())); + } + } + } + + /** {@code statement → actual_in:i} and {@code actual_out → statement}, per call site. */ + private static void actualEdges(JCallable c, Map body, List out) { + for (Map.Entry entry : new ArrayList<>(body.entrySet())) { + if (!"call".equals(entry.getValue().getKind())) { + continue; + } + String call = entry.getKey(); + String host = enclosingStatement(call, entry.getValue(), body); + for (int i = 0; body.containsKey(call + "/actual_in:" + i); i++) { + String port = call + "/actual_in:" + i; + out.add(edge(host, port, argumentName(body.get(port), i), ssa())); + } + String actualOut = call + "/actual_out"; + if (body.containsKey(actualOut)) { + out.add(edge(actualOut, host, RETURN_VALUE, ssa())); + } + } + } + + /** + * The innermost statement-like body node whose source range contains {@code call}, or the call + * itself when none does — which is the bare-call statement {@code foo(x);}, whose statement node + * and call node are one and the same (they share an addressing anchor). + */ + private static String enclosingStatement(String call, JBodyNode node, Map body) { + Span span = node.getSpan(); + if (span == null || span.getBytes() == null) { + return call; + } + String best = call; + long bestWidth = Long.MAX_VALUE; + for (Map.Entry candidate : body.entrySet()) { + if (candidate.getKey().equals(call) || !STATEMENT_KINDS.contains(candidate.getValue().getKind())) { + continue; + } + Span s = candidate.getValue().getSpan(); + if (s == null || s.getBytes() == null) { + continue; + } + if (s.getBytes()[0] <= span.getBytes()[0] && span.getBytes()[1] <= s.getBytes()[1]) { + long width = (long) s.getBytes()[1] - s.getBytes()[0]; + if (width < bestWidth) { + bestWidth = width; + best = candidate.getKey(); + } + } + } + return best; + } + + /** The value an {@code actual_in} port stands for — its own {@code of}, or {@code argN}. */ + private static String argumentName(JBodyNode port, int index) { + return port != null && port.getOf() != null ? port.getOf() : "arg" + index; + } + + /** The base segment of a k-limited access path ({@code a.b[*]} → {@code a}). */ + private static String baseOf(String accessPath) { + int cut = accessPath.length(); + for (int i = 0; i < accessPath.length(); i++) { + char ch = accessPath.charAt(i); + if (ch == '.' || ch == '[') { + cut = i; + break; + } + } + return accessPath.substring(0, cut); + } + + /** + * Appends the edges of {@code produced} that {@code c} does not already carry, then re-sorts on + * {@code (src,dst,var,prov)} — the key every ddg producer sorts on. + * + * @return the number of edges actually added + */ + private static int merge(JCallable c, List produced) { + if (produced.isEmpty()) { + return 0; + } + List merged = new ArrayList<>(c.getDdg()); + Set seen = new HashSet<>(); + for (JDdgEdge e : merged) { + seen.add(key(e)); + } + int added = 0; + for (JDdgEdge e : produced) { + if (seen.add(key(e))) { + merged.add(e); + added++; + } + } + if (added == 0) { + return 0; + } + merged.sort(Comparator.comparing(JDdgEdge::getSrc) + .thenComparing(JDdgEdge::getDst) + .thenComparing(JDdgEdge::getVar) + .thenComparing(e -> e.getProv().toString())); + c.setDdg(merged); + return added; + } + + private static String key(JDdgEdge e) { + return e.getSrc() + "\0" + e.getDst() + "\0" + e.getVar() + "\0" + e.getProv(); + } + + private static List ssa() { + List prov = new ArrayList<>(1); + prov.add("ssa"); + return prov; + } + + private static JDdgEdge edge(String src, String dst, String var, List prov) { + JDdgEdge e = new JDdgEdge(); + e.setSrc(src); + e.setDst(dst); + e.setVar(var); + e.getProv().addAll(prov); + return e; + } + + /** Every callable reachable from {@code modules}, mirroring {@link SdgVertices}'s traversal. */ + private static List callables(Map modules) { + List out = new ArrayList<>(); + for (JModule m : modules.values()) { + walkTypes(m.getTypes(), out::add); + } + return out; + } + + private static void walkTypes(Map types, Consumer visitor) { + for (JType t : types.values()) { + walkTypes(t.getTypes(), visitor); + for (JCallable c : t.getCallables().values()) { + visitor.accept(c); + walkTypes(c.getTypes(), visitor); + } + } + } +} diff --git a/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdgesTest.java b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdgesTest.java new file mode 100644 index 00000000..94d4fe0f --- /dev/null +++ b/src/test/java/com/ibm/cldk/syntactic_analysis/dataflow/SdgPortEdgesTest.java @@ -0,0 +1,151 @@ +package com.ibm.cldk.syntactic_analysis.dataflow; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import com.ibm.cldk.schema.JBodyNode; +import com.ibm.cldk.schema.JCallable; +import com.ibm.cldk.schema.JDdgEdge; +import com.ibm.cldk.schema.JModule; +import com.ibm.cldk.schema.V2Json; +import com.ibm.cldk.syntactic_analysis.L1Extractor; +import com.ibm.cldk.syntactic_analysis.L2CallGraph; +import java.nio.file.Paths; +import java.util.Arrays; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; +import org.junit.jupiter.api.Test; + +class SdgPortEdgesTest { + + private static final String FIXTURE = "src/test/resources/test-applications/l4-sdg-test"; + + private static final Set PORT_KINDS = + new HashSet<>(Arrays.asList("formal_in", "formal_out", "actual_in", "actual_out")); + + private static Map analyzed() throws Exception { + Map modules = L1Extractor.extractAll( + Paths.get(FIXTURE), "l4-sdg-test", null, new LinkedHashMap<>(), 3, 3, "ast"); + L2CallGraph.Result l2 = L2CallGraph.build("l4-sdg-test", modules, null, true); + SdgVertices.apply(modules); + SummaryPass.apply(modules, l2.callGraph(), 3); + SdgPortEdges.apply(modules); + return modules; + } + + private static List allCallables(Map modules) { + return modules.values().stream() + .flatMap(m -> m.getTypes().values().stream()) + .flatMap(t -> t.getCallables().values().stream()) + .collect(Collectors.toList()); + } + + private static JCallable callable(Map modules, String idSuffix) { + return allCallables(modules).stream() + .filter(c -> c.getId().endsWith(idSuffix)) + .findFirst() + .orElseThrow(); + } + + private static boolean hasEdge(JCallable c, String src, String dst) { + return c.getDdg().stream().anyMatch(e -> e.getSrc().equals(src) && e.getDst().equals(dst)); + } + + private static String kind(JCallable c, String localId) { + JBodyNode n = c.getBody().get(localId); + return n == null ? null : n.getKind(); + } + + @Test + void aFormalInReachesTheStatementsThatReadTheParameter() throws Exception { + Map modules = analyzed(); + JCallable a = callable(modules, "/Chain/a(int)"); + + // Chain.a is `return b(x + 1);` — the one statement reading x, which @entry already defines. + List fromPort = a.getDdg().stream() + .filter(e -> "@formal_in:0".equals(e.getSrc())) + .collect(Collectors.toList()); + assertFalse(fromPort.isEmpty(), "formal_in:0 must not have out-degree zero"); + for (JDdgEdge e : fromPort) { + assertEquals("x", e.getVar(), "the edge carries the parameter's own access path"); + assertTrue( + hasEdge(a, "@entry", e.getDst()), + "every formal_in edge mirrors an @entry definition of the same parameter"); + } + } + + @Test + void returningStatementsReachFormalOut() throws Exception { + Map modules = analyzed(); + JCallable c = callable(modules, "/Chain/c(int)"); + + List returns = c.getBody().entrySet().stream() + .filter(e -> "return".equals(e.getValue().getKind())) + .map(Map.Entry::getKey) + .collect(Collectors.toList()); + assertFalse(returns.isEmpty(), "the fixture returns a value"); + for (String r : returns) { + assertTrue(hasEdge(c, r, "@formal_out"), r + " → @formal_out"); + } + } + + @Test + void callSitePortsAreAttachedToTheStatementThatCarriesThem() throws Exception { + Map modules = analyzed(); + JCallable a = callable(modules, "/Chain/a(int)"); + + String call = a.getBody().entrySet().stream() + .filter(e -> "call".equals(e.getValue().getKind())) + .map(Map.Entry::getKey) + .findFirst() + .orElseThrow(); + String actualIn = call + "/actual_in:0"; + String actualOut = call + "/actual_out"; + + String host = a.getDdg().stream() + .filter(e -> e.getDst().equals(actualIn)) + .map(JDdgEdge::getSrc) + .findFirst() + .orElseThrow(() -> new AssertionError(actualIn + " has in-degree zero")); + assertTrue(hasEdge(a, actualOut, host), actualOut + " → " + host); + assertTrue( + a.getBody().containsKey(host) && !PORT_KINDS.contains(kind(a, host)), + "the attachment point is a real body node that carries dependence edges: " + host); + } + + @Test + void everyPortEdgeResolvesToABodyNodeOfItsOwnCallable() throws Exception { + Map modules = analyzed(); + for (JCallable c : allCallables(modules)) { + if (c.getDdg() == null) { + continue; + } + for (JDdgEdge e : c.getDdg()) { + assertTrue(c.getBody().containsKey(e.getSrc()), c.getId() + " src " + e.getSrc()); + assertTrue(c.getBody().containsKey(e.getDst()), c.getId() + " dst " + e.getDst()); + } + } + } + + @Test + void theLatticeIsNoLongerAnIsland() throws Exception { + Map modules = analyzed(); + long touching = allCallables(modules).stream() + .filter(c -> c.getDdg() != null) + .flatMap(c -> c.getDdg().stream() + .filter(e -> PORT_KINDS.contains(kind(c, e.getSrc())) + || PORT_KINDS.contains(kind(c, e.getDst())))) + .count(); + assertTrue(touching > 0, "at least one ddg edge touches a port"); + } + + @Test + void isDeterministic() throws Exception { + assertEquals(V2Json.compact().toJson(analyzed()), V2Json.compact().toJson(analyzed())); + } +}