Skip to content

Commit 47e82f7

Browse files
committed
Fix RULE-7-0-4 false positives for non-built-in operators
`InappropriateBitwiseOrShiftOperands.ql` reported operations that do not use the built-in bitwise or shift operators, most notably the stream insertion and extraction operators, e.g. `stream << value`. Resolved calls to a user provided operator are modelled as `FunctionCall`s and therefore never reach this query. The reported operations were instead the syntactic `LShiftExpr`/`RShiftExpr` nodes emitted for uninstantiated template bodies, where the operands are either unresolved (`unknown`, `auto &&`) or are the non-dependent result of an already resolved overload, such as `std::basic_ostream<char, std::char_traits<char>> &`. The built-in bitwise and shift operators are only applicable to integral and unscoped enumeration operands, so an operation with an operand of any other type does not use them. The query now requires both operands to be of such a type before reporting a violation. This does not introduce false negatives for dependent operands, because the template instantiations, in which the operand types are known, are still reported. Fixes #1177
1 parent 4d0376a commit 47e82f7

4 files changed

Lines changed: 90 additions & 1 deletion

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
- `RULE-7-0-4` - `InappropriateBitwiseOrShiftOperands.ql`:
2+
- Fixes #FP - the rule no longer reports operations that do not use the built-in bitwise or shift
3+
operators. The built-in operators are only applicable to integral and unscoped enumeration
4+
operands, so operations with operands of any other type either call a user-provided
5+
`operator`, such as the stream insertion and extraction operators, or are unresolved dependent
6+
expressions in an uninstantiated template body. The latter are still reported through the
7+
template instantiations, where the operand types are known.

cpp/misra/src/rules/RULE-7-0-4/InappropriateBitwiseOrShiftOperands.ql

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,46 @@ predicate isConstantExpression(Expr e) {
2222
e.isConstant()
2323
}
2424

