Merge pull request #1343 from rdmarsh2/rdmarsh/cpp/getUnspecifiedType

C++: add getUnspecifiedType() for exprs and decls

Author: jbj

change-notes/1.21/analysis-cpp.md

@@ -36,3 +36,4 @@
     - The taint tracking library now includes taint-specific edges for functions modeled in `semmle.code.cpp.models.interfaces.DataFlow`.
     - The taint tracking library adds flow through library functions that are modeled in `semmle.code.cpp.models.interfaces.Taint`. Queries can add subclasses of `TaintFunction` to specify additional flow.
 - There is a new `FoldExpr` class, representing C++17 fold expressions.
+- The member predicates `DeclarationEntry.getUnspecifiedType`, `Expr.getUnspecifiedType`, and `Variable.getUnspecifiedType` have been added. These should be preferred over the existing `getUnderlyingType` predicates.

cpp/ql/src/Best Practices/Exceptions/CatchingByValue.ql

@@ -13,6 +13,6 @@
 import cpp
 
 from CatchBlock cb, Class caughtType
-where caughtType = cb.getParameter().getType().getUnderlyingType().getUnspecifiedType()
+where caughtType = cb.getParameter().getUnspecifiedType()
 select cb,
   "This should catch a " + caughtType.getName() + " by (const) reference rather than by value."

cpp/ql/src/Best Practices/Unused Entities/UnusedLocals.ql

@@ -40,7 +40,7 @@ class TemplateDependentType extends Type {
  * A variable whose declaration has, or may have, side effects.
  */
 predicate declarationHasSideEffects(Variable v) {
-  exists(Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
+  exists(Class c | c = v.getUnspecifiedType() |
     c.hasConstructor() or
     c.hasDestructor()
   )

cpp/ql/src/Best Practices/Unused Entities/UnusedStaticVariables.ql

@@ -14,7 +14,7 @@
 import cpp
 
 predicate declarationHasSideEffects(Variable v) {
-  exists(Class c | c = v.getType().getUnderlyingType().getUnspecifiedType() |
+  exists(Class c | c = v.getUnspecifiedType() |
     c.hasConstructor() or c.hasDestructor()
   )
 }

cpp/ql/src/Likely Bugs/AmbiguouslySignedBitField.ql

@@ -17,10 +17,10 @@
 import cpp
 
 from BitField bf
-where not bf.getType().getUnspecifiedType().(IntegralType).isExplicitlySigned()
-  and not bf.getType().getUnspecifiedType().(IntegralType).isExplicitlyUnsigned()
-  and not bf.getType().getUnspecifiedType() instanceof Enum
-  and not bf.getType().getUnspecifiedType() instanceof BoolType
+where not bf.getUnspecifiedType().(IntegralType).isExplicitlySigned()
+  and not bf.getUnspecifiedType().(IntegralType).isExplicitlyUnsigned()
+  and not bf.getUnspecifiedType() instanceof Enum
+  and not bf.getUnspecifiedType() instanceof BoolType
   // At least for C programs on Windows, BOOL is a common typedef for a type
   // representing BoolType.
   and not bf.getType().hasName("BOOL")

cpp/ql/src/Likely Bugs/Arithmetic/BadCheckOdd.ql

@@ -15,7 +15,7 @@ import cpp
 from EqualityOperation t, RemExpr lhs, Literal rhs
 where t.getLeftOperand() = lhs and
       t.getRightOperand() = rhs and
-      lhs.getLeftOperand().getType().getUnspecifiedType().(IntegralType).isSigned() and
+      lhs.getLeftOperand().getUnspecifiedType().(IntegralType).isSigned() and
       lhs.getRightOperand().getValue() = "2" and
       rhs.getValue() = "1"
 select t, "Possibly invalid test for oddness. This will fail for negative numbers."

cpp/ql/src/Likely Bugs/Arithmetic/PointlessSelfComparison.qll

@@ -45,7 +45,7 @@ predicate pointlessSelfComparison(ComparisonOperation cmp) {
 predicate nanTest(EqualityOperation cmp) {
   pointlessSelfComparison(cmp) and
   exists (Type t
-  | t = cmp.getLeftOperand().getType().getUnspecifiedType()
+  | t = cmp.getLeftOperand().getUnspecifiedType()
   | t instanceof FloatingPointType or
     t instanceof TemplateParameter)
 }

cpp/ql/src/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql

@@ -55,5 +55,5 @@ predicate introducesNewField(Class derived, Class base) {
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, CastToPointerArithFlow cfg
 where cfg.hasFlowPath(source, sink)
-  and source.getNode().asExpr().getFullyConverted().getType().getUnspecifiedType() = sink.getNode().asExpr().getFullyConverted().getType().getUnspecifiedType()
+  and source.getNode().asExpr().getFullyConverted().getUnspecifiedType() = sink.getNode().asExpr().getFullyConverted().getUnspecifiedType()
 select sink, source, sink, "Pointer arithmetic here may be done with the wrong type because of the cast $@.", source, "here"

cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql

@@ -48,7 +48,7 @@ predicate gettextFunction(Function f, int arg) {
 
 predicate stringArray(Variable arr, AggregateLiteral init) {
   arr.getInitializer().getExpr() = init and
-  stringType(arr.getType().getUnspecifiedType().(ArrayType).getBaseType(), _)
+  stringType(arr.getUnspecifiedType().(ArrayType).getBaseType(), _)
   // Ideally, this predicate should also check that no item of `arr` is ever
   // reassigned, but such an analysis could get fairly complicated. Instead, we
   // just hope that nobody would initialize an array of constants and then

cpp/ql/src/Likely Bugs/Memory Management/AllocaInLoop.ql

@@ -75,7 +75,7 @@ class LoopWithAlloca extends Stmt {
       conditionRequires(eq, truth) and
       eq.getAnOperand().getValue().toInt() = 1 and
       e = eq.getAnOperand() and
-      e.getType().getUnspecifiedType() instanceof BoolType and
+      e.getUnspecifiedType() instanceof BoolType and
       not exists(e.getValue())
     )
     or
@@ -84,7 +84,7 @@ class LoopWithAlloca extends Stmt {
       conditionRequires(eq, truth.booleanNot()) and
       eq.getAnOperand().getValue().toInt() = 1 and
       e = eq.getAnOperand() and
-      e.getType().getUnspecifiedType() instanceof BoolType and
+      e.getUnspecifiedType() instanceof BoolType and
       not exists(e.getValue())
     )
     or