Java: Remove self-ref tracking; improve AccessPath.toString on numbers.

Author: aschackmull

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll

@@ -69,18 +69,9 @@ class Content extends TContent {
     path = "" and sl = 0 and sc = 0 and el = 0 and ec = 0
   }
   /** Gets the type of the object containing this content. */
-  abstract RefType getDeclaringType();
+  abstract RefType getContainerType();
   /** Gets the type of this content. */
   abstract Type getType();
-  /**
-   * Holds if this content may contain an object of the same type as the one
-   * that contains this content, and if this fact should be used to compress
-   * access paths.
-   *
-   * Examples include the tail pointer in a linked list or the left and right
-   * pointers in a binary tree.
-   */
-  predicate isSelfRef() { none() }
 }
 private class FieldContent extends Content, TFieldContent {
   Field f;
@@ -90,17 +81,17 @@ private class FieldContent extends Content, TFieldContent {
   override predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
     f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
   }
-  override RefType getDeclaringType() { result = f.getDeclaringType() }
+  override RefType getContainerType() { result = f.getDeclaringType() }
   override Type getType() { result = f.getType() }
 }
 private class CollectionContent extends Content, TCollectionContent {
   override string toString() { result = "collection" }
-  override RefType getDeclaringType() { none() }
+  override RefType getContainerType() { none() }
   override Type getType() { none() }
 }
 private class ArrayContent extends Content, TArrayContent {
   override string toString() { result = "array" }
-  override RefType getDeclaringType() { none() }
+  override RefType getContainerType() { none() }
   override Type getType() { none() }
 }
 
@@ -132,6 +123,11 @@ RefType getErasedRepr(Type t) {
   result instanceof VoidType // stub implementation
 }
 
+/** Gets a string representation of a type returned by `getErasedRepr`. */
+string ppReprType(Type t) {
+  result = t.toString()
+}
+
 /**
  * Holds if `t1` and `t2` are compatible, that is, whether data can flow from
  * a node of type `t1` to a node of type `t2`.

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImpl.qll

@@ -724,76 +724,27 @@ private predicate localFlowBigStep(
   localFlowExit(node2, config)
 }
 
-/**
- * Holds if `f` may contain an object of the same type, `t`, as the one
- * that contains `f`, and if this fact should be used to compress
- * access paths.
- *
- * Examples include the tail pointer in a linked list or the left and right
- * pointers in a binary tree.
- */
-private predicate selfRef(Content f, RefType t) {
-  t = f.getDeclaringType() and
-  f.isSelfRef()
-}
-
-private newtype TFlowContainer =
-  TRegularContent(Content f) { not selfRef(f, _) } or
-  TSelfRefContent(RefType t) { selfRef(_, t) }
-
-/**
- * A `Content` or a `Content` abstracted as its declaring type.
- *
- * Sequences of one or more `Content`s in the same declaring type for which
- * `isSelfRef()` holds are represented as a single `FlowContainer` in an
- * `AccessPath`.
- */
-private class FlowContainer extends TFlowContainer {
-  string toString() {
-    exists(Content f | this = TRegularContent(f) and result = f.toString())
-    or
-    exists(RefType t | this = TSelfRefContent(t) and result = t.toString())
-  }
-
-  predicate usesContent(Content f) {
-    this = TRegularContent(f)
-    or
-    exists(RefType t | this = TSelfRefContent(t) and selfRef(f, t))
-  }
-
-  RefType getContainerType() {
-    exists(Content f | this = TRegularContent(f) and result = f.getDeclaringType())
-    or
-    this = TSelfRefContent(result)
-  }
-}
-
 private newtype TAccessPathFront =
   TFrontNil(Type t) or
-  TFrontHead(FlowContainer f)
+  TFrontHead(Content f)
 
 /**
  * The front of an `AccessPath`. This is either a head or a nil.
  */
 private class AccessPathFront extends TAccessPathFront {
   string toString() {
-    exists(Type t | this = TFrontNil(t) | result = t.toString())
+    exists(Type t | this = TFrontNil(t) | result = ppReprType(t))
     or
-    exists(FlowContainer f | this = TFrontHead(f) | result = f.toString())
+    exists(Content f | this = TFrontHead(f) | result = f.toString())
   }
 
   Type getType() {
     this = TFrontNil(result)
     or
-    exists(FlowContainer head | this = TFrontHead(head) | result = head.getContainerType())
+    exists(Content head | this = TFrontHead(head) | result = head.getContainerType())
   }
 
-  predicate headUsesContent(Content f) {
-    exists(FlowContainer fc |
-      fc.usesContent(f) and
-      this = TFrontHead(fc)
-    )
-  }
+  predicate headUsesContent(Content f) { this = TFrontHead(f) }
 }
 
 private class AccessPathFrontNil extends AccessPathFront, TFrontNil { }
@@ -1004,7 +955,7 @@ private predicate consCand(Content f, AccessPathFront apf, Configuration config)
 
 private newtype TAccessPath =
   TNil(Type t) or
