Merge pull request github#1303 from jbj/hasQualifiedName

C++: Fix `getQualifiedName` performance issues

Author: rdmarsh2

change-notes/1.21/analysis-cpp.md

@@ -30,6 +30,8 @@
 | `()`-declared function called with too many arguments (`cpp/futile-params`) | Improved coverage | Query has been generalized to find all cases where the number of arguments exceedes the number of parameters of the function, provided the function is also properly declared/defined elsewhere. |
 
 ## Changes to QL libraries
+- The predicate `Declaration.hasGlobalName` now only holds for declarations that are not nested in a class. For example, it no longer holds for a member function `MyClass::myFunction` or a constructor `MyClass::MyClass`, whereas previously it would classify those two declarations as global names.
+- In class `Declaration`, predicates `getQualifiedName/0` and `hasQualifiedName/1` are no longer recommended for finding functions by name. Instead, use `hasGlobalName/1` and the new `hasQualifiedName/2` and `hasQualifiedName/3` predicates. This improves performance and makes it more reliable to identify names involving templates.
 - Additional support for definition by reference has been added to the `semmle.code.cpp.dataflow.TaintTracking` library.
     - The taint tracking library now includes taint-specific edges for functions modeled in `semmle.code.cpp.models.interfaces.DataFlow`.
     - The taint tracking library adds flow through library functions that are modeled in `semmle.code.cpp.models.interfaces.Taint`. Queries can add subclasses of `TaintFunction` to specify additional flow.

cpp/ql/src/Critical/DescriptorMayNotBeClosed.ql

