Merge branch 'main' into michaelrfairhurst/implement-cpp-package-floating-point

Author: lcartey

amendments.csv

@@ -24,7 +24,7 @@ c,MISRA-C-2012,Amendment4,RULE-8-9,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Amendment4,RULE-9-4,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Amendment4,RULE-10-1,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Amendment4,RULE-18-3,Yes,Clarification,Yes,Import
-c,MISRA-C-2012,Amendment4,RULE-1-4,Yes,Replace,No,Easy
+c,MISRA-C-2012,Amendment4,RULE-1-4,Yes,Replace,Yes,Easy
 c,MISRA-C-2012,Amendment4,RULE-9-1,Yes,Refine,Yes,Easy
 c,MISRA-C-2012,Corrigendum2,DIR-4-10,Yes,Clarification,Yes,Import
 c,MISRA-C-2012,Corrigendum2,RULE-7-4,Yes,Refine,Yes,Easy

c/cert/src/codeql-suites/cert-c-default.qls

@@ -0,0 +1,10 @@
+- description: CERT C 2016 (Default)
+- qlpack: codeql/cert-c-coding-standards
+- include:
+    kind:
+    - problem
+    - path-problem
+    - external/cert/obligation/rule
+- exclude:
+    tags contain:
+    - external/cert/default-disabled

c/cert/src/codeql-suites/cert-c-recommendation.qls

@@ -0,0 +1,10 @@
+- description: CERT C 2016 (Recommendations)
+- qlpack: codeql/cert-c-coding-standards
+- include:
+    kind:
+    - problem
+    - path-problem
+    - external/cert/obligation/recommendation
+- exclude:
+    tags contain:
+    - external/cert/default-disabled

c/cert/src/codeql-suites/cert-default.qls

@@ -1,9 +1,2 @@
-- description: CERT C 2016 (Default)
-- qlpack: codeql/cert-c-coding-standards
-- include:
-    kind:
-    - problem
-    - path-problem
-- exclude:
-    tags contain:
-    - external/cert/default-disabled
+- description: "DEPRECATED - CERT C 2016 - use cert-c-default.qls instead"
+- import: codeql-suites/cert-c-default.qls

c/cert/src/qlpack.yml

@@ -1,8 +1,9 @@
 name: codeql/cert-c-coding-standards
-version: 2.44.0-dev
+version: 2.45.0-dev
 description: CERT C 2016
 suites: codeql-suites
 license: MIT
+default-suite-file: codeql-suites/cert-c-default.qls
 dependencies:
   codeql/common-c-coding-standards: '*'
   codeql/cpp-all: 2.1.1

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

@@ -19,6 +19,12 @@ import codingstandards.c.cert
 import codingstandards.cpp.types.Compatible
 import ExternalIdentifiers
 
+predicate interestedInFunctions(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) {
+  not f1 = f2 and
+  f1.getDeclaration() = f2.getDeclaration() and
+  f1.getName() = f2.getName()
+}
+
 from ExternalIdentifiers d, FunctionDeclarationEntry f1, FunctionDeclarationEntry f2
 where
   not isExcluded(f1, Declarations2Package::incompatibleFunctionDeclarationsQuery()) and
@@ -29,10 +35,12 @@ where
   f1.getName() = f2.getName() and
   (
     //return type check
-    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalReturnTypes(f1, f2)
+    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalReturnTypes(f1,
+      f2)
     or
     //parameter type check
-    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig>::equalParameterTypes(f1, f2)
+    not FunctionDeclarationTypeEquivalence<TypesCompatibleConfig, interestedInFunctions/2>::equalParameterTypes(f1,
+      f2)
   ) and
   // Apply ordering on start line, trying to avoid the optimiser applying this join too early
   // in the pipeline

c/cert/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cert-c-coding-standards-tests
-version: 2.44.0-dev
+version: 2.45.0-dev
 extractor: cpp
 license: MIT
 dependencies:

c/common/src/codingstandards/c/TgMath.qll

@@ -1,21 +1,38 @@
 import cpp
 
