Python: fix augstore for the new CFG and add store/load test

In the legacy CFG the same Python 'Name' that is the target of an
augmented assignment has two distinct CFG nodes — a load node (context
3) earlier in the basic block and a store node (context 5) later.
'augstore(load, store)' relates the pair via dominance.

The new (shared) CFG canonicalises each AST expression to a single
CFG node, so 'load' and 'store' collapse to one. The dominance-based
'augstore' from the legacy implementation no longer holds (it would
require 'load.strictlyDominates(load)'), so 'isAugLoad' / 'isAugStore'
never fired and 'isStore' missed the AugAssign target entirely.

Redefines 'augstore' as reflexive on the AugAssign target's canonical
CFG node. With this change:

  * isAugLoad / isAugStore both fire on the single canonical node.
  * isStore fires (via 'or augstore(_, this)') — matching the legacy
    classification that an augmented-assignment target is a store.
  * isLoad does not fire (excluded by 'not augstore(_, this)').

Adds 'python/ql/test/library-tests/ControlFlow/store-load/' covering
plain load/store/delete, parameters, augmented assignment, tuple
unpacking, attribute and subscript stores. The test asserts the
classification directly on the new-CFG facade.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
yoff
2026-05-18 12:29:37 +00:00
parent 79db96c717
commit f5bf8ae8dd
4 changed files with 115 additions and 7 deletions

View File

@@ -218,15 +218,22 @@ class ControlFlowNode extends CfgImpl::ControlFlowNode {
}
/**
* Holds if `n` is an augmented assignment load and `store` is the
* corresponding store node.
* Holds if `load` is the load half of an augmented-assignment target,
* and `store` is the corresponding store half.
*
* In the legacy CFG (`Flow.qll`) the same Python `Name` had two
* distinct CFG nodes — a load node (context 3) earlier in the BB, and
* a store node (context 5) later. The legacy `augstore` related the
* pair via dominance.
*
* In the new (shared) CFG, the canonical node for an AST expression is
* unique, so `load` and `store` collapse onto the same CFG node. The
* predicate is therefore reflexive on the augmented-assignment
* target's canonical node.
*/
private predicate augstore(ControlFlowNode load, ControlFlowNode store) {
exists(Py::Expr load_store | exists(Py::AugAssign aa | aa.getTarget() = load_store) |
toAst(load) = load_store and
toAst(store) = load_store and
load.strictlyDominates(store)
)
exists(Py::AugAssign aa | aa.getTarget() = toAst(load)) and
load = store
}
/**

View File

@@ -0,0 +1,45 @@
/**
* Inline-expectations test for the store/load/delete/parameter
* classification predicates on the new-CFG facade.
*
* Each tag fires when the corresponding predicate (`isLoad`,
* `isStore`, `isDelete`, `isParameter`, `isAugLoad`, `isAugStore`)
* holds on the canonical CFG node wrapping a `Py::Name` with the
* given identifier.
*
* For subscript / attribute stores the tag fires on the Subscript /
* Attribute node itself, with `value` set to the rightmost identifier
* (the attribute name for `Attribute`, the index expression's textual
* form for `Subscript`).
*/
import python
import semmle.python.controlflow.internal.Cfg as Cfg
import utils.test.InlineExpectationsTest
module StoreLoadTest implements TestSig {
string getARelevantTag() { result = ["load", "store", "delete", "param", "augload", "augstore"] }
predicate hasActualResult(Location location, string element, string tag, string value) {
exists(Cfg::NameNode n |
location = n.getLocation() and
element = n.toString() and
value = n.getId() and
(
n.isLoad() and tag = "load"
or
n.isStore() and not n.isAugStore() and tag = "store"
or
n.isDelete() and tag = "delete"
or
n.isParameter() and tag = "param"
or
n.isAugLoad() and tag = "augload"
or
n.isAugStore() and tag = "augstore"
)
)
}
}
import MakeTest<StoreLoadTest>

View File

@@ -0,0 +1,56 @@
# Store/load/delete/parameter classification on the new-CFG facade.
#
# Each annotated location carries the (sorted, deduplicated) set of
# kinds the CFG facade reports there. Comparing against the legacy
# 'semmle.python.Flow' classification is done by the comparison query
# 'StoreLoadParity.ql' — annotations here are only the positive
# assertions for the new facade.
#
# Tags:
# load=<id> -- isLoad() fires on the Name
# store=<id> -- isStore() fires
# delete=<id> -- isDelete() fires
# param=<id> -- isParameter() fires
# augload=<id> -- isAugLoad() fires (the LHS of x += ... when read)
# augstore=<id> -- isAugStore() fires (the LHS of x += ... when written)
# --- plain load / store / delete ---
x = 1 # $ store=x
y = x + 1 # $ store=y load=x
print(y) # $ load=print load=y
del x # $ delete=x
# --- function definitions (parameters) ---
def f(a, b=2, *args, c, **kwargs): # $ store=f param=a param=b param=args param=c param=kwargs
return a + b + c # $ load=a load=b load=c
# --- augmented assignment splits one Name into load + store halves ---
def aug(): # $ store=aug
n = 0 # $ store=n
n += 1 # $ augload=n augstore=n
return n # $ load=n
# --- subscript / attribute stores ---
class C: # $ store=C
pass
def stores(obj, container, idx): # $ store=stores param=obj param=container param=idx
obj.attr = 1 # $ load=obj
container[idx] = 2 # $ load=container load=idx
return obj # $ load=obj
# --- tuple unpacking ---
def unpack(pair): # $ store=unpack param=pair
a, b = pair # $ store=a store=b load=pair
return a + b # $ load=a load=b