@@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo
 import Negativity
 
 predicate closeCall(FunctionCall fc, Variable v) {
-  fc.getTarget().hasQualifiedName("close") and v.getAnAccess() = fc.getArgument(0)
+  fc.getTarget().hasGlobalName("close") and v.getAnAccess() = fc.getArgument(0)
   or
   exists(FunctionCall midcall, Function mid, int arg |
     fc.getArgument(arg) = v.getAnAccess() and

cpp/ql/src/Critical/DescriptorNeverClosed.ql

@@ -13,7 +13,7 @@ import semmle.code.cpp.pointsto.PointsTo
 
 predicate closed(Expr e) {
   exists(FunctionCall fc |
-    fc.getTarget().hasQualifiedName("close") and
+    fc.getTarget().hasGlobalName("close") and
     fc.getArgument(0) = e
   )
 }

cpp/ql/src/Critical/GlobalUseBeforeInit.ql

@@ -30,7 +30,7 @@ predicate useFunc(GlobalVariable v, Function f) {
 }
 
 predicate uninitialisedBefore(GlobalVariable v, Function f) {
-  f.hasQualifiedName("main")
+  f.hasGlobalName("main")
   or
   exists(Call call, Function g |
     uninitialisedBefore(v, g) and

cpp/ql/src/Critical/InitialisationNotRun.ql

@@ -20,7 +20,7 @@ predicate global(GlobalVariable v) {
 }
 
 predicate mainCalled(Function f) {
-  f.getQualifiedName() = "main"
+  f.hasGlobalName("main")
   or
   exists(Function caller | mainCalled(caller) and allCalls(caller, f))
 }

cpp/ql/src/Critical/MemoryMayNotBeFreed.ql

@@ -55,7 +55,7 @@ predicate allocCallOrIndirect(Expr e) {
  * can cause memory leaks.
  */
 predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode verified) {
-  reallocCall.getTarget().hasQualifiedName("realloc") and
+  reallocCall.getTarget().hasGlobalName("realloc") and
   reallocCall.getArgument(0) = v.getAnAccess() and
   (
     exists(Variable newV, ControlFlowNode node |
@@ -82,7 +82,7 @@ predicate verifiedRealloc(FunctionCall reallocCall, Variable v, ControlFlowNode
 predicate freeCallOrIndirect(ControlFlowNode n, Variable v) {
   // direct free call
   freeCall(n, v.getAnAccess()) and
-  not n.(FunctionCall).getTarget().hasQualifiedName("realloc")
+  not n.(FunctionCall).getTarget().hasGlobalName("realloc")
   or
   // verified realloc call
   verifiedRealloc(_, v, n)

cpp/ql/src/Critical/OverflowCalculated.ql

@@ -14,8 +14,8 @@ import cpp
 
 class MallocCall extends FunctionCall {
   MallocCall() {
-    this.getTarget().hasQualifiedName("malloc") or
-    this.getTarget().hasQualifiedName("std::malloc")
+    this.getTarget().hasGlobalName("malloc") or
+    this.getTarget().hasQualifiedName("std", "malloc")
   }
 
   Expr getAllocatedSize() {
@@ -36,12 +36,12 @@ predicate spaceProblem(FunctionCall append, string msg) {
     malloc.getAllocatedSize() = add and
     buffer.getAnAccess() = strlen.getStringExpr() and
     (
-      insert.getTarget().hasQualifiedName("strcpy") or
-      insert.getTarget().hasQualifiedName("strncpy")
+      insert.getTarget().hasGlobalName("strcpy") or
+      insert.getTarget().hasGlobalName("strncpy")
     ) and
     (
-      append.getTarget().hasQualifiedName("strcat") or
-      append.getTarget().hasQualifiedName("strncat")
+      append.getTarget().hasGlobalName("strcat") or
+      append.getTarget().hasGlobalName("strncat")
     ) and
     malloc.getASuccessor+() = insert and
     insert.getArgument(1) = buffer.getAnAccess() and

cpp/ql/src/Critical/OverflowDestination.ql

@@ -25,7 +25,7 @@ import semmle.code.cpp.security.TaintTracking
 predicate sourceSized(FunctionCall fc, Expr src) {
   exists(string name |
     (name = "strncpy" or name = "strncat" or name = "memcpy" or name = "memmove") and
-    fc.getTarget().hasQualifiedName(name)
+    fc.getTarget().hasGlobalName(name)
   ) and
   exists(Expr dest, Expr size, Variable v |
     fc.getArgument(0) = dest and

cpp/ql/src/Critical/OverflowStatic.ql

@@ -59,21 +59,21 @@ predicate overflowOffsetInLoop(BufferAccess bufaccess, string msg) {
 }
 
 predicate bufferAndSizeFunction(Function f, int buf, int size) {
-  f.hasQualifiedName("read") and buf = 1 and size = 2
+  f.hasGlobalName("read") and buf = 1 and size = 2
   or
-  f.hasQualifiedName("fgets") and buf = 0 and size = 1
+  f.hasGlobalName("fgets") and buf = 0 and size = 1
   or
-  f.hasQualifiedName("strncpy") and buf = 0 and size = 2
+  f.hasGlobalName("strncpy") and buf = 0 and size = 2
   or
-  f.hasQualifiedName("strncat") and buf = 0 and size = 2
+  f.hasGlobalName("strncat") and buf = 0 and size = 2
   or
-  f.hasQualifiedName("memcpy") and buf = 0 and size = 2
+  f.hasGlobalName("memcpy") and buf = 0 and size = 2
   or
-  f.hasQualifiedName("memmove") and buf = 0 and size = 2
+  f.hasGlobalName("memmove") and buf = 0 and size = 2
   or
-  f.hasQualifiedName("snprintf") and buf = 0 and size = 1
+  f.hasGlobalName("snprintf") and buf = 0 and size = 1
   or
-  f.hasQualifiedName("vsnprintf") and buf = 0 and size = 1
+  f.hasGlobalName("vsnprintf") and buf = 0 and size = 1
 }
 
 class CallWithBufferSize extends FunctionCall {

cpp/ql/src/Critical/SizeCheck.ql

@@ -17,12 +17,12 @@ import cpp
 class Allocation extends FunctionCall {
   Allocation() {
     exists(string name |
-      this.getTarget().hasQualifiedName(name) and
+      this.getTarget().hasGlobalName(name) and
       (name = "malloc" or name = "calloc" or name = "realloc")
     )
   }
 
-  string getName() { result = this.getTarget().getQualifiedName() }
+  private string getName() { this.getTarget().hasGlobalName(result) }
 
   int getSize() {
     this.getName() = "malloc" and