Initial commit of experimental query cpp/guarded-free.

Author: mario-campos

cpp/ql/src/experimental/Best Practices/GuardedFree.cpp

@@ -0,0 +1,11 @@
+void test()
+{
+    char *foo = malloc(100);
+
+    // BAD
+    if (foo)          
+        free(foo);
+
+    // GOOD
+    free(foo);
+}

cpp/ql/src/experimental/Best Practices/GuardedFree.qhelp

@@ -0,0 +1,18 @@
+<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
+<qhelp>
+<overview>
+<p>The <code>free</code> function, which deallocates heap memory, may accept a NULL pointer and take no action. Therefore, it is unnecessary to check its argument for the value of NULL before a function call to <code>free</code>. As such, these guards may hinder performance and readability.</p>
+</overview>
+<recommendation>
+<p>A function call to <code>free</code> should not depend upon the value of its argument. Delete the <code>if</code> condition preceeding a function call to <code>free</code> when its only purpose is to check the value of the pointer to be freed.</p>
+</recommendation>
+<example>
+<sample src = "GuardedFree.cpp" />
+</example>
+<references>
+<li>
+    The Open Group Base Specifications Issue 7, 2018 Edition:
+    <a href="https://pubs.opengroup.org/onlinepubs/9699919799/functions/free.html">free - free allocated memory</a>
+</li>
+</references>
+</qhelp>

cpp/ql/src/experimental/Best Practices/GuardedFree.ql

@@ -0,0 +1,48 @@
+/**
+ * @name Guarded Free
+ * @description NULL-condition guards before function calls to the memory-deallocation
+ *              function free(3) are unnecessary, because passing NULL to free(3) is a no-op.
+ * @kind problem
+ * @problem.severity recommendation
+ * @precision very-high
+ * @id cpp/guarded-free
+ * @tags maintainability
+ *       readability
+ *       experimental
+ */
+
+import cpp
+
+class FreeCall extends FunctionCall {
+  FreeCall() { this.getTarget().hasName("free") }
+}
+
+from IfStmt stmt, FreeCall fc, Variable v
+where
+  stmt.getThen() = fc.getEnclosingStmt() and
+  (
+    stmt.getCondition() = v.getAnAccess() and
+    fc.getArgument(0) = v.getAnAccess()
+    or
+    exists(PointerDereferenceExpr cond, PointerDereferenceExpr arg |
+      fc.getArgument(0) = arg and
+      stmt.getCondition() = cond and
+      cond.getOperand+() = v.getAnAccess() and
+      arg.getOperand+() = v.getAnAccess()
+    )
+    or
+    exists(ArrayExpr cond, ArrayExpr arg |
+      fc.getArgument(0) = arg and
+      stmt.getCondition() = cond and
+      cond.getArrayBase+() = v.getAnAccess() and
+      arg.getArrayBase+() = v.getAnAccess()
+    )
+    or
+    exists(NEExpr eq |
+      fc.getArgument(0) = v.getAnAccess() and
+      stmt.getCondition() = eq and
+      eq.getAnOperand() = v.getAnAccess() and
+      eq.getAnOperand().getValue() = "0"
+    )
+  )
+select stmt, "unnecessary NULL check before call to $@", fc, "free"