Skip to content

Commit 6da55ef

Browse files
knewbury01knewbury01
authored
Merge pull request github#518 from rvermeulen/rvermeulen/fix-366
Fix FP reported in github#366
2 parents 5ff2a10 + 724e324 commit 6da55ef

File tree

7 files changed

+245
-144
lines changed

7 files changed

+245
-144
lines changed

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
`A4-5-1`: `EnumUsedInArithmeticContexts.ql`:
2+
- Address incorrect exclusion of the binary operator `&`.
3+
- Address incorrect inclusion of the unary operator `&`.
4+
- Fix FP reported in #366.

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

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -18,44 +18,26 @@
1818

1919
import cpp
2020
import codingstandards.cpp.autosar
21+
import codingstandards.cpp.Operator
22+
import codingstandards.cpp.Type
2123

22-
/*
23-
* Get an operand to all overloaded operator member functions, except:
24-
* operator[]
25-
* operator=
26-
* operator==
27-
* operator!=
28-
* operator&
29-
* operator<
30-
* operator<=
31-
* operator>
32-
* operator>=
33-
*/
34-
35-
Expr getAnOperandOfAllowedOverloadedOperator(FunctionCall fc) {
36-
fc.getAnArgument() = result and
37-
fc.getTarget().getName().regexpMatch("operator(?!\\[]$|=$|==$|!=$|&$|<$|<=$|>$|>=$).+")
38-
}
39-
40-
Expr getAnOperandOfAllowedOperation(Operation o) {
41-
o.getAnOperand() = result and
42-
not (
43-
o instanceof AssignExpr or
44-
o instanceof BitwiseAndExpr or
45-
o instanceof ComparisonOperation
46-
)
24+
class AllowedOperatorUse extends OperatorUse {
25+
AllowedOperatorUse() {
26+
this.getOperator() in ["[]", "=", "==", "!=", "<", "<=", ">", ">="]
27+
or
28+
this.(UnaryOperatorUse).getOperator() = "&"
29+
}
4730
}
4831

49-
from Expr e, Expr operand
32+
from OperatorUse operatorUse, Access access, Enum enum
5033
where
51-
not isExcluded(e, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
34+
not isExcluded(access, ExpressionsPackage::enumUsedInArithmeticContextsQuery()) and
35+
operatorUse.getAnOperand() = access and
5236
(
53-
operand = getAnOperandOfAllowedOverloadedOperator(e)
54-
or
55-
operand = getAnOperandOfAllowedOperation(e)
37+
access.(EnumConstantAccess).getTarget().getDeclaringEnum() = enum or
38+
access.(VariableAccess).getType() = enum
5639
) and
57-
(
58-
operand instanceof EnumConstantAccess or
59-
operand.(VariableAccess).getType() instanceof Enum
60-
)
61-
select e, "Enum $@ is used as an operand of arithmetic operation.", operand, "expression"
40+
not operatorUse instanceof AllowedOperatorUse and
41+
// Enums that implement the BitmaskType trait are an exception.
42+
not enum instanceof BitmaskType
43+
select access, "Enum $@ is used as an operand of arithmetic operation.", enum, enum.getName()

‎cpp/autosar/test/rules/A4-5-1/EnumUsedInArithmeticContexts.expected‎

Lines changed: 104 additions & 96 deletions
Large diffs are not rendered by default.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ void test_enum() {
1414
Avenue <= Avenue; // COMPLIANT
1515
Place > Road; // COMPLIANT
1616
Boulevard >= Avenue; // COMPLIANT
17-
Place &Avenue; // COMPLIANT
1817
arr[Road] = 1; // COMPLIANT
1918

2019
// arithmetic
@@ -40,6 +39,7 @@ void test_enum() {
4039
Road ^= Road; // NON_COMPLIANT
4140
Road >>= Road; // NON_COMPLIANT
4241
Road <<= Road; // NON_COMPLIANT
42+
Road &Road; // NON_COMPLIANT
4343
}
4444

4545
void test_enum_var() {
@@ -51,7 +51,7 @@ void test_enum_var() {
5151
a <= b; // COMPLIANT
5252
a > b; // COMPLIANT
5353
a >= b; // COMPLIANT
54-
a &b; // COMPLIANT
54+
Street *c = &a; // COMPLIANT
5555

5656
// arithmetic
5757
a + a; // NON_COMPLIANT
@@ -76,4 +76,5 @@ void test_enum_var() {
7676
a ^= 1; // NON_COMPLIANT
7777
a >>= 1; // NON_COMPLIANT
7878
a <<= 1; // NON_COMPLIANT
79+
a &b; // NON_COMPLIANT
7980
}

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

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ void test_enum_class() {
4343
FunctionalLang::Elm <= FunctionalLang::Haskell; // COMPLIANT
4444
FunctionalLang::Idris > FunctionalLang::SML; // COMPLIANT
4545
FunctionalLang::Haskell >= FunctionalLang::Idris; // COMPLIANT
46-
FunctionalLang::FSharp &FunctionalLang::OCaml; // COMPLIANT
4746

4847
// arithmetic
4948
FunctionalLang::ML + 1; // NON_COMPLIANT
@@ -59,15 +58,16 @@ void test_enum_class() {
5958
!FunctionalLang::Scheme; // NON_COMPLIANT
6059

6160
// bitwise
62-
FunctionalLang::Elm | FunctionalLang::Racket; // NON_COMPLIANT
63-
~FunctionalLang::Idris; // NON_COMPLIANT
64-
FunctionalLang::ML ^ FunctionalLang::OCaml; // NON_COMPLIANT
65-
FunctionalLang::OCaml >> 1; // NON_COMPLIANT
66-
FunctionalLang::Lisp << 1; // NON_COMPLIANT
67-
l &= FunctionalLang::OCaml; // NON_COMPLIANT
68-
l ^= 1; // NON_COMPLIANT
69-
l >>= 1; // NON_COMPLIANT
70-
l <<= 1; // NON_COMPLIANT
61+
FunctionalLang::Elm | FunctionalLang::Racket; // NON_COMPLIANT
62+
~FunctionalLang::Idris; // NON_COMPLIANT
63+
FunctionalLang::ML ^ FunctionalLang::OCaml; // NON_COMPLIANT
64+
FunctionalLang::OCaml >> 1; // NON_COMPLIANT
65+
FunctionalLang::Lisp << 1; // NON_COMPLIANT
66+
l &= FunctionalLang::OCaml; // NON_COMPLIANT
67+
l ^= 1; // NON_COMPLIANT
68+
l >>= 1; // NON_COMPLIANT
69+
l <<= 1; // NON_COMPLIANT
70+
FunctionalLang::FSharp &FunctionalLang::OCaml; // NON_COMPLIANT
7171
}
7272

7373
void test_enum_class_vars() {
@@ -79,7 +79,7 @@ void test_enum_class_vars() {
7979
a <= b; // COMPLIANT
8080
a > a; // COMPLIANT
8181
a >= a; // COMPLIANT
82-
a &b; // COMPLIANT
82+
FunctionalLang *c = &a; // COMPLIANT
8383

8484
// arithmetic
8585
a + 1; // NON_COMPLIANT
@@ -100,4 +100,21 @@ void test_enum_class_vars() {
100100
a ^ b; // NON_COMPLIANT
101101
a >> 1; // NON_COMPLIANT
102102
a << 1; // NON_COMPLIANT
103+
a &b; // NON_COMPLIANT
104+
}
105+
106+
enum class byte : unsigned char {};
107+
108+
byte operator&(byte lhs, byte rhs) { return lhs; }
109+
byte operator|(byte lhs, byte rhs) { return lhs; }
110+
byte operator^(byte lhs, byte rhs) { return lhs; }
111+
byte operator~(byte lhs) { return lhs; }
112+
byte operator&=(byte lhs, byte rhs) { return lhs; }
113+
byte operator|=(byte lhs, byte rhs) { return lhs; }
114+
115+
void test_bitmasktype_enum_class() { // COMPLIANT - byte implements the
116+
// BitmaskType trait.
117+
byte one, two;
118+
119+
one &two;
103120
}

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,3 +264,64 @@ class UserOverloadedOperator extends Function {
264264
not this.isCompilerGenerated()
265265
}
266266
}
267+
268+
private newtype TOperatorUse =
269+
TBuiltinOperatorUse(Operation op) or
270+
TOverloadedOperatorUse(FunctionCall call, Operator op) { op.getACallToThisFunction() = call }
271+
272+
/**
273+
* A class to reason about builtin operator and overloaded operator use.
274+
*/
275+
class OperatorUse extends TOperatorUse {
276+
string toString() {
277+
exists(Operation op | result = op.toString() and this = TBuiltinOperatorUse(op))
278+
or
279+
exists(Operator op | result = op.toString() and this = TOverloadedOperatorUse(_, op))
280+
}
281+
282+
predicate isOverloaded() { this = TOverloadedOperatorUse(_, _) }
283+
284+
Operation asBuiltin() { this = TBuiltinOperatorUse(result) }
285+
286+
Operator asOverloaded(FunctionCall call) { this = TOverloadedOperatorUse(call, result) }
287+
288+
Type getType() {
289+
result = this.asBuiltin().getType()
290+
or
291+
result = this.asOverloaded(_).getType()
292+
}
293+
294+
Parameter getParameter(int index) { result = this.asOverloaded(_).getParameter(index) }
295+
296+
Parameter getAParameter() { result = this.asOverloaded(_).getParameter(_) }
297+
298+
Expr getAnOperand() {
299+
result = this.asBuiltin().getAnOperand()
300+
or
301+
exists(FunctionCall call, Operator op | op = this.asOverloaded(call) |
302+
result = call.getAnArgument()
303+
)
304+
}
305+
306+
Location getLocation() {
307+
result = this.asBuiltin().getLocation()
308+
or
309+
exists(FunctionCall call, Operator op | op = this.asOverloaded(call) |
310+
result = call.getLocation()
311+
)
312+
}
313+
314+
string getOperator() {
315+
result = this.asBuiltin().getOperator()
316+
or
317+
result = this.asOverloaded(_).getName().regexpCapture("^operator(.*)$", 1)
318+
}
319+
}
320+
321+
class UnaryOperatorUse extends OperatorUse {
322+
UnaryOperatorUse() {
323+
this.asBuiltin() instanceof UnaryOperation
324+
or
325+
this.asOverloaded(_).getNumberOfParameters() = 0
326+
}
327+
}

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,34 @@ class IncompleteType extends Class {
2323
IncompleteType() { not hasDefinition() }
2424
}
2525

26+
/**
27+
* A type that implements the BitmaskType trait.
28+
* https://en.cppreference.com/w/cpp/named_req/BitmaskType
29+
*/
30+
abstract class BitmaskType extends Type { }
31+
32+
/**
33+
* Holds if `enum` implements required overload `overload` to implement
34+
* the BitmaskType trait.
35+
*/
36+
private predicate isRequiredEnumOverload(Enum enum, Function overload) {
37+
overload.getName().regexpMatch("operator([&|^~]|&=|\\|=)") and
38+
forex(Parameter p | p = overload.getAParameter() |
39+
(
40+
p.getType() = enum
41+
or
42+
p.getType().(ReferenceType).getBaseType() = enum
43+
)
44+
)
45+
}
46+
47+
private class EnumBitmaskType extends BitmaskType, Enum {
48+
EnumBitmaskType() {
49+
// Implements all the required overload
50+
count(Function overload | isRequiredEnumOverload(this, overload)) = 6
51+
}
52+
}
53+
2654
/**
2755
* A type without `const` and `volatile` specifiers.
2856
*/

0 commit comments

Comments
 (0)