First pass at addressing cross-compiler compatibility in MISRA 2023.

Handles clang findings and gcc findings.

Many issues were merely updates to the test cases, however, additional
work has been done to properly handle tgmath.h and stdatomic.h macros
across gcc and clang results.

Author: MichaelRFairhurst

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/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/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,17 @@ 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

@@ -59,14 +59,14 @@ Type canonicalize(Type type) {
 }
 
 Type getEffectiveStandardType(Expr e) {
-  result = canonicalize(getPromotedType(e.getExplicitlyConverted()))
+  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,

c/misra/src/rules/RULE-21-25/InvalidMemoryOrderArgument.ql

@@ -14,11 +14,23 @@
 
 import cpp
 import codingstandards.c.misra
+import codingstandards.cpp.StdFunctionOrMacro
 import semmle.code.cpp.dataflow.new.DataFlow
 
+class MemoryOrderEnum extends Enum {
+  MemoryOrderEnum() {
+    this.hasGlobalOrStdName("memory_order")
+    or
+    exists(TypedefType t |
+      t.getName() = "memory_order" and
+      t.getBaseType() = this
+    )
+  }
+}
+
 /* A member of the set of memory orders defined in the `memory_order` enum */
 class MemoryOrder extends EnumConstant {
-  MemoryOrder() { getDeclaringEnum().getName() = "memory_order" }
+  MemoryOrder() { getDeclaringEnum() instanceof MemoryOrderEnum }
 
   int getIntValue() { result = getValue().toInt() }
 }
@@ -49,59 +61,6 @@ class MemoryOrderConstantExpr extends Expr {
   string getMemoryOrderString() { result = ord.getName() }
 }
 
-/**
- * A `stdatomic.h` function which accepts a `memory_order` value as a parameter.
- */
-class MemoryOrderedStdAtomicFunction extends Function {
-  int orderParamIdx;
-
-  MemoryOrderedStdAtomicFunction() {
-    exists(int baseParamIdx, int baseParams, string prefix, string regex, string basename |
-      regex = "__(c11_)?atomic_([a-z_]+)" and
-      prefix = getName().regexpCapture(regex, 1) and
-      basename = "atomic_" + getName().regexpCapture(regex, 2) + ["", "_explicit"] and
-      (
-        basename in ["atomic_thread_fence", "atomic_signal_fence"] and
-        baseParamIdx = 0 and
-        baseParams = 1
-        or
-        basename in ["atomic_load", "atomic_flag_clear", "atomic_flag_test_and_set"] and
-        baseParamIdx = 1 and
-        baseParams = 2
-        or
-        basename in [
-            "atomic_store", "atomic_fetch_" + ["add", "sub", "or", "xor", "and"], "atomic_exchange"
-          ] and
-        baseParamIdx = 2 and
-        baseParams = 3
-        or
-        basename in ["atomic_compare_exchange_" + ["strong", "weak"]] and
-        baseParamIdx = [3, 4] and
-        baseParams = 5
-      ) and
-      (
-        // GCC case, may have one or two inserted parameters, e.g.:
-        // __atomic_load(8, &repr->a, &desired, order)
-        // or
-        // __atomic_load_8(&repr->a, &desired, order)
-        prefix = "" and
-        exists(int extraParams |
-          extraParams = getNumberOfParameters() - baseParams and
-          extraParams >= 0 and
-          orderParamIdx = baseParamIdx + extraParams
-        )
-        or
-        // Clang case, no inserted parameters:
-        // __c11_atomic_load(object, order)
-        prefix = "c11_" and
-        orderParamIdx = baseParamIdx
-      )
-    )
-  }
-
-  int getOrderParameterIdx() { result = orderParamIdx }
-}
-
 module MemoryOrderFlowConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node node) {
     // Direct usage of memory order constant
@@ -118,9 +77,8 @@ module MemoryOrderFlowConfig implements DataFlow::ConfigSig {
   }
 
   predicate isSink(DataFlow::Node node) {
-    exists(FunctionCall fc |
-      node.asExpr() =
-        fc.getArgument(fc.getTarget().(MemoryOrderedStdAtomicFunction).getOrderParameterIdx())
+    exists(AtomicallySequencedCall call |
+      call.getAMemoryOrderArgument() = node.asExpr()
     )
   }
 }
@@ -140,7 +98,7 @@ string describeMemoryOrderNode(DataFlow::Node node) {
 }
 
 from
-  Expr argument, Function function, string value, MemoryOrderFlow::PathNode source,
+  Expr argument, AtomicallySequencedCall function, string value, MemoryOrderFlow::PathNode source,
   MemoryOrderFlow::PathNode sink
 where
   not isExcluded(argument, Concurrency6Package::invalidMemoryOrderArgumentQuery()) and
@@ -149,6 +107,6 @@ where
   value = describeMemoryOrderNode(source.getNode()) and
   // Double check that we didn't find flow from something equivalent to the allowed value.
   not value = any(AllowedMemoryOrder e).getName() and
-  function.getACallToThisFunction().getAnArgument() = argument
+  function.getAMemoryOrderArgument() = argument
 select argument, source, sink, "Invalid memory order '$@' in call to function '$@'.", value, value,
   function, function.getName()

c/misra/src/rules/RULE-9-7/UninitializedAtomicObject.ql

@@ -31,7 +31,11 @@ class ThreadSpawningFunction extends Function {
 }
 
 class AtomicInitAddressOfExpr extends AddressOfExpr {
-  AtomicInitAddressOfExpr() { exists(AtomicInitCall c | this = c.getArgument(0)) }
+  AtomicInitAddressOfExpr() {
+    // StdFunctionOrMacro arguments are not necessarily reliable, so we look for any AddressOfExpr
+    // that is an argument to a call to `atomic_init`.
+    exists(AtomicInitCall c | this = c.getAnArgument())
+  }
 }
 
 ControlFlowNode getARequiredInitializationPoint(LocalScopeVariable v) {

c/misra/test/rules/DIR-5-1/test.c

@@ -99,7 +99,7 @@ void many_thread13_calls_nonreentrant_funcs(void *p) {
   wcsrtombs(NULL, NULL, 0, NULL); // NON-COMPLIANT
 }
 
-void main() {
+int main(int argc, char *argv[]) {
   thrd_t single_thread1;
   thrd_t many_thread2;
   thrd_t single_thread3;

c/misra/test/rules/DIR-5-3/test.c

@@ -14,7 +14,7 @@ void func_called_from_main(void);
 void make_threads_called_from_func_called_from_main(void);
 void make_threads_called_from_main_pthread_thrd(void);
 
-void main() {
+int main(int argc, char *argv[]) {
   thrd_create(&g1, &thrd_func, NULL);             // COMPLIANT
   pthread_create(&g2, NULL, &pthread_func, NULL); // COMPLIANT
 

c/misra/test/rules/RULE-1-5/UseOfObsoleteMacroAtomicVarInit.expected.gcc

@@ -0,0 +1 @@
+| test.c:29:18:29:36 | ATOMIC_VAR_INIT(VALUE) | Usage of macro ATOMIC_VAR_INIT() is declared obscelescent in C18, and discouraged in earlier C versions. |