Precision & recall improvements for the Andersen points-to analysis#2
Open
drmckay wants to merge 28 commits into
Open
Precision & recall improvements for the Andersen points-to analysis#2drmckay wants to merge 28 commits into
drmckay wants to merge 28 commits into
Conversation
The Andersen worklist re-unioned the whole points-to set of a variable into every subset successor on every fire. PointsToSet already shipped the delta primitives (diffBits/absorb) but they were unused. Track, per variable, the allocation-site indices already propagated out of it and push only the delta each fire. addSubsetEdge (full seed of a newly created edge) and the field / virtual-call discharges (idempotent / seen-guarded, read the full current set) are unchanged, so the least fixpoint is identical — this is a speedup, not a semantics change. Verified: x2cpg compiles; javasrc2cpg CallGraphTests 7/7 pass.
The solver keyed pt / subsetOut / propagated / loads / stores / vcalls / worklist by the packed "$ctx|$var" string, so every hot-loop map access hashed and compared a full string. Intern each packed variable to a dense int id once (when it is first built) and key all hot-path maps by that id; the fixpoint loop no longer touches a string. solve() re-keys its result back to the packed strings for consumers, so PointerAnalysis is unchanged. This also sets up array-backed cycle elimination (union-find over int ids) as the next step. Verified: x2cpg compiles; javasrc2cpg CallGraphTests + CallTests + ConstructorInvocationTests 27/27 pass.
Recursive and mutually-recursive copies create cycles in the subset (copy) graph; every member of a strongly connected component shares one final points-to set, but without collapsing them the worklist churns the same allocation sites around the cycle O(cycle length) times. Add union-find over the interned variable ids. While propagating along v -> d, when d's set is left unchanged and equal to v's and d can reach v in the subset graph, v and d are in the same SCC and are merged (deferred until after the propagation loop; the merged node's delta bookkeeping is reset and it is re-enqueued). All hot-path map accesses resolve through find(). The call graph is unaffected: refineCallEdges reads resolvedCallTargets, keyed by call node id and derived from receiver alloc types, independent of how points-to variables are grouped. Verified: x2cpg compiles; new AndersenSolverCycleTests (copy-cycle collapse, acyclic control, two-alloc cycle) pass; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests and dataflow ObjectTests pass. The dataflow MethodReturnTests 'external method with semantic' case fails identically on pristine upstream aa3c8e0, so it is a pre-existing failure unrelated to this change.
When a base variable's points-to set grew, every deferred load/store on it was re-discharged against the whole set, each call re-iterating all base allocations and allocating a fresh Set[String] of their types — pure waste after the first fire, since addSubsetEdge is idempotent. Track, per base, the allocation indices already wired into field slots; on each fire wire only the newly-arrived allocations' types into every deferred load/store slot. A newly added load/store constraint still gets the full current base set once via dischargeLoad/dischargeStore in interpret. A cycle collapse resets the per-base bookkeeping (like the propagation delta) and re-enqueues the representative. Verified: x2cpg compiles; new AndersenSolverFieldTests (store->load flow, two-type base growth, distinct field names) and AndersenSolverCycleTests pass; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests and dataflow ObjectTests pass.
add / unionInPlace / absorb each computed mutable.BitSet.size (a full popcount over all words) before and after the mutation just to report whether the set changed, making a single-element add O(setSize/64) and every union two full popcounts. add now uses mutable.BitSet.add, which is O(1) and already reports whether the bit was newly set. unionInPlace / absorb compute the difference (delta &~ bits), which short-circuits on the first differing word, and union only when it is non-empty; unionInPlace delegates to absorb. Change-report semantics are identical. Verified: AndersenSolverCycleTests, AndersenSolverFieldTests and javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests pass.
enqueue never checked whether a variable was already queued, so a hot variable could sit in the FIFO many times and be dequeued and fully re-processed (all lookups + discharges) with no new information. Track the representatives currently on the worklist in a set; enqueue is a no-op when the variable is already queued. The flag is cleared on dequeue *before* processing, so a re-enqueue triggered during processing still takes effect, and a cycle collapse clears the merged node's flag. The fixpoint result is unchanged (delta propagation already ensures a dequeue processes the variable's current set), only redundant fires are removed. Verified: AndersenSolverCycleTests, AndersenSolverFieldTests and javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests pass.
dischargeVirtualCall re-materialised the whole receiver points-to set (rset.iterator.toArray) on every fire and then filtered each element through the per-call `seen` set — an O(|receiver set|) scan plus array allocation even when `seen` rejected every element. `seen` is a mutable.BitSet now, so the discharge computes the delta directly (rset.diffBits(seen)) and returns immediately when no new receiver allocation has arrived; only the new allocation sites are resolved and instantiated. Semantics are unchanged — the same allocations are processed exactly once per call site. Verified: AndersenSolverCycleTests, AndersenSolverFieldTests and javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests pass.
pt.getOrElse(x, PointsToSet.empty) allocated a fresh throwaway PointsToSet (and its mutable.BitSet) on every read-miss, immediately discarded. Add a shared read-only PointsToSet.EMPTY sentinel and use it for the eight getOrElse read-miss lookups, all of which only read the result (nonEmpty / diffBits / typesOf) and short-circuit when it is empty, so the sentinel is never mutated. The five getOrElseUpdate sites, which insert and mutate, keep allocating a fresh instance. Verified: AndersenSolverCycleTests, AndersenSolverFieldTests and javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests pass.
`new Foo(...)` lowers to a Block whose last child is the tmp identifier holding the freshly allocated object; its inner `$tmp = <operator>.alloc` assignment is already turned into an Alloc by handleAssignment. But exprVar mapped the Block to None, so an allocation in argument / return / nested position was dropped and the object type never entered any points-to set — defeating factory/builder dispatch resolution. exprVar now maps a Block to its last expression child (the tmp identifier), which already points to the allocation, so no new Alloc emission is needed. A `return new Impl()` factory dispatch now resolves precisely (new test). An allocation in argument position reaches argVars but is not yet bound to the callee parameter, due to a separate argument->parameter index mismatch (pending test, fixed next). Verified: new PointerAnalysisFactoryTests (return position) passes; javasrc2cpg CallGraphTests, ConstructorInvocationTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
argVars was built with call.argument.flatMap(exprVar), which drops primitive arguments and (on static calls) has no receiver slot, so the resulting Vector no longer aligned with parameter indices — the solver's argVars.lift(paramIndex) bound arguments to the wrong parameter or missed them entirely. Only the CPG AST argument edges were tested, so this points-to mis-binding went unnoticed. Key argVars by Joern argumentIndex (Map[Int, String]) — receiver/this = 0, explicit arguments from 1, matching MethodParameterIn.index — and look them up with .get(index) in both the static and virtual call discharge. Reference arguments (including allocations in argument position from the previous commit) now reach the correct callee parameter. Verified: PointerAnalysisFactoryTests argument-position case now resolves (pending removed); javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
A Java cast lowers to `<operator>.cast(TypeRef, operand)`, an operator call that exprVar rejected and handleAssignment did not match, so `Foo f = (Foo) bar` and casts in argument/return position dropped the object — severing the points-to chain on exactly the values that most need it (collection.get(), Optional.get(), widened factory results). A cast is identity for points-to, so exprVar now maps `<operator>.cast` to its operand (the last argument; the target type is a TypeRef in argument position 1), and a cast RHS in an assignment emits a Copy from the operand. Verified: new cast test (dispatch through `(Greeter) o` resolves precisely to the concrete type) passes; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, SpecialOperatorTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
Array access `a[i]` lowers to `<operator>.indexAccess(base, index)`, an operator call, so an array store (`a[i] = x`, index-access LHS) and an array load (`x = a[i]`, index-access RHS or value position) were both dropped — array elements carried no points-to, and a reference stored into an array never resolved a later dispatch on it. Model an array index-insensitively as an object with one synthetic `[]` element slot, on the existing Load/Store field machinery: `a[i] = x` stores into the base's `[]` slot, `x = a[i]` loads it, and `a[i]` in value position reads the slot. Array allocation (`new T[n]`) already produces a typed `<operator>.alloc`, so the slot resolves per concrete array type. Collections (list.add/get) are handled separately next. Verified: new array-element dispatch test resolves precisely; javasrc2cpg CallGraphTests, ConstructorInvocationTests, ArrayTests, dataflow ArrayTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
list.add(x) / list.get() are virtual calls into JDK types that have no method body in the CPG, so they resolved to nothing: a reference put into a collection never flowed to a later read, and an interface stored in a List never resolved a dispatch on the element. Model a collection element-insensitively as an object with one synthetic <collElem> slot, recognised conservatively by receiver type (java.util.* or a core collection interface name, since the JDK type hierarchy is not in the CPG) and a fixed set of accessor methods: add/offer/push/put/set/... store the last reference argument into the slot, get/poll/peek/remove/... load it. Handled before virtual-call emission, on the existing Load/Store field machinery. Verified: new collection-dispatch test resolves precisely; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, ArrayTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
A lambda or method reference lowers to a MethodRef node whose methodFullName is the synthetic lambda method (or referenced method); exprVar had no case for it, so a functional variable (Runnable/Supplier/Function) carried no points-to and the later r.run() / s.get() dispatch resolved to nothing. Model a MethodRef as an allocation of a functional object whose type is the target method's full name, and resolve any functional dispatch on it straight to that method in lookupMethod (the receiver "type" is itself a method full name). Also handle a block- or method-ref-valued assignment RHS. Also exclude java.util.function.* (functional interfaces) from the collection model, so their accessors (Supplier.get, ...) stay virtual dispatches that resolve to the lambda body instead of being mistaken for collection loads. Verified: new lambda + method-reference dispatch tests resolve to the lambda body / referenced method; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests and the x2cpg solver unit tests pass. The pre-existing LambdaTests "captured variables" AST failure is unrelated (fails identically on upstream aa3c8e0).
Value-position field reads (a field access as a call argument or return operand) used a declared-type slot F:<declaredType>:f via fieldSlotFromAccess, while writes and assignment-loads use the concrete-type slot F:<concreteType>:f resolved from pt(base). For a polymorphic or unknown-typed base these differ, so a value stored into the concrete slot was not read back — silently losing the flow (e.g. sink(obj.f), return obj.f). exprVar now emits a Load through the base pointer variable for a value-position field read, resolving to the same concrete slot the write used. fieldSlotFromAccess is removed. This exposed that a direct `new T(...)` in value position (e.g. a store RHS, which lowers to an <init>/alloc call, not a block) was dropped by exprVar, so the store never reached the concrete field slot. exprVar now models a direct allocation call into a synthetic variable. Verified: new polymorphic-base field-read test resolves precisely; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
lookupMethod tried the exact (name, signature) then fell back to the first name-matching method in an unordered map, so an overloaded call whose call-site signature did not match textually (generics erasure, bridge methods, ANY-typed args) was routed to a nondeterministic, possibly wrong overload — a false-target CALL edge (taint into the wrong body) or a missed target. lookupMethods now returns a set: an exact (name, signature) match gives a single precise target; otherwise it returns ALL same-name overloads on the most specific type that has one. Deterministic and recall-preserving (over-approximate) rather than arbitrary. The virtual call discharge instantiates and binds each resolved target. Verified: new overload test routes to the exact overload (not the decoy); javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests and the x2cpg solver unit tests pass.
A static field access `Type.field` has a TypeRef base (or, inconsistently, a type-named Identifier) rather than an object, so resolving its slot through the base's points-to gave nothing: the write in <clinit> (`INSTANCE = new Foo()`) never discharged and a read (`return INSTANCE`, `Holder.INSTANCE`) saw an empty slot — classic getInstance() singletons carried no object. Recognise a static field access (base is a TypeRef, or an Identifier whose name is a known type) and scope its slot directly to the declaring type (F:Type:field), copying into it on write and out of it on read, instead of the pt(base)-resolved instance-field machinery. Verified: new singleton dispatch test resolves precisely; javasrc2cpg CallGraphTests, CallTests, ConstructorInvocationTests, dataflow ObjectTests, dataflow StaticMemberTests and the x2cpg solver unit tests pass.
collectServiceProxyRegistrations and collectVerticleDeployments hooked every register / registerService / deployVerticle call by name, with no type guard (unlike the Guice and Dropwizard collectors, which gate on ancestorTypeFullNames). Any unrelated same-named call in user code or another framework — e.g. a 3-arg register(Iface.class, impl) — produced a spurious interface->impl binding, a precision defect that misroutes dispatch. Guard each hook with isVertxCall: a genuine Vert.x call resolves to a method under the io.vertx package. Unrelated calls no longer bind; real ServiceBinder.register / ProxyHelper.registerService / Vertx.deployVerticle still do. Verified: new DiCollectorTests (unrelated register not bound; genuine io.vertx register bound); javasrc2cpg CallGraphTests, PointerAnalysisFactoryTests and the x2cpg solver unit tests pass.
seedInjectedFields looked up implsFor on the raw field type of an injected member. For a collection field (@Inject List<Handler>) that raw type is java.util.List, which has no DI bindings, so Spring's "inject all beans of a type" was invisible — the generic element type was never extracted. For a collection-typed injected field, extract the generic element type from the member's JVM generic signature (Ljava.util.List<LHandler;>; -> Handler), model the field as a synthetic collection, and seed its element slot (the same <collElem> slot collection reads use) with an alloc of every impl of the element type. Verified: new collection-injection test resolves a dispatch on an element read from an injected List to every bean impl; javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests and the x2cpg solver unit tests pass.
seedInjectedParameters only injected constructor parameters when the constructor, method, or parameter carried an inject annotation. Spring 4.3+ injects a bean's single constructor implicitly, with no @Autowired, so those parameters were missed. When a DI-managed class (a registered class or a bound impl) has exactly one constructor, autowire its parameters from the DI bindings like an annotated injection point. Verified: new test resolves a dispatch on an unannotated single-constructor parameter to every bean impl; javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests, ConstructorInvocationTests and the x2cpg solver unit tests pass.
seedInjectedFields seeded an injected field with an alloc of every impl of the field type, so an interface with several impls produced several dispatch targets even at a @qualifier("name") pinned site that names exactly one bean — a precision defect (spurious targets). Read the @qualifier value and keep only the impl whose default bean name (simple name decapitalised) matches it. If no impl name matches — e.g. an explicit @component("name") we do not resolve — fall back to all impls, so precision never costs recall. Verified: new test injects only the qualified bean (dispatch resolves to it, not the other impl); javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests and the x2cpg solver unit tests pass.
A Spring FactoryBean<T> supplies its bean from getObject() rather than by being the bean itself, so an @Autowired T resolved to nothing — the SpringCollector never looked at getObject(). Collect a binding from the getObject() return type to the concrete type it allocates and returns (Greeter -> RealGreeter), the same shape as an @bean factory method. The existing field/parameter seeding then injects it. Verified: new FactoryBean test resolves an @Autowired dispatch to the concrete type getObject() returns (not another impl); javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests, CallTests, ConstructorInvocationTests and the x2cpg solver unit tests pass.
Class.forName("Foo").newInstance() creates a Foo, but the JDK newInstance returns Object with
no allocation, so reflectively created objects were invisible and any later dispatch on them
went unresolved.
When a newInstance call's receiver is Class.forName with a string-literal argument, model the
result as an alloc of the named type. Handled before dispatch, since the virtual call into
java.lang.Class resolves to nothing anyway.
Verified: new reflection test allocates the named type and resolves a dispatch through the cast
of the result; javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests,
CallTests and the x2cpg solver unit tests pass.
reflectiveAllocType only recognised Class.forName(name).newInstance(). The common Class.forName(name).getDeclaredConstructor(...).newInstance(...) chain (and getConstructor) was missed, so those reflectively created objects stayed invisible. Walk the receiver chain through getDeclaredConstructor / getConstructor back to the forName string literal, so the newInstance result is an alloc of the named type regardless of how the constructor was obtained. Verified: new getDeclaredConstructor().newInstance() test resolves a dispatch through the cast of the result; javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests pass.
ServiceLoader.load(Foo.class) is iterated to obtain Foo instances, but the loader carried no points-to (its contents come from META-INF/services, which is not in the CPG), so a dispatch on an iterated element resolved to nothing. Model the load result as a synthetic collection whose element slot holds every impl of the service interface (from the CPG's inheritance edges, since META-INF is unavailable). Add iterator()/stream()/... as collection pass-throughs (the view carries the same elements) and next() as an element load, so `for (Foo f : load(Foo.class))` reads the impls. Verified: new ServiceLoader test resolves a dispatch on an iterated element to every impl of the service; javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests, CallTests, ConstructorInvocationTests and the x2cpg solver unit tests pass.
astForCatchClause lowered only the catch body, dropping the exception parameter entirely: `catch (T e)` left `e` as an unresolved identifier with no backing local, so nothing downstream could bind the caught exception to a variable. Introduce a local for the parameter in the catch scope (same shape as the for-each item local) and register it, so uses of the caught variable resolve to it via REF edges. Verified: new test asserts the catch parameter becomes a typed local referenced by its uses; javasrc2cpg ControlStructureTests, dataflow TryTests, CapturingTests, TypeInferenceTests pass.
collectFromMethod never visited throw operands or catch clauses, so a thrown object never reached the catch parameter and any dispatch on the caught variable resolved to nothing. For each `<operator>.throw`, take the thrown object's pointer variable and static type; for each catch clause, copy every thrown object whose type is the catch type or a subtype of it into the catch parameter local. Over-approximates the throw/catch pairing (any throw in the method reaches any catch) but stays type-sound via the transitive supertype check. Verified: new test flows a thrown SubException to catch (MyException e) and resolves e.handle() to SubException.handle only (not a sibling exception's); x2cpg solver unit tests, javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests, ControlStructureTests, TryTests pass.
exprVar and handleAssignment had no case for `<operator>.conditional`, so `x = cond ? a : b` dropped the ternary entirely — x carried no points-to and any dispatch on it went unresolved. Model the conditional's result as the union of its two branches (then/else arguments), and copy that union into the assignment target. A cast/allocation/nested branch flows through since each branch is resolved via exprVar. Verified: new test points the ternary result to both branch allocations and resolves a dispatch on it to each (not a third impl); x2cpg solver unit tests, javasrc2cpg PointerAnalysisFactoryTests, DiCollectorTests, CallGraphTests, CallTests pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #1 (solver performance). That branch is still open and this one is stacked on it, so the diff here currently also lists those 8 performance commits — they drop out automatically once #1 merges. The new work is the 20 precision/recall commits summarized below.
What this changes
The Andersen points-to analysis previously dropped or over-approximated many common Java constructs, so CPG call-graph and dataflow queries saw missing or imprecise targets. This branch closes those gaps, each as a self-contained, tested commit.
Allocation & expression modelling
foo(new A()), factory/builder returns) flow into points-to sets instead of being dropped.(T) xpreserve points-to (identity) instead of erasing it.cond ? a : bpoints to the union of both branches.Collections, arrays & functional interfaces
add/get/poll/..., index-insensitive) and arrays (a[i]).run/apply/get/accept/...) resolves to the synthetic method.Overload / generics resolution
Static fields & singletons
Type.field = ..., whether the base is aTypeRefor a type-named identifier) propagate, so singletons flow.Dependency injection
io.vertx.*calls, so a same-namedregister(...)in other frameworks no longer produces false bindings.@Inject List<Handler>) seeds every bean of the element type (from the member's generic signature).@Autowired).@Qualifier("name")narrows a multi-impl injection to the named bean, with a recall-preserving fallback to all impls when the name can't be resolved.FactoryBean<T>binds the bean type to the concrete typegetObject()returns.Reflection
Class.forName(name).newInstance()and...getDeclaredConstructor().newInstance()allocate the named type (from the string literal), resolved through the constructor chain.ServiceLoader.load(Foo.class)modelled as a collection whose elements are the service interface's implementations (CHA, since META-INF/services is not in the CPG), withiterator()/stream()pass-through andnext()as an element load.Exception flow
javasrc2cpg): the catch-clause parametercatch (T e)becomes a proper local in the catch scope — previously the parameter was dropped entirely, leavingean unresolved identifier. Useful to any analysis needing the caught-exception binding, not just PTA.Testing
19 dedicated recall/precision cases in
PointerAnalysisFactoryTests, plusDiCollectorTests, aControlStructureTestscatch-parameter test, and the x2cpg solver unit tests (AndersenSolverCycleTests,AndersenSolverFieldTests). Each commit was built and its tests run in a JDK21 container; the surrounding call-graph and dataflow suites stay green.Deliberately out of scope
Pruning over-approximate CHA edges — removing PTA-refuted targets from CALL edges — was considered and rejected. It is unsound whenever points-to is only partially modelled (framework-callback parameters invoked by Spring/Vert.x,
cond ? new A() : libraryReturn()branches): the points-to set is non-empty but incomplete, and removing a CHA edge then drops a real target fromreachableByFlows— a lost finding in a security CPG. The resolver-aware path (PointerAnalysisCallResolver.getResolvedCalledMethods) already delivers the precision; raw CALL edges stay CHA-sound on purpose.