Fix FP reported in #366

change_notes/2024-01-31-fix-fp-a4-5-1.md

@@ -0,0 +1,4 @@
+`A4-5-1`: `EnumUsedInArithmeticContexts.ql`:
+    - Address incorrect exclusion of the binary operator `&`.
+    - Address incorrect inclusion of the unary operator `&`.
+    - Fix FP reported in #366.

cpp/autosar/src/rules/A4-5-1/EnumUsedInArithmeticContexts.ql

@@ -18,44 +18,26 @@
 
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.Operator
+import codingstandards.cpp.Type
 
-/*
- * Get an operand to all overloaded operator member functions, except:
- *   operator[]
- *   operator=
- *   operator==
- *   operator!=
- *   operator&
- *   operator<
- *   operator<=
- *   operator>
- *   operator>=
- */
-
-Expr getAnOperandOfAllowedOverloadedOperator(FunctionCall fc) {
-  fc.getAnArgument() = result and
-  fc.getTarget().getName().regexpMatch("operator(?!\\[]$|=$|==$|!=$|&$|<$|<=$|>$|>=$).+")
-}
-
-Expr getAnOperandOfAllowedOperation(Operation o) {
-  o.getAnOperand() = result and
-  not (
-    o instanceof AssignExpr or
-    o instanceof BitwiseAndExpr or
-    o instanceof ComparisonOperation
-  )
+class AllowedOperatorUse extends OperatorUse {
+  AllowedOperatorUse() {
+    this.getOperator() in ["[]", "=", "==", "!=", "<", "<=", ">", ">="]
+    or
+    this.(UnaryOperatorUse).getOperator() = "&"
+  }
 }
 
-from Expr e, Expr operand
+from OperatorUse operatorUse, Access access, Enum enum
 where
-  not isExcluded(e, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
+  not isExcluded(access, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
+  operatorUse.getAnOperand() = access and
   (
-    operand = getAnOperandOfAllowedOverloadedOperator(e)
-    or
-    operand = getAnOperandOfAllowedOperation(e)
+    access.(EnumConstantAccess).getTarget().getDeclaringEnum() = enum or
+    access.(VariableAccess).getType() = enum
   ) and
-  (
-    operand instanceof EnumConstantAccess or
-    operand.(VariableAccess).getType() instanceof Enum
-  )
-select e, "Enum $@ is used as an operand of arithmetic operation.", operand, "expression"
+  not operatorUse instanceof AllowedOperatorUse and
+  // Enums that implement the BitmaskType trait are an exception.
+  not enum instanceof BitmaskType
+select access, "Enum $@ is used as an operand of arithmetic operation.", enum, enum.getName()

cpp/autosar/test/rules/A4-5-1/enum.cpp

@@ -14,7 +14,6 @@ void test_enum() {
   Avenue <= Avenue;    // COMPLIANT
   Place > Road;        // COMPLIANT
   Boulevard >= Avenue; // COMPLIANT
-  Place &Avenue;       // COMPLIANT
   arr[Road] = 1;       // COMPLIANT
 
   // arithmetic
@@ -40,6 +39,7 @@ void test_enum() {
   Road ^= Road;          // NON_COMPLIANT
   Road >>= Road;         // NON_COMPLIANT
   Road <<= Road;         // NON_COMPLIANT
+  Road &Road;            // NON_COMPLIANT
 }
 
 void test_enum_var() {
@@ -51,7 +51,7 @@ void test_enum_var() {
   a <= b;          // COMPLIANT
   a > b;           // COMPLIANT
   a >= b;          // COMPLIANT
-  a &b;            // COMPLIANT
+  Street *c = &a;  // COMPLIANT
 
   // arithmetic
   a + a; // NON_COMPLIANT
@@ -76,4 +76,5 @@ void test_enum_var() {
   a ^= 1;  // NON_COMPLIANT
   a >>= 1; // NON_COMPLIANT
   a <<= 1; // NON_COMPLIANT
+  a &b;    // NON_COMPLIANT
 }

cpp/autosar/test/rules/A4-5-1/enum_class.cpp

@@ -43,7 +43,6 @@ void test_enum_class() {
   FunctionalLang::Elm <= FunctionalLang::Haskell;   // COMPLIANT
   FunctionalLang::Idris > FunctionalLang::SML;      // COMPLIANT
   FunctionalLang::Haskell >= FunctionalLang::Idris; // COMPLIANT
-  FunctionalLang::FSharp &FunctionalLang::OCaml;    // COMPLIANT
 
   // arithmetic
   FunctionalLang::ML + 1;                   // NON_COMPLIANT
@@ -59,15 +58,16 @@ void test_enum_class() {
   !FunctionalLang::Scheme;                        // NON_COMPLIANT
 
   // bitwise
-  FunctionalLang::Elm | FunctionalLang::Racket; // NON_COMPLIANT
-  ~FunctionalLang::Idris;                       // NON_COMPLIANT
-  FunctionalLang::ML ^ FunctionalLang::OCaml;   // NON_COMPLIANT
-  FunctionalLang::OCaml >> 1;                   // NON_COMPLIANT
-  FunctionalLang::Lisp << 1;                    // NON_COMPLIANT
-  l &= FunctionalLang::OCaml;                   // NON_COMPLIANT
-  l ^= 1;                                       // NON_COMPLIANT
-  l >>= 1;                                      // NON_COMPLIANT
-  l <<= 1;                                      // NON_COMPLIANT
+  FunctionalLang::Elm | FunctionalLang::Racket;  // NON_COMPLIANT
+  ~FunctionalLang::Idris;                        // NON_COMPLIANT
+  FunctionalLang::ML ^ FunctionalLang::OCaml;    // NON_COMPLIANT
+  FunctionalLang::OCaml >> 1;                    // NON_COMPLIANT
+  FunctionalLang::Lisp << 1;                     // NON_COMPLIANT
+  l &= FunctionalLang::OCaml;                    // NON_COMPLIANT
+  l ^= 1;                                        // NON_COMPLIANT
+  l >>= 1;                                       // NON_COMPLIANT
+  l <<= 1;                                       // NON_COMPLIANT
+  FunctionalLang::FSharp &FunctionalLang::OCaml; // NON_COMPLIANT
 }
 
 void test_enum_class_vars() {
@@ -79,7 +79,7 @@ void test_enum_class_vars() {
   a <= b;                                    // COMPLIANT
   a > a;                                     // COMPLIANT
   a >= a;                                    // COMPLIANT
-  a &b;                                      // COMPLIANT
+  FunctionalLang *c = &a;                    // COMPLIANT
 
   // arithmetic
   a + 1; // NON_COMPLIANT
@@ -100,4 +100,21 @@ void test_enum_class_vars() {
   a ^ b;  // NON_COMPLIANT
   a >> 1; // NON_COMPLIANT
   a << 1; // NON_COMPLIANT
+  a &b;   // NON_COMPLIANT
+}
+
+enum class byte : unsigned char {};
+
+byte operator&(byte lhs, byte rhs) { return lhs; }
+byte operator|(byte lhs, byte rhs) { return lhs; }
+byte operator^(byte lhs, byte rhs) { return lhs; }
+byte operator~(byte lhs) { return lhs; }
+byte operator&=(byte lhs, byte rhs) { return lhs; }
+byte operator|=(byte lhs, byte rhs) { return lhs; }
+
+void test_bitmasktype_enum_class() { // COMPLIANT - byte implements the
+                                     // BitmaskType trait.
+  byte one, two;
+
+  one &two;
 }

cpp/common/src/codingstandards/cpp/Operator.qll

@@ -264,3 +264,64 @@ class UserOverloadedOperator extends Function {
     not this.isCompilerGenerated()
   }
 }
+
+private newtype TOperatorUse =
+  TBuiltinOperatorUse(Operation op) or
+  TOverloadedOperatorUse(FunctionCall call, Operator op) { op.getACallToThisFunction() = call }
+
+/**
+ * A class to reason about builtin operator and overloaded operator use.
+ */
+class OperatorUse extends TOperatorUse {
+  string toString() {
+    exists(Operation op | result = op.toString() and this = TBuiltinOperatorUse(op))
+    or
+    exists(Operator op | result = op.toString() and this = TOverloadedOperatorUse(_, op))
+  }
+
+  predicate isOverloaded() { this = TOverloadedOperatorUse(_, _) }
+
+  Operation asBuiltin() { this = TBuiltinOperatorUse(result) }
+
+  Operator asOverloaded(FunctionCall call) { this = TOverloadedOperatorUse(call, result) }
+
+  Type getType() {
+    result = this.asBuiltin().getType()
+    or
+    result = this.asOverloaded(_).getType()
+  }
+
+  Parameter getParameter(int index) { result = this.asOverloaded(_).getParameter(index) }
+
+  Parameter getAParameter() { result = this.asOverloaded(_).getParameter(_) }
+
+  Expr getAnOperand() {
+    result = this.asBuiltin().getAnOperand()
+    or
+    exists(FunctionCall call, Operator op | op = this.asOverloaded(call) |
+      result = call.getAnArgument()
+    )
+  }
+
+  Location getLocation() {
+    result = this.asBuiltin().getLocation()
+    or
+    exists(FunctionCall call, Operator op | op = this.asOverloaded(call) |
+      result = call.getLocation()
+    )
+  }
+
+  string getOperator() {
+    result = this.asBuiltin().getOperator()
+    or
+    result = this.asOverloaded(_).getName().regexpCapture("^operator(.*)$", 1)
+  }
+}
+
+class UnaryOperatorUse extends OperatorUse {
+  UnaryOperatorUse() {
+    this.asBuiltin() instanceof UnaryOperation
+    or
+    this.asOverloaded(_).getNumberOfParameters() = 0
+  }
+}

cpp/common/src/codingstandards/cpp/Type.qll

@@ -23,6 +23,34 @@ class IncompleteType extends Class {
   IncompleteType() { not hasDefinition() }
 }
 
+/**
+ * A type that implements the BitmaskType trait.
+ * https://en.cppreference.com/w/cpp/named_req/BitmaskType
+ */
+abstract class BitmaskType extends Type { }
+
+/**
+ * Holds if `enum` implements required overload `overload` to implement
+ * the BitmaskType trait.
+ */
+private predicate isRequiredEnumOverload(Enum enum, Function overload) {
+  overload.getName().regexpMatch("operator([&|^~]|&=|\\|=)") and
+  forex(Parameter p | p = overload.getAParameter() |
+    (
+      p.getType() = enum
+      or
+      p.getType().(ReferenceType).getBaseType() = enum
+    )
+  )
+}
+
+private class EnumBitmaskType extends BitmaskType, Enum {
+  EnumBitmaskType() {
+    // Implements all the required overload
+    count(Function overload | isRequiredEnumOverload(this, overload)) = 6
+  }
+}
+
 /**
  * A type without `const` and `volatile` specifiers.
  */