Merge pull request #562 from geoffw0/cpp-308

CPP: Fix FPs for 'Resource not released in destructor' involving virtual method calls

cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql

@@ -96,13 +96,19 @@ private predicate exprReleases(Expr e, Expr released, string kind) {
   ) or exists(Function f, int arg |
     // `e` is a call to a function that releases one of it's parameters,
     // and `released` is the corresponding argument
-    e.(FunctionCall).getTarget() = f and
+    (
+      e.(FunctionCall).getTarget() = f or
+      e.(FunctionCall).getTarget().(MemberFunction).getAnOverridingFunction+() = f
+    ) and
     e.(FunctionCall).getArgument(arg) = released and
     exprReleases(_, exprOrDereference(f.getParameter(arg).getAnAccess()), kind)
   ) or exists(Function f, ThisExpr innerThis |
     // `e` is a call to a method that releases `this`, and `released`
     // is the object that is called
-    e.(FunctionCall).getTarget() = f and
+    (
+      e.(FunctionCall).getTarget() = f or
+      e.(FunctionCall).getTarget().(MemberFunction).getAnOverridingFunction+() = f
+    ) and
     e.(FunctionCall).getQualifier() = exprOrDereference(released) and
     innerThis.getEnclosingFunction() = f and
     exprReleases(_, innerThis, kind)

cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/AV Rule 79.expected

@@ -8,6 +8,7 @@
 | DeleteThis.cpp:56:3:56:24 | ... = ... | Resource ptr10 is acquired by class MyClass3 but not released anywhere in this class. |
 | DeleteThis.cpp:58:3:58:24 | ... = ... | Resource ptr12 is acquired by class MyClass3 but not released anywhere in this class. |
 | DeleteThis.cpp:60:3:60:24 | ... = ... | Resource ptr14 is acquired by class MyClass3 but not released anywhere in this class. |
+| DeleteThis.cpp:127:3:127:20 | ... = ... | Resource d is acquired by class MyClass9 but not released anywhere in this class. |
 | ExternalOwners.cpp:49:3:49:20 | ... = ... | Resource a is acquired by class MyScreen but not released anywhere in this class. |
 | ListDelete.cpp:21:3:21:21 | ... = ... | Resource first is acquired by class MyThingColection but not released anywhere in this class. |
 | NoDestructor.cpp:23:3:23:20 | ... = ... | Resource n is acquired by class MyClass5 but not released anywhere in this class. |

cpp/ql/test/query-tests/jsf/4.10 Classes/AV Rule 79/DeleteThis.cpp

@@ -82,3 +82,67 @@ private:
 	MyClass2 *ptr10, *ptr11, *ptr12, *ptr13, *ptr14, *ptr15;
 	MyClass2 *ptr20;
 };
+
+class MyClass4
+{
+public:
+	virtual void Release() = 0;
+};
+
+class MyClass5 : public MyClass4
+{
+public:
+	void Release()
+	{
+		delete this;
+	}
+};
+
+class MyClass6 : public MyClass5
+{
+};
+
+class MyClass7 : public MyClass4
+{
+public:
+	void Release()
+	{
+		// do nothing
+	}
+};
+
+class MyClass8 : public MyClass7
+{
+};
+
+class MyClass9
+{
+public:
+	MyClass9()
+	{
+		a = new MyClass5(); // GOOD
+		b = new MyClass5(); // GOOD
+		c = new MyClass6(); // GOOD
+
+		d = new MyClass7(); // BAD
+		e = new MyClass7(); // BAD [NOT DETECTED]
+		f = new MyClass8(); // BAD [NOT DETECTED]
+	}
+ 	~MyClass9()
+	{
+		a->Release(); // MyClass5::Release()
+		b->Release(); // MyClass5::Release()
+		c->Release(); // MyClass5::Release()
+
+		d->Release(); // MyClass7::Release()
+		e->Release(); // MyClass7::Release()
+		f->Release(); // MyClass7::Release() 
+	}
+ 	MyClass5 *a;
+	MyClass4 *b;
+	MyClass4 *c;
+
+	MyClass7 *d;
+	MyClass4 *e;
+	MyClass4 *f;
+};