C++: Include phi read nodes in SSA.

There's a small fix to the mapping from 'global def -> use'.

Finally, this commit also accepts a test failure related to new missing
types for phi nodes. The fix for that is in the next commit.

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll

@@ -530,15 +530,15 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
   exists(IRBlock bb1, int i1, SourceVariable v |
     defOrUse1.asDefOrUse().hasIndexInBlock(bb1, i1, v)
   |
-    exists(IRBlock bb2, int i2, Definition def |
-      adjacentDefRead(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
+    exists(IRBlock bb2, int i2, DefinitionExt def |
+      adjacentDefReadExt(pragma[only_bind_into](def), pragma[only_bind_into](bb1),
         pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
       def.getSourceVariable() = v and
       use.asDefOrUse().(UseImpl).hasIndexInBlock(bb2, i2, v)
     )
     or
     exists(PhiNode phi |
-      lastRefRedef(_, bb1, i1, phi) and
+      lastRefRedefExt(_, bb1, i1, phi) and
       use.asPhi() = phi and
       phi.getSourceVariable() = pragma[only_bind_into](v)
     )
@@ -550,11 +550,18 @@ predicate adjacentDefRead(DefOrUse defOrUse1, UseOrPhi use) {
  * flows to `useOrPhi`.
  */
 private predicate globalDefToUse(GlobalDef globalDef, UseOrPhi useOrPhi) {
-  exists(IRBlock bb1, int i1, IRBlock bb2, int i2, SourceVariable v |
-    globalDef.hasIndexInBlock(bb1, i1, v) and
-    adjacentDefRead(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
-      pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
-    useOrPhi.asDefOrUse().hasIndexInBlock(bb2, i2, v)
+  exists(IRBlock bb1, int i1, SourceVariable v | globalDef.hasIndexInBlock(bb1, i1, v) |
+    exists(IRBlock bb2, int i2 |
+      adjacentDefReadExt(_, pragma[only_bind_into](bb1), pragma[only_bind_into](i1),
+        pragma[only_bind_into](bb2), pragma[only_bind_into](i2)) and
+      useOrPhi.asDefOrUse().hasIndexInBlock(bb2, i2, v)
+    )
+    or
+    exists(PhiNode phi |
+      lastRefRedefExt(_, bb1, i1, phi) and
+      useOrPhi.asPhi() = phi and
+      phi.getSourceVariable() = pragma[only_bind_into](v)
+    )
   )
 }
 
@@ -666,17 +673,17 @@ private predicate ssaFlowImpl(SsaDefOrUse defOrUse, Node nodeFrom, Node nodeTo,
  * Holds if `def` is the corresponding definition of
  * the SSA library's `definition`.
  */
-private Definition ssaDefinition(Def def) {
+private DefinitionExt ssaDefinition(Def def) {
   exists(IRBlock block, int i, SourceVariable sv |
     def.hasIndexInBlock(block, i, sv) and
-    result.definesAt(sv, block, i)
+    result.definesAt(sv, block, i, _)
   )
 }
 
 /** Gets a node that represents the prior definition of `node`. */
 private Node getAPriorDefinition(SsaDefOrUse defOrUse) {
-  exists(IRBlock bb, int i, SourceVariable sv, Definition def, DefOrUse defOrUse0 |
-    SsaCached::lastRefRedef(pragma[only_bind_into](def), pragma[only_bind_into](bb),
+  exists(IRBlock bb, int i, SourceVariable sv, DefinitionExt def, DefOrUse defOrUse0 |
+    lastRefRedefExt(pragma[only_bind_into](def), pragma[only_bind_into](bb),
       pragma[only_bind_into](i), ssaDefinition(defOrUse)) and
     def.getSourceVariable() = sv and
     defOrUse0.hasIndexInBlock(bb, i, sv) and
@@ -702,7 +709,7 @@ pragma[nomagic]
 private predicate fromPhiNodeToUse(PhiNode phi, SourceVariable sv, IRBlock bb1, int i1, UseOrPhi use) {
   exists(IRBlock bb2, int i2 |
     use.asDefOrUse().hasIndexInBlock(bb2, i2, sv) and
-    adjacentDefRead(pragma[only_bind_into](phi), pragma[only_bind_into](bb1),
+    adjacentDefReadExt(pragma[only_bind_into](phi), pragma[only_bind_into](bb1),
       pragma[only_bind_into](i1), pragma[only_bind_into](bb2), pragma[only_bind_into](i2))
   )
 }
@@ -711,13 +718,13 @@ private predicate fromPhiNodeToUse(PhiNode phi, SourceVariable sv, IRBlock bb1,
 predicate fromPhiNode(SsaPhiNode nodeFrom, Node nodeTo) {
   exists(PhiNode phi, SourceVariable sv, IRBlock bb1, int i1, UseOrPhi use |
     phi = nodeFrom.getPhiNode() and
-    phi.definesAt(sv, bb1, i1) and
+    phi.definesAt(sv, bb1, i1, _) and
     useToNode(use, nodeTo)
   |
     fromPhiNodeToUse(phi, sv, bb1, i1, use)
     or
     exists(PhiNode phiTo |
-      lastRefRedef(phi, _, _, phiTo) and
+      lastRefRedefExt(phi, _, _, phiTo) and
       nodeTo.(SsaPhiNode).getPhiNode() = phiTo
     )
   )
@@ -796,8 +803,8 @@ module SsaCached {
    * path between them without any read of `def`.
    */
   cached
-  predicate adjacentDefRead(Definition def, IRBlock bb1, int i1, IRBlock bb2, int i2) {
-    SsaImpl::adjacentDefRead(def, bb1, i1, bb2, i2)
+  predicate adjacentDefReadExt(DefinitionExt def, IRBlock bb1, int i1, IRBlock bb2, int i2) {
+    SsaImpl::adjacentDefReadExt(def, _, bb1, i1, bb2, i2)
   }
 
   /**
@@ -806,8 +813,8 @@ module SsaCached {
    * without passing through another read or write.
    */
   cached
-  predicate lastRefRedef(Definition def, IRBlock bb, int i, Definition next) {
-    SsaImpl::lastRefRedef(def, bb, i, next)
+  predicate lastRefRedefExt(DefinitionExt def, IRBlock bb, int i, DefinitionExt next) {
+    SsaImpl::lastRefRedefExt(def, _, bb, i, next)
   }
 }
 
@@ -818,8 +825,8 @@ private newtype TSsaDefOrUse =
     or
     // Like in the pruning stage, we only include definition that's live after the
     // write as the final definitions computed by SSA.
-    exists(Definition def, SourceVariable sv, IRBlock bb, int i |
-      def.definesAt(sv, bb, i) and
+    exists(DefinitionExt def, SourceVariable sv, IRBlock bb, int i |
+      def.definesAt(sv, bb, i, _) and
       defOrUse.(DefImpl).hasIndexInBlock(bb, i, sv)
     )
   } or
@@ -967,9 +974,14 @@ class Def extends DefOrUse {
 
 private module SsaImpl = SsaImplCommon::Make<SsaInput>;
 
-class PhiNode = SsaImpl::PhiNode;
+class PhiNode extends SsaImpl::DefinitionExt {
+  PhiNode() {
+    this instanceof SsaImpl::PhiNode or
+    this instanceof SsaImpl::PhiReadNode
+  }
+}
 
-class Definition = SsaImpl::Definition;
+class DefinitionExt = SsaImpl::DefinitionExt;
 
 class UncertainWriteDefinition = SsaImpl::UncertainWriteDefinition;
 

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/ssa0/SsaInternals.qll

@@ -225,8 +225,8 @@ private newtype TSsaDefOrUse =
     or
     // If `defOrUse` is a definition we only include it if the
     // SSA library concludes that it's live after the write.
-    exists(Definition def, SourceVariable sv, IRBlock bb, int i |
-      def.definesAt(sv, bb, i) and
+    exists(DefinitionExt def, SourceVariable sv, IRBlock bb, int i |
+      def.definesAt(sv, bb, i, _) and
       defOrUse.(DefImpl).hasIndexInBlock(bb, i, sv)
     )
   } or
@@ -301,6 +301,11 @@ class Def extends DefOrUse {
 
 private module SsaImpl = SsaImplCommon::Make<SsaInput>;
 
-class PhiNode = SsaImpl::PhiNode;
+class PhiNode extends SsaImpl::DefinitionExt {
+  PhiNode() {
+    this instanceof SsaImpl::PhiNode or
+    this instanceof SsaImpl::PhiReadNode
+  }
+}
 
-class Definition = SsaImpl::Definition;
+class DefinitionExt = SsaImpl::DefinitionExt;

cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp

@@ -626,5 +626,5 @@ void test_def_via_phi_read(bool b)
     use(buffer);
   }
   intPointerSource(buffer);
-  sink(buffer); // $ ast MISSING: ir
+  sink(buffer); // $ ast,ir
 }

cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected

@@ -1,3 +1,8 @@
 failures
 astTypeBugs
 irTypeBugs
+| test.cpp:15:3:15:6 | test.cpp:15:3:15:6 | test.cpp:15:3:15:6 | Phi |
+| test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | Phi |
+| test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | test.cpp:43:10:43:20 | Phi |
+| test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | Phi |
+| test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | test.cpp:628:3:628:18 | Phi |