Java: Deprecate PrimitiveType.getADefaultValue()

java/change-notes/2021-10-02-getADefaultValue-deprecation.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The predicate `PrimitiveType.getADefaultValue()` has been deprecated for removal in a future version because its behavior is misleading.

java/ql/lib/semmle/code/java/Type.qll

@@ -947,6 +947,10 @@ class PrimitiveType extends Type, @primitive {
   }
 
   /**
+   * DEPRECATED: This predicate will be removed in a future version because
+   * its behavior is misleading and it does not find all literals with default
+   * value, see [GitHub issue #6615](https://github.com/github/codeql/issues/6615).
+   *
    * Gets a default value for this primitive type, as assigned by the compiler
    * for variables that are declared but not initialized explicitly.
    * Typically zero for numeric and character types and `false` for `boolean`.
@@ -955,7 +959,7 @@ class PrimitiveType extends Type, @primitive {
    * considered to be default values of all other numeric types, even if they
    * require an explicit cast.
    */
-  Literal getADefaultValue() {
+  deprecated Literal getADefaultValue() {
     getName() = "boolean" and result.getLiteral() = "false"
     or
     getName() = "char" and

java/ql/src/Violations of Best Practice/Dead Code/DeadStoreOfLocal.ql

@@ -27,13 +27,24 @@ predicate flowStep(Expr decl, Expr init) {
   decl.(CastExpr).getExpr() = init
 }
 
+predicate isDefaultValueLiteral(Literal l, Type t) {
+  t instanceof RefType and l instanceof NullLiteral
+  or
+  // Checking here for primitive suffices to make sure that literals below are valid default
+  t instanceof PrimitiveType and
+  (
+    l.(BooleanLiteral).getBooleanValue() = false or
+    l.(CharacterLiteral).getValue() = 0.toUnicode() or
+    l.(DoubleLiteral).getDoubleValue() = 0 or
+    l.(FloatingPointLiteral).getFloatValue() = 0 or
+    l.(IntegerLiteral).getIntValue() = 0 or
+    l.(LongLiteral).getValue() = "0"
+  )
+}
+
 predicate excludedInit(Type t, Expr decl) {
   exists(Expr init | flowStep(decl, init) |
-    // The `null` literal for reference types.
-    t instanceof RefType and init instanceof NullLiteral
-    or
-    // The default value for primitive types.
-    init = t.(PrimitiveType).getADefaultValue()
+    isDefaultValueLiteral(init, t)
     or
     // The expression `-1` for integral types.
     t instanceof IntegralType and minusOne(init)

java/ql/test/query-tests/UnreadLocal/A.java

@@ -71,6 +71,101 @@ public void ex4() {
         if (valid) return;
     }
 
+    public void use(Object o) { }
+
+    public void badNonDefault() {
+        String s1 = "";
+        s1 = "a";
+        use(s1);
+
+        String s2 = "null";
+        s2 = "a";
+        use(s2);
+
+        Object o = false; // `false` is not the default for Object
+        o = true;
+        use(o);
+
+        boolean b = true;
+        b = false;
+        use(b);
+
+        char c1 = '\1';
+        c1 = '1';
+        use(c1);
+
+        char c2 = '0'; // is not \0
+        c2 = '1';
+        use(c2);
+
+        double d = 1d;
+        d = 0d;
+        use(d);
+
+        float f = 1f;
+        f = 0f;
+        use(f);
+
+        int i = 1;
+        i = 0;
+        use(i);
+
+        long l = 1L;
+        l = 0L;
+        use(l);
+    }
+
+    // Ignore if stored value is default value
+    public void goodDefault() {
+        String s = null;
+        s = "a";
+        use(s);
+
+        boolean b = false;
+        b = true;
+        use(b);
+
+        char c1 = '\0';
+        c1 = '1';
+        use(c1);
+
+        char c2 = 0;
+        c2 = 1;
+        use(c2);
+
+        double d1 = 0d;
+        d1 = 1d;
+        use(d1);
+
+        double d2 = '\0';
+        d2 = 1;
+        use(d2);
+
+        float f1 = 0f;
+        f1 = 1f;
+        use(f1);
+
+        float f2 = 0;
+        f2 = 1;
+        use(f2);
+
+        int i1 = 0;
+        i1 = 1;
+        use(i1);
+
+        int i2 = '\0';
+        i2 = '1';
+        use(i2);
+
+        long l1 = 0L;
+        l1 = 1L;
+        use(l1);
+
+        long l2 = 0;
+        l2 = 1;
+        use(l2);
+    }
+
     // ensure extraction of java.lang.RuntimeException
     public void noop() throws RuntimeException { }
 }

java/ql/test/query-tests/UnreadLocal/DeadStoreOfLocal.expected

@@ -3,3 +3,13 @@
 | A.java:40:14:40:16 | ++... | This assignment to y is useless: the value is always overwritten before it is read. |
 | A.java:55:13:55:17 | ...=... | This assignment to x is useless: the value is always overwritten before it is read. |
 | A.java:64:17:64:29 | ...=... | This assignment to valid is useless: the value is always overwritten before it is read. |
+| A.java:77:16:77:22 | s1 | This assignment to s1 is useless: the value is always overwritten before it is read. |
+| A.java:81:16:81:26 | s2 | This assignment to s2 is useless: the value is always overwritten before it is read. |
+| A.java:85:16:85:24 | o | This assignment to o is useless: the value is always overwritten before it is read. |
+| A.java:89:17:89:24 | b | This assignment to b is useless: the value is always overwritten before it is read. |
+| A.java:93:14:93:22 | c1 | This assignment to c1 is useless: the value is always overwritten before it is read. |
+| A.java:97:14:97:21 | c2 | This assignment to c2 is useless: the value is always overwritten before it is read. |
+| A.java:101:16:101:21 | d | This assignment to d is useless: the value is always overwritten before it is read. |
+| A.java:105:15:105:20 | f | This assignment to f is useless: the value is always overwritten before it is read. |
+| A.java:109:13:109:17 | i | This assignment to i is useless: the value is always overwritten before it is read. |
+| A.java:113:14:113:19 | l | This assignment to l is useless: the value is always overwritten before it is read. |