25+
/**
26+
* Gets the type `t` with any top-level typedefs, cv-qualifiers and reference stripped.
27+
*
28+
* Note that `getUnspecifiedType()` does not strip references, so the base type of a reference
29+
* type has to be resolved explicitly.
30+
*/
31+
Type getStrippedOperandType(Type t) {
32+
exists(Type unspecified | unspecified = t.getUnspecifiedType() |
33+
if unspecified instanceof ReferenceType
34+
then result = unspecified.(ReferenceType).getBaseType().getUnspecifiedType()
35+
else result = unspecified
36+
)
37+
}
38+
39+
/**
40+
* Holds if a built-in bitwise or shift operator can be applied to an operand of type `t`, i.e. `t`
41+
* is an integral type or an unscoped enumeration type.
42+
*/
43+
predicate isBuiltInOperandType(Type t) {
44+
exists(Type stripped | stripped = getStrippedOperandType(t) |
45+
stripped instanceof IntegralType
46+
or
47+
stripped instanceof Enum and not stripped instanceof ScopedEnum
48+
)
49+
}
50+
51+
/**
52+
* Holds if the operation with operands `left` and `right` uses the built-in bitwise or shift
53+
* operator, rather than a user-provided overloaded operator.
54+
*
55+
* The built-in operators are only applicable to integral and unscoped enumeration operands. An
56+
* operation with an operand of any other type either calls a user-provided `operator`, or is an
57+
* unresolved dependent expression in an uninstantiated template body. The latter is reported
58+
* through the template instantiations instead, where the operand types are known.
59+
*/
60+
predicate isBuiltInOperation(Expr left, Expr right) {
61+
isBuiltInOperandType(left.getExplicitlyConverted().getType()) and
62+
isBuiltInOperandType(right.getExplicitlyConverted().getType())
63+
}
64+
2565
bindingset[right, leftType]
2666
pragma[inline_late]
2767
predicate isValidShiftConstantRange(Expr right, Type leftType) {
@@ -59,7 +99,8 @@ where
5999
(
60100
// Binary bitwise operators (excluding shift operations) - both operands must be unsigned
61101
exists(BinaryBitwiseOpOrAssignOp op, Type operandType |
62-
not op instanceof BinaryShiftOpOrAssignOp
102+
not op instanceof BinaryShiftOpOrAssignOp and
103+
isBuiltInOperation(op.getLeftOperand(), op.getRightOperand())
63104
|
64105
x = op.getLeftOperand() and
65106
operandType = op.getLeftOperand().getExplicitlyConverted().getType() and
@@ -82,6 +123,7 @@ where
82123
exists(ComplementExpr comp, Type opType |
83124
x = comp.getOperand() and
84125
opType = comp.getOperand().getExplicitlyConverted().getType() and
126+
isBuiltInOperandType(opType) and
85127
not MisraCpp23BuiltInTypes::isUnsignedType(opType) and
86128
message =
87129
"Bit complement operator '~' requires unsigned operand, but has type '" + opType + "'."
@@ -90,6 +132,7 @@ where
90132
// Shift operators - left operand must be unsigned
91133
exists(BinaryShiftOpOrAssignOp shift, Type leftType |
92134
x = shift.getLeftOperand() and
135+
isBuiltInOperation(shift.getLeftOperand(), shift.getRightOperand()) and
93136
leftType = shift.getLeftOperand().getExplicitlyConverted().getType() and
94137
not MisraCpp23BuiltInTypes::isUnsignedType(leftType) and
95138
not isSignedConstantLeftShiftException(shift) and
@@ -102,6 +145,7 @@ where
102145
exists(BinaryShiftOpOrAssignOp shift, Expr right, Type rightType, Type leftType |
103146
right = shift.getRightOperand() and
104147
x = right and
148+
isBuiltInOperation(shift.getLeftOperand(), right) and
105149
rightType = right.getExplicitlyConverted().getType() and
106150
leftType = shift.getLeftOperand().getExplicitlyConverted().getType()
107151
|

cpp/misra/test/rules/RULE-7-0-4/InappropriateBitwiseOrShiftOperands.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,4 @@
4747
| test.cpp:156:3:156:12 | 1073741824 | Shift operator '<<' requires unsigned left operand, but has type 'int'. |
4848
| test.cpp:162:3:162:5 | s32 | Shift operator '<<' requires unsigned left operand, but has type 'int32_t'. |
4949
| test.cpp:170:3:170:5 | s32 | Shift operator '>>' requires unsigned left operand, but has type 'int32_t'. |
50+
| test.cpp:201:3:201:7 | value | Shift operator '<<' requires unsigned left operand, but has type 'signed int'. |

cpp/misra/test/rules/RULE-7-0-4/test.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,41 @@ void test_right_shift_signed_operands() {
168168

169169
u32 >> 1U; // COMPLIANT
170170
s32 >> 1U; // NON_COMPLIANT
171+
}
172+
173+
class TestStream {
174+
public:
175+
TestStream &operator<<(std::int32_t value);
176+
TestStream &operator>>(std::int32_t &value);
177+
};
178+
179+
void test_overloaded_shift_operators() {
180+
TestStream stream;
181+
std::int32_t s32 = 1;
182+
183+
// User provided operators, not the built-in shift operators
184+
stream << 1; // COMPLIANT
185+
stream << s32; // COMPLIANT
186+
stream >> s32; // COMPLIANT
187+
}
188+
189+
template <typename T>
190+
void test_overloaded_shift_operators_in_template(TestStream &stream,
191+
const T &value) {
192+
// The left operand of the second `<<` is the `TestStream &` returned by the
193+
// first one, and the right operand is dependent, so the operation is
194+
// unresolved in the uninstantiated template body
195+
stream << 1 << value; // COMPLIANT
196+
}
197+
198+
template <typename T> void test_dependent_shift_operands(T value) {
199+
// Dependent operands are unresolved in the uninstantiated template body, but
200+
// reported through the instantiation below
201+
value << 2; // NON_COMPLIANT
202+
}
203+
204+
void test_template_instantiations() {
205+
TestStream stream;
206+
test_overloaded_shift_operators_in_template(stream, 1);
207+
test_dependent_shift_operands<std::int32_t>(1);
171208
}

0 commit comments

Comments
 (0)