-  TCons(FlowContainer f, int len) { len in [1 .. 5] }
+  TCons(Content f, int len) { len in [1 .. 5] }
 
 /**
  * Conceptually a list of `Content`s followed by a `Type`, but only the first
@@ -1016,7 +967,7 @@ private newtype TAccessPath =
 private class AccessPath extends TAccessPath {
   abstract string toString();
 
-  FlowContainer getHead() { this = TCons(result, _) }
+  Content getHead() { this = TCons(result, _) }
 
   int len() {
     this = TNil(_) and result = 0
@@ -1027,27 +978,27 @@ private class AccessPath extends TAccessPath {
   Type getType() {
     this = TNil(result)
     or
-    exists(FlowContainer head | this = TCons(head, _) | result = head.getContainerType())
+    exists(Content head | this = TCons(head, _) | result = head.getContainerType())
   }
 
   abstract AccessPathFront getFront();
 }
 
 private class AccessPathNil extends AccessPath, TNil {
-  override string toString() { exists(Type t | this = TNil(t) | result = t.toString()) }
+  override string toString() { exists(Type t | this = TNil(t) | result = ppReprType(t)) }
 
   override AccessPathFront getFront() { exists(Type t | this = TNil(t) | result = TFrontNil(t)) }
 }
 
 private class AccessPathCons extends AccessPath, TCons {
   override string toString() {
-    exists(FlowContainer f, int len | this = TCons(f, len) |
+    exists(Content f, int len | this = TCons(f, len) |
       result = f.toString() + ", ... (" + len.toString() + ")"
     )
   }
 
   override AccessPathFront getFront() {
-    exists(FlowContainer f | this = TCons(f, _) | result = TFrontHead(f))
+    exists(Content f | this = TCons(f, _) | result = TFrontHead(f))
   }
 }
 

java/ql/src/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll

@@ -118,7 +118,7 @@ private module ImplCommon {
       node1.(ArgumentNode).argumentOf(call, i1) and
       node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and
       compatibleTypes(node1.getTypeBound(), f.getType()) and
-      compatibleTypes(node2.getTypeBound(), f.getDeclaringType())
+      compatibleTypes(node2.getTypeBound(), f.getContainerType())
     )
   }
 
@@ -149,7 +149,7 @@ private module ImplCommon {
       setterReturn(p, f) and
       arg.argumentOf(node2.asExpr(), _) and
       compatibleTypes(node1.getTypeBound(), f.getType()) and
-      compatibleTypes(node2.getTypeBound(), f.getDeclaringType())
+      compatibleTypes(node2.getTypeBound(), f.getContainerType())
     )
   }
 
@@ -174,7 +174,7 @@ private module ImplCommon {
       viableParamArg(p, arg) and
       getter(p, f) and
       arg.argumentOf(node2.asExpr(), _) and
-      compatibleTypes(node1.getTypeBound(), f.getDeclaringType()) and
+      compatibleTypes(node1.getTypeBound(), f.getContainerType()) and
       compatibleTypes(node2.getTypeBound(), f.getType())
     )
   }

java/ql/src/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -118,20 +118,10 @@ class Content extends TContent {
   }
 
   /** Gets the type of the object containing this content. */
-  abstract RefType getDeclaringType();
+  abstract RefType getContainerType();
 
   /** Gets the type of this content. */
   abstract Type getType();
-
-  /**
-   * Holds if this content may contain an object of the same type as the one
-   * that contains this content, and if this fact should be used to compress
-   * access paths.
-   *
-   * Examples include the tail pointer in a linked list or the left and right
-   * pointers in a binary tree.
-   */
-  predicate isSelfRef() { none() }
 }
 
 private class FieldContent extends Content, TFieldContent {
@@ -147,25 +137,23 @@ private class FieldContent extends Content, TFieldContent {
     f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
   }
 
-  override RefType getDeclaringType() { result = f.getDeclaringType() }
+  override RefType getContainerType() { result = f.getDeclaringType() }
 
   override Type getType() { result = getFieldTypeBound(f) }
-
-  override predicate isSelfRef() { compatibleTypes(getDeclaringType(), getType()) }
 }
 
 private class CollectionContent extends Content, TCollectionContent {
   override string toString() { result = "collection" }
 
-  override RefType getDeclaringType() { none() }
+  override RefType getContainerType() { none() }
 
   override Type getType() { none() }
 }
 
 private class ArrayContent extends Content, TArrayContent {
   override string toString() { result = "array" }
 
-  override RefType getDeclaringType() { none() }
+  override RefType getContainerType() { none() }
 
   override Type getType() { none() }
 }
@@ -212,6 +200,13 @@ RefType getErasedRepr(Type t) {
   )
 }
 
+/** Gets a string representation of a type returned by `getErasedRepr`. */
+string ppReprType(Type t) {
+  if t.(BoxedType).getPrimitiveType().getName() = "double"
+  then result = "Number"
+  else result = t.toString()
+}
+
 private predicate canContainBool(Type t) {
   t instanceof BooleanType or
   any(BooleanType b).(RefType).getASourceSupertype+() = t
@@ -227,12 +222,9 @@ predicate compatibleTypes(Type t1, Type t2) {
     e1 = getErasedRepr(t1) and
     e2 = getErasedRepr(t2)
   |
-    /*
-     * Because of `getErasedRepr`, `erasedHaveIntersection` is a sufficient
-     * compatibility check, but `conContainBool` is kept as a dummy disjunct
-     * to get the proper join-order.
-     */
-
+    // Because of `getErasedRepr`, `erasedHaveIntersection` is a sufficient
+    // compatibility check, but `conContainBool` is kept as a dummy disjunct
+    // to get the proper join-order.
     erasedHaveIntersection(e1, e2)
     or
     canContainBool(e1) and canContainBool(e2)