Merge remote-tracking branch 'upstream/master' into mergeback-20181112

cpp/ql/src/Critical/InconsistentNullnessTesting.cpp

@@ -1,10 +1,10 @@
 void* f() {
-	block = malloc(BLOCK_SIZE);
+	block = (MyBlock *)malloc(sizeof(MyBlock));
 	if (block) { //correct: block is checked for nullness here
 		block->id = NORMAL_BLOCK_ID;
 	}
 	//...
 	/* make sure data-portion is null-terminated */
-	block[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here
+	block->data[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here
 	return block;
 }

cpp/ql/src/Critical/OverflowCalculated.cpp

@@ -1,5 +1,5 @@
 void f(char* string) {
-	// wrong: allocates space for characters, put not zero terminator
+	// wrong: allocates space for characters, but not zero terminator
 	char* buf = malloc(strlen(string));
 	strcpy(buf, string);
 

cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql

@@ -100,6 +100,7 @@ where // EQExprs are covered by CompareWhereAssignMeant.ql
       not accessInInitOfForStmt(peivc) and
       not peivc.isCompilerGenerated() and
       not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and
+      not peivc.isFromTemplateInstantiation(_) and
       parent = peivc.getParent() and
       not parent.isInMacroExpansion() and
       not parent instanceof PureExprInVoidContext and

cpp/ql/test/format.json

@@ -0,0 +1,11 @@
+[
+  {
+    "pattern": [
+      "**/*.c",
+      "**/*.cpp",
+      "**/*.h",
+      "**/*.hpp"
+    ],
+    "allowMixedTabsAndSpaces": true
+  }
+]

cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp

@@ -5,11 +5,11 @@ int external();
 class Base {
 public:
   virtual int thingy() {
-    1;
+    1; // BAD
   }
 
   int our_thingy() {
-    Base::thingy();
+    Base::thingy(); // BAD
     return 2;
   }
 };
@@ -17,14 +17,14 @@ public:
 class Derived : public Base {
 public:
   virtual int thingy() {
-    external();
+    external(); // GOOD
     return 3;
   }
 };
 
 void internal() {
   Base* ptr = new Derived();
-  ptr->thingy();
+  ptr->thingy(); // GOOD
 }
 
 }

cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected

@@ -1,6 +1,5 @@
 | calls.cpp:8:5:8:5 | 1 | This expression has no effect. | calls.cpp:8:5:8:5 | 1 |  |
 | calls.cpp:12:5:12:16 | call to thingy | This expression has no effect (because $@ has no external side effects). | calls.cpp:7:15:7:20 | thingy | thingy |
-| templatey.cpp:4:3:4:8 | ... << ... | This expression has no effect. | templatey.cpp:4:3:4:8 | ... << ... |  |
 | templatey.cpp:39:3:39:23 | call to pointless_add_numbers | This expression has no effect (because $@ has no external side effects). | templatey.cpp:29:5:29:25 | pointless_add_numbers | pointless_add_numbers |
 | volatile.c:9:5:9:5 | c | This expression has no effect. | volatile.c:9:5:9:5 | c |  |
 | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array |  |

cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp

@@ -1,7 +1,7 @@
 template <typename T>
 void foo(T x, T y)
 {
-  x << y;
+  x << y; // GOOD (effect depends on T)
 };
 
 struct streamable
@@ -15,9 +15,9 @@ void operator<<(streamable& lhs, streamable& rhs)
 int main()
 {
   int x = 3;
-  foo(x, x);
+  foo(x, x); // BAD [NOT DETECTED]
   streamable y;
-  foo(y, y);
+  foo(y, y); // BAD [NOT DETECTED]
   return 0;
 }
 
@@ -34,7 +34,7 @@ int pointless_add_numbers(int lhs, int rhs)
 void call_add_numbers()
 {
   int accum = 0;
-  add_numbers(accum, 4);
-  add_numbers(accum, 10);
-  pointless_add_numbers(accum, 20);
+  add_numbers(accum, 4); // GOOD
+  add_numbers(accum, 10); // GOOD
+  pointless_add_numbers(accum, 20); // BAD
 }

cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c

@@ -6,18 +6,18 @@ char *pc;
 volatile char *pv;
 
 void f(void) {
-    c;
-    v;
+    c; // BAD
+    v; // (accesses to volatile variables are considered impure)
 
-    pc[5];
+    pc[5]; // BAD
     pv[5];
     ((volatile char *)pc)[5];
 
-    *pc;
+    *pc; // BAD
     *pv;
     *((volatile char *)pc);
 
-    *(pc + 5);
+    *(pc + 5); // BAD
     *(pv + 5);
     *((volatile char *)(pc + 5));
     *(((volatile char *)pc) + 5);

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

@@ -1,5 +1,6 @@
 | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 |
 | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 |
+| template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ |
 | test.c:7:5:7:5 | 0 | This expression has no effect. | test.c:7:5:7:5 | 0 |  |
 | test.c:9:8:9:8 | 1 | This expression has no effect. | test.c:9:8:9:8 | 1 |  |
 | test.c:9:11:9:11 | 2 | This expression has no effect. | test.c:9:11:9:11 | 2 |  |
@@ -19,11 +20,6 @@
 | test.c:26:15:26:16 | 32 | This expression has no effect. | test.c:26:15:26:16 | 32 |  |
 | test.c:27:9:27:10 | 33 | This expression has no effect. | test.c:27:9:27:10 | 33 |  |
 | test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
 | test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
-| test.cpp:26:3:26:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ |
 | test.cpp:62:5:62:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:47:14:47:22 | operator= | operator= |
 | test.cpp:65:5:65:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:55:7:55:7 | operator= | operator= |

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

@@ -0,0 +1,7 @@
+
+#define UNUSED(x) (x)
+
+void test2(int param)
+{
+	UNUSED(param); // GOOD
+}

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

@@ -0,0 +1,22 @@
+
+template<class T>
+void Increment(T &t) {
+	t++; // GOOD (sometimes has an effect)
+}
+
+class Nothing {
+public:
+	Nothing operator++(int) {
+		return *this; // do nothing
+	}
+};
+
+void myTemplateTest() {
+	int i = 0;
+	Nothing n;
+
+	i++; // GOOD (always has an effect)
+	n++; // BAD (never has an effect)
+	Increment(i);
+	Increment(n);
+}

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

@@ -23,7 +23,7 @@ public:
 
 		++arg1; // pure, does nothing
 		++arg2; // pure, does nothing
-		++arg3; // not pure in all cases (when _It is int this has a side-effect) [FALSE POSITIVE]
+		++arg3; // not pure in all cases (when _It is int this has a side-effect)
 
 		return arg2;
 	}