Merge pull request #781 from github/lcartey/2-37-0-performance-improv…

…ements

Performance improvements since upgrade to 2.16.6

c/cert/src/rules/DCL40-C/IncompatibleFunctionDeclarations.ql

@@ -16,30 +16,32 @@
 
 import cpp
 import codingstandards.c.cert
+import codingstandards.cpp.Compatible
 import ExternalIdentifiers
 
-//checks if they are incompatible based on return type, number of parameters and parameter types
-predicate checkMatchingFunction(FunctionDeclarationEntry d, FunctionDeclarationEntry d2) {
-  not d.getType() = d2.getType()
-  or
-  not d.getNumberOfParameters() = d2.getNumberOfParameters()
-  or
-  exists(ParameterDeclarationEntry p, ParameterDeclarationEntry p2, int i |
-    d.getParameterDeclarationEntry(i) = p and
-    d2.getParameterDeclarationEntry(i) = p2 and
-    not p.getType() = p2.getType()
-  )
-}
-
 from ExternalIdentifiers d, FunctionDeclarationEntry f1, FunctionDeclarationEntry f2
 where
   not isExcluded(f1, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
   not isExcluded(f2, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
-  f1 = d.getADeclarationEntry() and
-  f2 = d.getADeclarationEntry() and
   not f1 = f2 and
-  f1.getLocation().getStartLine() >= f2.getLocation().getStartLine() and
+  f1.getDeclaration() = d and
+  f2.getDeclaration() = d and
   f1.getName() = f2.getName() and
-  checkMatchingFunction(f1, f2)
+  (
+    //return type check
+    not typesCompatible(f1.getType(), f2.getType())
+    or
+    //parameter type check
+    parameterTypesIncompatible(f1, f2)
+    or
+    not f1.getNumberOfParameters() = f2.getNumberOfParameters()
+  ) and
+  // Apply ordering on start line, trying to avoid the optimiser applying this join too early
+  // in the pipeline
+  exists(int f1Line, int f2Line |
+    f1.getLocation().hasLocationInfo(_, f1Line, _, _, _) and
+    f2.getLocation().hasLocationInfo(_, f2Line, _, _, _) and
+    f1Line >= f2Line
+  )
 select f1, "The object $@ is not compatible with re-declaration $@", f1, f1.getName(), f2,
   f2.getName()

c/cert/src/rules/MSC39-C/DoNotCallVaArgOnAVaListThatHasAnIndeterminateValue.ql

@@ -71,12 +71,19 @@ predicate sameSource(VaAccess e1, VaAccess e2) {
   )
 }
 
+/**
+ * Extracted to avoid poor magic join ordering on the `isExcluded` predicate.
+ */
+predicate query(VaAccess va_acc, VaArgArg va_arg, FunctionCall fc) {
+  sameSource(va_acc, va_arg) and
+  fc = preceedsFC(va_acc) and
+  fc.getTarget().calls*(va_arg.getEnclosingFunction())
+}
+
 from VaAccess va_acc, VaArgArg va_arg, FunctionCall fc
 where
   not isExcluded(va_acc,
     Contracts7Package::doNotCallVaArgOnAVaListThatHasAnIndeterminateValueQuery()) and
-  sameSource(va_acc, va_arg) and
-  fc = preceedsFC(va_acc) and
-  fc.getTarget().calls*(va_arg.getEnclosingFunction())
+  query(va_acc, va_arg, fc)
 select va_acc, "The value of " + va_acc.toString() + " is indeterminate after the $@.", fc,
   fc.toString()

c/misra/src/rules/RULE-9-4/RepeatedInitializationOfAggregateObjectElement.ql

@@ -62,33 +62,31 @@ int getMaxDepth(ArrayAggregateLiteral al) {
 
 // internal recursive predicate for `hasMultipleInitializerExprsForSameIndex`
 predicate hasMultipleInitializerExprsForSameIndexInternal(
-  ArrayAggregateLiteral al1, ArrayAggregateLiteral al2, Expr out_al1_expr, Expr out_al2_expr
+  ArrayAggregateLiteral root, Expr e1, Expr e2
 ) {
-  exists(int shared_index, Expr al1_expr, Expr al2_expr |
-    // an `Expr` initializing an element of the same index in both `al1` and `al2`
-    shared_index = [0 .. al1.getArraySize() - 1] and
-    al1_expr = al1.getAnElementExpr(shared_index) and
-    al2_expr = al2.getAnElementExpr(shared_index) and
-    // but not the same `Expr`
-    not al1_expr = al2_expr and
-    (
-      // case A - the children are not aggregate literals
-      // holds if `al1` and `al2` both hold for .getElement[sharedIndex]
-      not al1_expr instanceof ArrayAggregateLiteral and
-      out_al1_expr = al1_expr and
-      out_al2_expr = al2_expr
-      or
-      // case B - `al1` and `al2` both have an aggregate literal child at the same index, so recurse
-      hasMultipleInitializerExprsForSameIndexInternal(al1_expr, al2_expr, out_al1_expr, out_al2_expr)
-    )
+  root = e1 and root = e2
+  or
+  exists(ArrayAggregateLiteral parent1, ArrayAggregateLiteral parent2, int shared_index |
+    hasMultipleInitializerExprsForSameIndexInternal(root, parent1, parent2) and
+    shared_index = [0 .. parent1.getArraySize() - 1] and
+    e1 = parent1.getAnElementExpr(shared_index) and
+    e2 = parent2.getAnElementExpr(shared_index)
   )
 }
 
 /**
  * Holds if `expr1` and `expr2` both initialize the same array element of `root`.
  */
 predicate hasMultipleInitializerExprsForSameIndex(ArrayAggregateLiteral root, Expr expr1, Expr expr2) {
-  hasMultipleInitializerExprsForSameIndexInternal(root, root, expr1, expr2)
+  hasMultipleInitializerExprsForSameIndexInternal(root, expr1, expr2) and
+  not root = expr1 and
+  not root = expr2 and
+  not expr1 = expr2 and
+  (
+    not expr1 instanceof ArrayAggregateLiteral
+    or
+    not expr2 instanceof ArrayAggregateLiteral
+  )
 }
 
 /**

change_notes/2024-10-24-compatible-types.md

@@ -0,0 +1,2 @@
+ - `DCL40-C` - `IncompatibleFunctionDeclarations.ql`:
+   - Reduce false positives by identifying compatible integer arithmetic types (e.g. "signed int" and "int").

cpp/common/src/codingstandards/cpp/Compatible.qll

@@ -1,5 +1,7 @@
 import cpp
 
+pragma[noinline]
+pragma[nomagic]
 predicate typesCompatible(Type t1, Type t2) {
   t1 = t2
   or
@@ -8,6 +10,7 @@ predicate typesCompatible(Type t1, Type t2) {
 }
 
 predicate parameterTypesIncompatible(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  f1.getDeclaration() = f2.getDeclaration() and
   exists(ParameterDeclarationEntry p1, ParameterDeclarationEntry p2, int i |
     p1 = f1.getParameterDeclarationEntry(i) and
     p2 = f2.getParameterDeclarationEntry(i)
@@ -17,6 +20,7 @@ predicate parameterTypesIncompatible(FunctionDeclarationEntry f1, FunctionDeclar
 }
 
 predicate parameterNamesIncompatible(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  f1.getDeclaration() = f2.getDeclaration() and
   exists(ParameterDeclarationEntry p1, ParameterDeclarationEntry p2, int i |
     p1 = f1.getParameterDeclarationEntry(i) and
     p2 = f2.getParameterDeclarationEntry(i)

cpp/common/src/codingstandards/cpp/rules/notdistinctidentifier/NotDistinctIdentifier.qll

@@ -12,6 +12,16 @@ abstract class NotDistinctIdentifierSharedQuery extends Query { }
 
 Query getQuery() { result instanceof NotDistinctIdentifierSharedQuery }
 
+bindingset[d, d2]
+pragma[inline_late]
+private predicate after(ExternalIdentifiers d, ExternalIdentifiers d2) {
+  exists(int dStartLine, int d2StartLine |
+    d.getLocation().hasLocationInfo(_, dStartLine, _, _, _) and
+    d2.getLocation().hasLocationInfo(_, d2StartLine, _, _, _) and
+    dStartLine >= d2StartLine
+  )
+}
+
 query predicate problems(
   ExternalIdentifiers d, string message, ExternalIdentifiers d2, string nameplaceholder
 ) {
@@ -20,10 +30,10 @@ query predicate problems(
   d.getName().length() >= 31 and
   d2.getName().length() >= 31 and
   not d = d2 and
-  d.getLocation().getStartLine() >= d2.getLocation().getStartLine() and
   d.getSignificantName() = d2.getSignificantName() and
   not d.getName() = d2.getName() and
   nameplaceholder = d2.getName() and
+  after(d, d2) and
   message =
     "External identifer " + d.getName() +
       " is nondistinct in characters at or over 31 limit, compared to $@"