Merge pull request #3061 from MathiasVP/fix-constant-comparison

C++: Fix getValue in SimpleRangeAnalysis

Author: Robert Marsh

cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll

@@ -93,17 +93,27 @@ private float wideningUpperBounds(ArithmeticType t) {
 
 /**
  * Gets the value of the expression `e`, if it is a constant.
- * This predicate also handles the case of constant variables initialized in compilation units,
- * which doesn't necessarily have a getValue() result from the extractor.
+ * This predicate also handles the case of constant variables initialized in different
+ * compilation units, which doesn't necessarily have a getValue() result from the extractor.
  */
 private string getValue(Expr e) {
   if exists(e.getValue())
   then result = e.getValue()
   else
-    exists(VariableAccess access, Variable v |
+    /*
+     * It should be safe to propagate the initialization value to a variable if:
+     * The type of v is const, and
+     * The type of v is not volatile, and
+     * Either:
+     *   v is a local/global variable, or
+     *   v is a static member variable
+     */
+
+    exists(VariableAccess access, StaticStorageDurationVariable v |
+      not v.getUnderlyingType().isVolatile() and
+      v.getUnderlyingType().isConst() and
       e = access and
       v = access.getTarget() and
-      v.getUnderlyingType().isConst() and
       result = getValue(v.getAnAssignedValue())
     )
 }

cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.cpp

@@ -0,0 +1,29 @@
+void func_with_default_arg(const int n = 0) {
+    if(n <= 10) {}
+}
+
+struct A {
+    const int int_member = 0;
+    A(int n) : int_member(n) {
+        if(int_member <= 10) {
+                    
+        }
+    }
+};
+
+struct B {
+    B(const int n = 0) {
+        if(n <= 10) {}
+    }
+};
+
+const volatile int volatile_const_global = 0;
+
+void test1() {
+    func_with_default_arg(100);
+
+    A a(100);
+    if(a.int_member <= 10) {}
+
+    if(volatile_const_global <= 10) {}
+}