Dataflow: Address review comments.

github · Dec 4, 2024 · 29d3c51 · 29d3c51
1 parent 5042753
commit 29d3c51
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 15 deletions.
diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll
@@ -1423,6 +1423,12 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
         private class TypOption = TypOption::Option;
 
+        private string ppStored(TypOption stored) {
+          exists(string ppt | ppt = stored.toString() |
+            if stored.isNone() or ppt = "" then result = "" else result = " : " + ppt
+          )
+        }
+
         /* Begin: Stage logic. */
         pragma[nomagic]
         private Typ getNodeTyp(NodeEx node) {
@@ -1604,14 +1610,8 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
 
           private string ppTyp() { result = t.toString() and result != "" }
 
-          private string ppStored() {
-            exists(string ppt | ppt = stored.toString() |
-              if stored.isNone() or ppt = "" then result = "" else result = " : " + ppt
-            )
-          }
-
           override string toString() {
-            result = p + concat(" : " + this.ppTyp()) + " " + ap + this.ppStored()
+            result = p + concat(" : " + this.ppTyp()) + " " + ap + ppStored(stored)
           }
 
           override Location getLocation() { result = p.getLocation() }
@@ -3050,12 +3050,6 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
               )
             }
 
-            private string ppStored() {
-              exists(string ppt | ppt = stored.toString() |
-                if stored.isNone() or ppt = "" then result = "" else result = " : " + ppt
-              )
-            }
-
             private string ppCtx() { result = " <" + cc + ">" }
 
             private string ppSummaryCtx() {
@@ -3066,7 +3060,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
             }
 
             override string toString() {
-              result = node.toString() + this.ppType() + this.ppAp() + this.ppStored()
+              result = node.toString() + this.ppType() + this.ppAp() + ppStored(stored)
             }
 
             /**
@@ -3075,7 +3069,7 @@ module MakeImpl<LocationSig Location, InputSig<Location> Lang> {
              */
             string toStringWithContext() {
               result =
-                node.toString() + this.ppType() + this.ppAp() + this.ppStored() + this.ppCtx() +
+                node.toString() + this.ppType() + this.ppAp() + ppStored(stored) + this.ppCtx() +
                   this.ppSummaryCtx()
             }
 

diff --git a/shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll b/shared/dataflow/codeql/dataflow/internal/DataFlowImplConsistency.qll
@@ -77,6 +77,7 @@ module MakeConsistency<
 {
   private import DataFlowLang
   private import TaintTrackingLang
+  private import DataFlowImplCommon::MakeImplCommon<Location, DataFlowLang> as ImplCommon
 
   final private class NodeFinal = Node;
 
@@ -127,6 +128,16 @@ module MakeConsistency<
     )
   }
 
+  query predicate uniqueTopType(string msg) {
+    exists(int c |
+      c = count(DataFlowType t | ImplCommon::isTopType(t)) and
+      c != 1 and
+      msg =
+        "DataFlowType should have one top type that's compatiable with all other types, but has " +
+          c + "."
+    )
+  }
+
   query predicate uniqueNodeLocation(Node n, string msg) {
     exists(int c |
       c = count(n.getLocation()) and
@@ -352,6 +363,9 @@ module MakeConsistency<
     type = "Node should have one type" and
     result = count(Node n | uniqueType(n, _))
     or
+    type = "DataFlowType should have one top type" and
+    result = count( | uniqueTopType(_) | 1)
+    or
     type = "Node should have one location" and
     result = count(Node n | uniqueNodeLocation(n, _))
     or