diff --git a/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll b/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll index 3a19049c520..fab4db883b8 100644 --- a/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll +++ b/python/ql/lib/semmle/python/controlflow/internal/Cfg.qll @@ -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 } /** diff --git a/python/ql/test/library-tests/ControlFlow/store-load/StoreLoadTest.expected b/python/ql/test/library-tests/ControlFlow/store-load/StoreLoadTest.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/python/ql/test/library-tests/ControlFlow/store-load/StoreLoadTest.ql b/python/ql/test/library-tests/ControlFlow/store-load/StoreLoadTest.ql new file mode 100644 index 00000000000..8d3a3462920 --- /dev/null +++ b/python/ql/test/library-tests/ControlFlow/store-load/StoreLoadTest.ql @@ -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 diff --git a/python/ql/test/library-tests/ControlFlow/store-load/test.py b/python/ql/test/library-tests/ControlFlow/store-load/test.py new file mode 100644 index 00000000000..dfca45a0b47 --- /dev/null +++ b/python/ql/test/library-tests/ControlFlow/store-load/test.py @@ -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= -- isLoad() fires on the Name +# store= -- isStore() fires +# delete= -- isDelete() fires +# param= -- isParameter() fires +# augload= -- isAugLoad() fires (the LHS of x += ... when read) +# augstore= -- 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