-private string getATgMathMacroName(boolean allowComplex) {
+private string getATgMathMacroName(boolean allowComplex, int numberOfParameters) {
   allowComplex = true and
+  numberOfParameters = 1 and
   result =
     [
       "acos", "acosh", "asin", "asinh", "atan", "atanh", "carg", "cimag", "conj", "cos", "cosh",
-      "cproj", "creal", "exp", "fabs", "log", "pow", "sin", "sinh", "sqrt", "tan", "tanh"
+      "cproj", "creal", "exp", "fabs", "log", "sin", "sinh", "sqrt", "tan", "tanh"
+    ]
+  or
+  allowComplex = true and
+  numberOfParameters = 2 and
+  result = "pow"
+  or
+  allowComplex = false and
+  numberOfParameters = 1 and
+  result =
+    [
+      "cbrt", "ceil", "erf", "erfc", "exp2", "expm1", "floor", "ilogb", "lgamma", "llrint",
+      "llround", "log10", "log1p", "log2", "logb", "lrint", "lround", "nearbyint", "rint", "round",
+      "tgamma", "trunc",
     ]
   or
   allowComplex = false and
+  numberOfParameters = 2 and
   result =
     [
-      "atan2", "cbrt", "ceil", "copysign", "erf", "erfc", "exp2", "expm1", "fdim", "floor", "fma",
-      "fmax", "fmin", "fmod", "frexp", "hypot", "ilogb", "ldexp", "lgamma", "llrint", "llround",
-      "log10", "log1p", "log2", "logb", "lrint", "lround", "nearbyint", "nextafter", "nexttoward",
-      "remainder", "remquo", "rint", "round", "scalbn", "scalbln", "tgamma", "trunc",
+      "atan2", "copysign", "fdim", "fmax", "fmin", "fmod", "frexp", "hypot", "ldexp", "nextafter",
+      "nexttoward", "remainder", "scalbn", "scalbln"
     ]
+  or
+  allowComplex = false and
+  numberOfParameters = 3 and
+  result = ["fma", "remquo"]
 }
 
 private predicate hasOutputArgument(string macroName, int index) {
@@ -27,19 +44,41 @@ private predicate hasOutputArgument(string macroName, int index) {
 class TgMathInvocation extends MacroInvocation {
   Call call;
   boolean allowComplex;
+  int numberOfParameters;
 
   TgMathInvocation() {
-    this.getMacro().getName() = getATgMathMacroName(allowComplex) and
+    this.getMacro().getName() = getATgMathMacroName(allowComplex, numberOfParameters) and
     call = getBestCallInExpansion(this)
   }
 
+  /** Account for extra parameters added by gcc */
+  private int getParameterOffset() {
+    // Gcc calls look something like: `__builtin_tgmath(cosf, cosd, cosl, arg)`, in this example
+    // there is a parameter offset of 3, so `getOperandArgument(0)` is equivalent to
+    // `call.getArgument(3)`.
+    result = call.getNumberOfArguments() - numberOfParameters
+  }
+
   Expr getOperandArgument(int i) {
-    result = call.getArgument(i) and
-    not hasOutputArgument(call.getTarget().getName(), i)
+    i >= 0 and
+    result = call.getArgument(i + getParameterOffset()) and
+    //i in [0..numberOfParameters - 1] and
+    not hasOutputArgument(getMacro().getName(), i)
+  }
+
+  /** Get all explicit conversions, except those added by clang in the macro body */
+  Expr getExplicitlyConvertedOperandArgument(int i) {
+    exists(Expr explicitConv |
+      explicitConv = getOperandArgument(i).getExplicitlyConverted() and
+      // clang explicitly casts most arguments, but not some integer arguments such as in `scalbn`.
+      if call.getTarget().getName().matches("__tg_%") and explicitConv instanceof Conversion
+      then result = explicitConv.(Conversion).getExpr()
+      else result = explicitConv
+    )
   }
 
   int getNumberOfOperandArguments() {
-    result = call.getNumberOfArguments() - count(int i | hasOutputArgument(getMacroName(), i))
+    result = numberOfParameters - count(int i | hasOutputArgument(getMacroName(), i))
   }
 
   Expr getAnOperandArgument() { result = getOperandArgument(_) }

c/common/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/common-c-coding-standards
-version: 2.44.0-dev
+version: 2.45.0-dev
 license: MIT
 dependencies:
   codeql/common-cpp-coding-standards: '*'

c/common/test/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/common-c-coding-standards-tests
-version: 2.44.0-dev
+version: 2.45.0-dev
 extractor: cpp
 license: MIT
 dependencies:

c/common/test/rules/functionnoreturnattributecondition/test.c

@@ -77,7 +77,7 @@ _Noreturn void test_noreturn_f10(int i) { // COMPLIANT
   case 4:
     thrd_exit(0);
     break;
-  default:
+  default:;
     jmp_buf jb;
     longjmp(jb, 0);
   }

c/misra/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/misra-c-coding-standards
-version: 2.44.0-dev
+version: 2.45.0-dev
 description: MISRA C 2012
 suites: codeql-suites
 license: MIT

c/misra/src/rules/RULE-13-2/UnsequencedAtomicReads.ql

@@ -17,15 +17,16 @@ import semmle.code.cpp.dataflow.TaintTracking
 import codingstandards.c.misra
 import codingstandards.c.Ordering
 import codingstandards.c.orderofevaluation.VariableAccessOrdering
+import codingstandards.cpp.StdFunctionOrMacro
 
 class AtomicAccessInFullExpressionOrdering extends Ordering::Configuration {
   AtomicAccessInFullExpressionOrdering() { this = "AtomicAccessInFullExpressionOrdering" }
 
   override predicate isCandidate(Expr e1, Expr e2) {
     exists(AtomicVariableAccess a, AtomicVariableAccess b, FullExpr e | a = e1 and b = e2 |
       a.getTarget() = b.getTarget() and
-      a.(ConstituentExpr).getFullExpr() = e and
-      b.(ConstituentExpr).getFullExpr() = e and
+      a.getARead().(ConstituentExpr).getFullExpr() = e and
+      b.getARead().(ConstituentExpr).getFullExpr() = e and
       not a = b
     )
   }
@@ -39,21 +40,40 @@ class AtomicAccessInFullExpressionOrdering extends Ordering::Configuration {
 class AtomicVariableAccess extends VariableAccess {
   AtomicVariableAccess() { getTarget().getType().hasSpecifier("atomic") }
 
-  /* Get the `atomic_<read|write>()` call this VarAccess occurs in. */
-  FunctionCall getAtomicFunctionCall() {
-    exists(AddressOfExpr addrParent, FunctionCall fc |
-      fc.getTarget().getName().matches("__c11_atomic%") and
+  /* Get the `atomic_load()` call this VarAccess occurs in. */
+  Expr getAtomicFunctionRead() {
+    exists(AddressOfExpr addrParent, AtomicReadOrWriteCall fc |
+      fc.getName().matches("atomic_load%") and
+      // StdFunctionOrMacro arguments are not necessarily reliable, so we look for any AddressOfExpr
+      // that is an argument to a call to `atomic_load`.
       addrParent = fc.getArgument(0) and
       addrParent.getAnOperand() = this and
-      result = fc
+      result = fc.getExpr()
+    )
+  }
+
+  /* Get the `atomic_store()` call this VarAccess occurs in. */
+  Expr getAtomicFunctionWrite(Expr storedValue) {
+    exists(AddressOfExpr addrParent, AtomicReadOrWriteCall fc |
+      addrParent = fc.getArgument(0) and
+      addrParent.getAnOperand() = this and
+      result = fc.getExpr() and
+      (
+        fc.getName().matches(["%store%", "%exchange%", "%fetch_%"]) and
+        not fc.getName().matches("%compare%") and
+        storedValue = fc.getArgument(1)
+        or
+        fc.getName().matches(["%compare%"]) and
+        storedValue = fc.getArgument(2)
+      )
     )
   }
 
   /**
    * Gets an assigned expr, either in the form `x = <result>` or `atomic_store(&x, <result>)`.
    */
   Expr getAnAssignedExpr() {
-    result = getAtomicFunctionCall().getArgument(1)
+    exists(getAtomicFunctionWrite(result))
     or
     exists(AssignExpr assign |
       assign.getLValue() = this and
@@ -65,19 +85,25 @@ class AtomicVariableAccess extends VariableAccess {
    * Gets the expression holding this variable access, either in the form `x` or `atomic_read(&x)`.
    */
   Expr getARead() {
-    result = getAtomicFunctionCall()
+    result = getAtomicFunctionRead()
     or
     result = this
   }
 }
 
 from
   AtomicAccessInFullExpressionOrdering config, FullExpr e, Variable v, AtomicVariableAccess va1,
-  AtomicVariableAccess va2
+  AtomicVariableAccess va2, Expr va1Read, Expr va2Read
 where
   not isExcluded(e, SideEffects3Package::unsequencedAtomicReadsQuery()) and
-  e = va1.(ConstituentExpr).getFullExpr() and
-  config.isUnsequenced(va1, va2) and
+  va1Read = va1.getARead() and
+  va2Read = va2.getARead() and
+  e = va1Read.(ConstituentExpr).getFullExpr() and
+  // Careful here. The `VariableAccess` in a pair of atomic function calls may not be unsequenced,
+  // for instance in gcc where atomic functions expand to StmtExprs, which have clear sequences.
+  // In this case, the result of `getARead()` for a pair of atomic function calls may be
+  // unsequenced even though the `VariableAccess`es within those calls are not.
+  config.isUnsequenced(va1Read, va2Read) and
   v = va1.getTarget() and
   v = va2.getTarget() and
   // Exclude cases where the variable is assigned a value tainted by the other variable access.

c/misra/src/rules/RULE-21-22/TgMathArgumentWithInvalidEssentialType.ql

@@ -34,14 +34,19 @@ string getAllowedTypesString(TgMathInvocation call) {
   else result = "essentially signed, unsigned, or real floating type"
 }
 
-from TgMathInvocation call, Expr arg, int argIndex, Type type, EssentialTypeCategory category
+from
+  TgMathInvocation call, Expr convertedArg, Expr unconverted, int argIndex, Type type,
+  EssentialTypeCategory category
 where
   not isExcluded(call, EssentialTypes2Package::tgMathArgumentWithInvalidEssentialTypeQuery()) and
-  arg = call.getOperandArgument(argIndex) and
-  type = getEssentialType(arg) and
+  // We must handle conversions specially, as clang inserts casts in the macro body we want to ignore.
+  convertedArg = call.getExplicitlyConvertedOperandArgument(argIndex) and
+  unconverted = convertedArg.getUnconverted() and
+  // Do not use `convertedArg.getEssentialType()`, as that is affected by clang's casts in the macro body.
+  type = getEssentialTypeBeforeConversions(convertedArg) and
   category = getEssentialTypeCategory(type) and
   not category = getAnAllowedEssentialTypeCategory(call)
-select arg,
+select unconverted,
   "Argument " + (argIndex + 1) + " provided to type-generic macro '" + call.getMacroName() +
     "' has " + category.toString().toLowerCase() + ", which is not " + getAllowedTypesString(call) +
     "."

c/misra/src/rules/RULE-21-23/TgMathArgumentsWithDifferingStandardType.ql

@@ -58,15 +58,13 @@ Type canonicalize(Type type) {
   else result = type
 }
 
-Type getEffectiveStandardType(Expr e) {
-  result = canonicalize(getPromotedType(e.getExplicitlyConverted()))
-}
+Type getEffectiveStandardType(Expr e) { result = canonicalize(getPromotedType(e)) }
 
 from TgMathInvocation call, Type firstType
 where
   not isExcluded(call, EssentialTypes2Package::tgMathArgumentsWithDifferingStandardTypeQuery()) and
-  firstType = getEffectiveStandardType(call.getAnOperandArgument()) and
-  not forall(Expr arg | arg = call.getAnOperandArgument() |
+  firstType = getEffectiveStandardType(call.getExplicitlyConvertedOperandArgument(0)) and
+  not forall(Expr arg | arg = call.getExplicitlyConvertedOperandArgument(_) |
     firstType = getEffectiveStandardType(arg)
   )
 select call,