diff --git a/c/misra/src/codingstandards/c/misra/EssentialTypes.qll b/c/misra/src/codingstandards/c/misra/EssentialTypes.qll index a1b8d6fdb0..7df233d2ab 100644 --- a/c/misra/src/codingstandards/c/misra/EssentialTypes.qll +++ b/c/misra/src/codingstandards/c/misra/EssentialTypes.qll @@ -355,13 +355,17 @@ class EssentialLiteral extends EssentialExpr, Literal { else ( if this.(CharLiteral).getCharacter().length() = 1 then result instanceof PlainCharType - else ( - getStandardType().(IntegralType).isSigned() and - result = stlr(this) - or - not getStandardType().(IntegralType).isSigned() and - result = utlr(this) - ) + else + exists(Type underlyingStandardType | + underlyingStandardType = getStandardType().getUnderlyingType() + | + if underlyingStandardType instanceof IntType + then + if underlyingStandardType.(IntType).isSigned() + then result = stlr(this) + else result = utlr(this) + else result = underlyingStandardType + ) ) } } diff --git a/c/misra/src/rules/RULE-12-2/RightHandOperandOfAShiftRange.ql b/c/misra/src/rules/RULE-12-2/RightHandOperandOfAShiftRange.ql index 891ca1e82a..bd77bdacd2 100644 --- a/c/misra/src/rules/RULE-12-2/RightHandOperandOfAShiftRange.ql +++ b/c/misra/src/rules/RULE-12-2/RightHandOperandOfAShiftRange.ql @@ -20,14 +20,51 @@ class ShiftExpr extends BinaryBitwiseOperation { ShiftExpr() { this instanceof LShiftExpr or this instanceof RShiftExpr } } -from ShiftExpr e, Expr right, int max_val +MacroInvocation getAMacroInvocation(ShiftExpr se) { result.getAnExpandedElement() = se } + +Macro getPrimaryMacro(ShiftExpr se) { + exists(MacroInvocation mi | + mi = getAMacroInvocation(se) and + not exists(MacroInvocation otherMi | + otherMi = getAMacroInvocation(se) and otherMi.getParentInvocation() = mi + ) and + result = mi.getMacro() + ) +} + +from + ShiftExpr e, Expr right, int max_val, float lowerBound, float upperBound, Type essentialType, + string extraMessage, Locatable optionalPlaceholderLocation, string optionalPlaceholderMessage where not isExcluded(right, Contracts7Package::rightHandOperandOfAShiftRangeQuery()) and right = e.getRightOperand().getFullyConverted() and - max_val = (8 * getEssentialType(e.getLeftOperand()).getSize()) - 1 and + essentialType = getEssentialType(e.getLeftOperand()) and + max_val = (8 * essentialType.getSize()) - 1 and + upperBound = upperBound(right) and + lowerBound = lowerBound(right) and + ( + lowerBound < 0 or + upperBound > max_val + ) and + // If this shift happens inside a macro, then report the macro as well + // for easier validation ( - lowerBound(right) < 0 or - upperBound(right) > max_val + if exists(getPrimaryMacro(e)) + then + extraMessage = " from expansion of macro $@" and + exists(Macro m | + m = getPrimaryMacro(e) and + optionalPlaceholderLocation = m and + optionalPlaceholderMessage = m.getName() + ) + else ( + extraMessage = "" and + optionalPlaceholderLocation = e and + optionalPlaceholderMessage = "" + ) ) select right, - "The right hand operand of the shift operator shall lie in the range 0 to " + max_val + "." + "The possible range of the right operand of the shift operator (" + lowerBound + ".." + upperBound + + ") is outside the the valid shift range (0.." + max_val + + ") for the essential type of the left operand (" + essentialType + ")" + extraMessage + ".", + optionalPlaceholderLocation, optionalPlaceholderMessage diff --git a/c/misra/test/c/misra/EssentialTypes.expected b/c/misra/test/c/misra/EssentialTypes.expected index 8bf299bd63..d9245311da 100644 --- a/c/misra/test/c/misra/EssentialTypes.expected +++ b/c/misra/test/c/misra/EssentialTypes.expected @@ -38,3 +38,6 @@ | test.c:26:3:26:3 | f | float | float | essentially Floating type | | test.c:27:3:27:5 | f32 | float32_t | float32_t | essentially Floating type | | test.c:28:3:28:6 | cf32 | float | float | essentially Floating type | +| test.c:32:3:32:3 | 1 | signed char | signed char | essentially Signed type | +| test.c:33:3:33:4 | 1 | unsigned char | unsigned char | essentially Unsigned type | +| test.c:34:3:34:5 | 1 | unsigned long | unsigned long | essentially Unsigned type | diff --git a/c/misra/test/c/misra/test.c b/c/misra/test/c/misra/test.c index 8788f7e93a..d7064c4c12 100644 --- a/c/misra/test/c/misra/test.c +++ b/c/misra/test/c/misra/test.c @@ -26,4 +26,10 @@ void testCategoriesForComplexTypes() { f; // Should be essentially Floating type f32; // Should be essentially Floating type cf32; // Should be essentially Floating type +} + +void testConstants() { + 1; // Essentially signed char + 1U; // Essentially unsigned char + 1UL; // Essentially unsigned long } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-12-2/RightHandOperandOfAShiftRange.expected b/c/misra/test/rules/RULE-12-2/RightHandOperandOfAShiftRange.expected index a4deb83a14..5ac6f8bfd4 100644 --- a/c/misra/test/rules/RULE-12-2/RightHandOperandOfAShiftRange.expected +++ b/c/misra/test/rules/RULE-12-2/RightHandOperandOfAShiftRange.expected @@ -1,10 +1,12 @@ -| test.c:8:10:8:10 | 8 | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:9:10:9:11 | - ... | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:10:10:10:14 | ... + ... | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:11:10:11:14 | ... + ... | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:13:21:13:22 | 16 | The right hand operand of the shift operator shall lie in the range 0 to 15. | -| test.c:16:9:16:9 | 8 | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:21:9:21:10 | 64 | The right hand operand of the shift operator shall lie in the range 0 to 63. | -| test.c:25:10:25:10 | 8 | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:26:10:26:11 | 64 | The right hand operand of the shift operator shall lie in the range 0 to 7. | -| test.c:30:16:30:17 | 64 | The right hand operand of the shift operator shall lie in the range 0 to 63. | +| test.c:8:10:8:10 | 8 | The possible range of the right operand of the shift operator (8..8) is outside the the valid shift range (0..7) for the essential type of the left operand (uint8_t). | test.c:8:3:8:10 | ... >> ... | | +| test.c:9:10:9:11 | - ... | The possible range of the right operand of the shift operator (-1..-1) is outside the the valid shift range (0..7) for the essential type of the left operand (uint8_t). | test.c:9:3:9:11 | ... >> ... | | +| test.c:10:10:10:14 | ... + ... | The possible range of the right operand of the shift operator (8..8) is outside the the valid shift range (0..7) for the essential type of the left operand (uint8_t). | test.c:10:3:10:14 | ... >> ... | | +| test.c:11:10:11:14 | ... + ... | The possible range of the right operand of the shift operator (8..8) is outside the the valid shift range (0..7) for the essential type of the left operand (uint8_t). | test.c:11:3:11:14 | ... << ... | | +| test.c:13:21:13:22 | 16 | The possible range of the right operand of the shift operator (16..16) is outside the the valid shift range (0..15) for the essential type of the left operand (uint16_t). | test.c:13:3:13:22 | ... << ... | | +| test.c:16:9:16:9 | 8 | The possible range of the right operand of the shift operator (8..8) is outside the the valid shift range (0..7) for the essential type of the left operand (unsigned char). | test.c:16:3:16:9 | ... << ... | | +| test.c:21:9:21:10 | 64 | The possible range of the right operand of the shift operator (64..64) is outside the the valid shift range (0..63) for the essential type of the left operand (unsigned long). | test.c:21:3:21:10 | ... << ... | | +| test.c:26:10:26:11 | 64 | The possible range of the right operand of the shift operator (64..64) is outside the the valid shift range (0..63) for the essential type of the left operand (unsigned long). | test.c:26:3:26:11 | ... << ... | | +| test.c:30:16:30:17 | 64 | The possible range of the right operand of the shift operator (64..64) is outside the the valid shift range (0..63) for the essential type of the left operand (unsigned long). | test.c:30:3:30:17 | ... << ... | | +| test.c:34:8:34:8 | y | The possible range of the right operand of the shift operator (0..4294967295) is outside the the valid shift range (0..31) for the essential type of the left operand (unsigned int). | test.c:34:3:34:8 | ... >> ... | | +| test.c:40:8:40:8 | y | The possible range of the right operand of the shift operator (-2147483648..2147483647) is outside the the valid shift range (0..31) for the essential type of the left operand (signed int). | test.c:40:3:40:8 | ... >> ... | | +| test.c:42:8:42:8 | y | The possible range of the right operand of the shift operator (-31..31) is outside the the valid shift range (0..31) for the essential type of the left operand (signed int). | test.c:42:3:42:8 | ... >> ... | | diff --git a/c/misra/test/rules/RULE-12-2/test.c b/c/misra/test/rules/RULE-12-2/test.c index 449a47b7ae..db7b7b062d 100644 --- a/c/misra/test/rules/RULE-12-2/test.c +++ b/c/misra/test/rules/RULE-12-2/test.c @@ -29,3 +29,15 @@ void f1() { ULONG_MAX << 8; // COMPLIANT ULONG_MAX << 64; // NON_COMPLIANT } + +void unsignedRemAssign(unsigned int y, unsigned int x) { + x >> y; // NON_COMPLIANT + y %= 32; + x >> y; // COMPLIANT +} + +void signedRemAssign(signed int y, signed int x) { + x >> y; // NON_COMPLIANT + y %= 32; + x >> y; // NON_COMPLIANT - may be negative +} \ No newline at end of file diff --git a/change_notes/2024-09-11-rule-12-2-improvements.md b/change_notes/2024-09-11-rule-12-2-improvements.md new file mode 100644 index 0000000000..0e713a5088 --- /dev/null +++ b/change_notes/2024-09-11-rule-12-2-improvements.md @@ -0,0 +1,6 @@ +- `RULE-12-2` - `RightHandOperandOfAShiftRange.ql`: + - Reduce false positives related to ranges determined by `%=`. + - Reduce false positives for integer constants with explicit size suffix were incorrectly identified as smaller types. + - Improve explanation of results, providing additional information on types and size ranges. + - Combine results stemming from the expansion of a macro, where the result is not dependent on the context. + \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/SimpleRangeAnalysisCustomizations.qll b/cpp/common/src/codingstandards/cpp/SimpleRangeAnalysisCustomizations.qll index 469fe9a738..5144f63dc2 100644 --- a/cpp/common/src/codingstandards/cpp/SimpleRangeAnalysisCustomizations.qll +++ b/cpp/common/src/codingstandards/cpp/SimpleRangeAnalysisCustomizations.qll @@ -151,6 +151,40 @@ private class CastEnumToIntegerSimpleRange extends SimpleRangeAnalysisExpr, Cast override predicate dependsOnChild(Expr child) { child = getExpr() } } +/** + * A range analysis extension that supports `%=`. + */ +private class RemAssignSimpleRange extends SimpleRangeAnalysisExpr, AssignRemExpr { + override float getLowerBounds() { + exists(float maxDivisorNegated, float dividendLowerBounds | + // Find the max divisor, negated e.g. `%= 32` would be `-31` + maxDivisorNegated = (getFullyConvertedUpperBounds(getRValue()).abs() - 1) * -1 and + // Find the lower bounds of the dividend + dividendLowerBounds = getFullyConvertedLowerBounds(getLValue()) and + // The lower bound is calculated in two steps: + // 1. Determine the maximum of the dividend lower bound and maxDivisorNegated. + // When the dividend is negative this will result in a negative result + // 2. Find the minimum with 0. If the divided is always >0 this will produce 0 + // otherwise it will produce the lowest negative number that can be held + // after the modulo. + result = 0.minimum(dividendLowerBounds.maximum(maxDivisorNegated)) + ) + } + + override float getUpperBounds() { + exists(float maxDivisor, float maxDividend | + // The maximum divisor value is the absolute value of the divisor minus 1 + maxDivisor = getFullyConvertedUpperBounds(getRValue()).abs() - 1 and + // value if > 0 otherwise 0 + maxDividend = getFullyConvertedUpperBounds(getLValue()).maximum(0) and + // In the case the numerator is definitely less than zero, the result could be negative + result = maxDividend.minimum(maxDivisor) + ) + } + + override predicate dependsOnChild(Expr expr) { expr = getAChild() } +} + /** * functions that read a character from the STDIN, * or return EOF if it fails to do so.