Skip to content

Commit a0f0606

Browse files
committed
Fix RULE-7-0-4 false positives for non-numeric operand types
`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`. The operand checks used `not MisraCpp23BuiltInTypes::isUnsignedType(type)`. That predicate takes a `NumericType`, so it does not hold for a type that has no MISRA numeric type at all, and negating it makes the check vacuously true for every such type. Class types and the unresolved dependent types of uninstantiated template bodies were therefore reported as non-compliant operands. The checks now use `MisraCpp23BuiltInTypes::isSignedType(type)` instead, which only holds for an operand that has a MISRA numeric type and is signed. This does not introduce false negatives for dependent operands, because the template instantiations, in which the operand types are known, are still reported. Operands of character type, of a non-standard integral type, and of an unscoped enumeration type without a fixed underlying type are consequently no longer reported, because none of them has a MISRA numeric type. Unscoped enumerations without a fixed underlying type are covered by RULE-10-2-3. Fixes #1177
1 parent 4d0376a commit a0f0606

4 files changed

Lines changed: 86 additions & 6 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
- `RULE-7-0-4` - `InappropriateBitwiseOrShiftOperands.ql`:
2+
- Fixes #1177 - the rule no longer reports operands whose type is not a MISRA numeric type. The
3+
operand checks used `not isUnsignedType(operandType)`, which is vacuously true for every type
4+
that has no MISRA numeric type at all, such as class types and the unresolved dependent types
5+
of uninstantiated template bodies. As a result the rule reported operations that do not use
6+
the built-in operators, most notably the stream insertion and extraction operators. The checks
7+
now use `isSignedType(operandType)` instead.
8+
- Operands of character type, of a non-standard integral type, and of an unscoped enumeration
9+
type without a fixed underlying type are consequently no longer reported, because none of them
10+
has a MISRA numeric type. Unscoped enumerations without a fixed underlying type are covered by
11+
`RULE-10-2-3`.

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,15 @@ where
6363
|
6464
x = op.getLeftOperand() and
6565
operandType = op.getLeftOperand().getExplicitlyConverted().getType() and
66-
not MisraCpp23BuiltInTypes::isUnsignedType(operandType) and
66+
MisraCpp23BuiltInTypes::isSignedType(operandType) and
6767
message =
6868
"Bitwise operator '" + op.getOperator() +
6969
"' requires unsigned numeric operands, but the left operand has type '" + operandType +
7070
"'."
7171
or
7272
x = op.getRightOperand() and
7373
operandType = op.getRightOperand().getExplicitlyConverted().getType() and
74-
not MisraCpp23BuiltInTypes::isUnsignedType(operandType) and
74+
MisraCpp23BuiltInTypes::isSignedType(operandType) and
7575
message =
7676
"Bitwise operator '" + op.getOperator() +
7777
"' requires unsigned numeric operands, but the right operand has type '" + operandType +
@@ -82,7 +82,7 @@ where
8282
exists(ComplementExpr comp, Type opType |
8383
x = comp.getOperand() and
8484
opType = comp.getOperand().getExplicitlyConverted().getType() and
85-
not MisraCpp23BuiltInTypes::isUnsignedType(opType) and
85+
MisraCpp23BuiltInTypes::isSignedType(opType) and
8686
message =
8787
"Bit complement operator '~' requires unsigned operand, but has type '" + opType + "'."
8888
)
@@ -91,7 +91,7 @@ where
9191
exists(BinaryShiftOpOrAssignOp shift, Type leftType |
9292
x = shift.getLeftOperand() and
9393
leftType = shift.getLeftOperand().getExplicitlyConverted().getType() and
94-
not MisraCpp23BuiltInTypes::isUnsignedType(leftType) and
94+
MisraCpp23BuiltInTypes::isSignedType(leftType) and
9595
not isSignedConstantLeftShiftException(shift) and
9696
message =
9797
"Shift operator '" + shift.getOperator() +
@@ -112,7 +112,7 @@ where
112112
"Shift operator '" + shift.getOperator() + "' shifts by " + right.getValue().toInt() +
113113
" which is not within the valid range 0.." + ((leftType.getSize() * 8) - 1) + "."
114114
else (
115-
not MisraCpp23BuiltInTypes::isUnsignedType(rightType) and
115+
MisraCpp23BuiltInTypes::isSignedType(rightType) and
116116
message =
117117
"Shift operator '" + shift.getOperator() +
118118
"' requires unsigned right operand, but has type '" + rightType + "'."

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,6 @@
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'. |
51+
| test.cpp:226:3:226:4 | e3 | Bitwise operator '&' requires unsigned numeric operands, but the left operand has type 'UnscopedEnumSignedUnderlyingType'. |
52+
| test.cpp:226:7:226:8 | e3 | Bitwise operator '&' requires unsigned numeric operands, but the right operand has type 'UnscopedEnumSignedUnderlyingType'. |

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

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,4 +168,70 @@ void test_right_shift_signed_operands() {
168168

169169
u32 >> 1U; // COMPLIANT
170170
s32 >> 1U; // NON_COMPLIANT
171-
}
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);
208+
}
209+
enum UnscopedEnumNoFixedUnderlyingType { EnumeratorA = 1, EnumeratorB = 2 };
210+
211+
enum UnscopedEnumUnsignedUnderlyingType : unsigned int { EnumeratorC = 1 };
212+
213+
enum UnscopedEnumSignedUnderlyingType : int { EnumeratorD = 1 };
214+
215+
void test_enum_operands() {
216+
UnscopedEnumNoFixedUnderlyingType e1 = EnumeratorA;
217+
UnscopedEnumUnsignedUnderlyingType e2 = EnumeratorC;
218+
UnscopedEnumSignedUnderlyingType e3 = EnumeratorD;
219+
220+
// Without a fixed underlying type the enum has no MISRA numeric type, so the
221+
// operands are not analysed by this rule
222+
e1 &e1; // COMPLIANT
223+
224+
e2 &e2; // COMPLIANT
225+
226+
e3 &e3; // NON_COMPLIANT
227+
}
228+
229+
void test_character_type_operands() {
230+
char32_t c32 = 1;
231+
232+
// `char32_t` is of character type, not of numeric type, and is always
233+
// unsigned
234+
c32 &c32; // COMPLIANT
235+
c32 << 1U; // COMPLIANT
236+
~c32; // COMPLIANT
237+
}

0 commit comments

Comments
 (0)