Merge pull request #1492 from geoffw0/exprnoeffectweak

CPP: Fix for 'Expression has no effect' on calls to weak functions

Author: jbj

change-notes/1.22/analysis-cpp.md

@@ -11,6 +11,7 @@
 
 | **Query**                  | **Expected impact**    | **Change**                                                       |
 |----------------------------|------------------------|------------------------------------------------------------------|
+| Expression has no effect (`cpp/useless-expression`) | Fewer false positive results | Calls to functions with the `weak` attribute are no longer considered to be side effect free, because they could be overridden with a different implementation at link time. |
 | Suspicious pointer scaling (`cpp/suspicious-pointer-scaling`) | Lower precision | The precision of this query has been reduced to "medium". This coding pattern is used intentionally and safely in a number of real-world projects. Results are no longer displayed on LGTM unless you choose to display them. |
 
 ## Changes to QL libraries

cpp/ql/src/semmle/code/cpp/exprs/Call.qll

@@ -275,12 +275,14 @@ class FunctionCall extends Call, @funbindexpr {
   override predicate mayBeImpure() {
     this.getChild(_).mayBeImpure() or
     this.getTarget().mayHaveSideEffects() or
-    isVirtual()
+    isVirtual() or
+    getTarget().getAnAttribute().getName() = "weak"
   }
   override predicate mayBeGloballyImpure() {
     this.getChild(_).mayBeGloballyImpure() or
     this.getTarget().mayHaveSideEffects() or
-    isVirtual()
+    isVirtual() or
+    getTarget().getAnAttribute().getName() = "weak"
   }
 }
 

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected

@@ -33,3 +33,4 @@
 | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array |  |
 | volatile.c:16:5:16:7 | * ... | This expression has no effect. | volatile.c:16:5:16:7 | * ... |  |
 | volatile.c:20:5:20:13 | * ... | This expression has no effect. | volatile.c:20:5:20:13 | * ... |  |
+| weak.c:18:2:18:18 | call to myNothingFunction | This expression has no effect (because $@ has no external side effects). | weak.c:2:5:2:21 | myNothingFunction | myNothingFunction |

cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/weak.c

@@ -0,0 +1,20 @@
+
+int myNothingFunction()
+{
+	// does nothing
+
+	return 0;
+}
+
+int __attribute__((__weak__)) myWeakNothingFunction()
+{
+	// does nothing, but we could be overridden at the linker stage with a non-weak definition
+	// from elsewhere in the program.
+
+	return 0;
+}
+
+void testWeak() {
+	myNothingFunction(); // BAD
+	myWeakNothingFunction(); // GOOD
+}