From edd9071c275b42238337abf7c024b0116212840f Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 20 Sep 2024 10:52:13 +0100 Subject: [PATCH] Rule 7.2: Remove false positives in macros and implicit conversions - Remove false positives where integer constants are generated from macros. - Remove false positives where a signed integer is implicitly converted to unsigned, which is permitted by the standard. --- .../UOrUSuffixRepresentedInUnsignedType.ql | 13 +- ...rUSuffixRepresentedInUnsignedType.expected | 11 +- c/misra/test/rules/RULE-7-2/test.c | 266 +++++++++++++++--- change_notes/2024-09-20-fix-7-2-fps.md | 3 + rule_packages/c/Syntax.json | 5 +- 5 files changed, 252 insertions(+), 46 deletions(-) create mode 100644 change_notes/2024-09-20-fix-7-2-fps.md diff --git a/c/misra/src/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.ql b/c/misra/src/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.ql index b1dca9ac4a..b8f8d59718 100644 --- a/c/misra/src/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.ql +++ b/c/misra/src/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.ql @@ -19,6 +19,13 @@ from Literal l where not isExcluded(l, SyntaxPackage::uOrUSuffixRepresentedInUnsignedTypeQuery()) and not l instanceof StringLiteral and - l.getImplicitlyConverted().getType().(IntegralType).isUnsigned() and - not exists(l.getValueText().toUpperCase().indexOf("U")) -select l, "Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix." + // Determine if the extractor deduced that the literal is unsigned, based on the C rules + l.getType().(IntegralType).isUnsigned() and + // And report if the literal does not contain a 'U' or 'u' suffix, e.g. explicitly unsigned + not exists(l.getValueText().toUpperCase().indexOf("U")) and + // Exclude constants generated by macro expansions, because the suffix information is lost in this + // case, so can cause false positives. + not l.isInMacroExpansion() +select l, + "Unsigned literal " + l.getValueText() + + " does not explicitly express sign with a 'U' or 'u' suffix." diff --git a/c/misra/test/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.expected b/c/misra/test/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.expected index 4a131f4eaa..07cd56b3d9 100644 --- a/c/misra/test/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.expected +++ b/c/misra/test/rules/RULE-7-2/UOrUSuffixRepresentedInUnsignedType.expected @@ -1,5 +1,6 @@ -| test.c:8:20:8:21 | 0 | Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix. | -| test.c:9:20:9:22 | 0 | Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix. | -| test.c:33:6:33:6 | 1 | Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix. | -| test.c:35:6:35:9 | 1 | Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix. | -| test.c:37:6:37:8 | 1 | Unsigned literal does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:111:3:111:12 | 2147483648 | Unsigned literal 0x80000000 does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:116:3:116:20 | 9223372036854775808 | Unsigned literal 0x8000000000000000 does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:139:3:139:21 | 9223372036854775808 | Unsigned literal 0x8000000000000000l does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:162:3:162:21 | 9223372036854775808 | Unsigned literal 0x8000000000000000L does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:185:3:185:22 | 9223372036854775808 | Unsigned literal 0x8000000000000000ll does not explicitly express sign with a 'U' or 'u' suffix. | +| test.c:208:3:208:22 | 9223372036854775808 | Unsigned literal 0x8000000000000000LL does not explicitly express sign with a 'U' or 'u' suffix. | diff --git a/c/misra/test/rules/RULE-7-2/test.c b/c/misra/test/rules/RULE-7-2/test.c index da62825755..b95d2b1e02 100644 --- a/c/misra/test/rules/RULE-7-2/test.c +++ b/c/misra/test/rules/RULE-7-2/test.c @@ -1,39 +1,231 @@ +// Assumed platform in qltest is linux_x86_64, so +// int, long, long long sizes are assumed to be 32, 64, 64 bits respectively -long a1 = 0L; // COMPLIANT -long a2 = 0LL; // COMPLIANT -long a3 = 0uL; // COMPLIANT -long a4 = 0Lu; // COMPLIANT -long a5 = 0LU; // COMPLIANT - -unsigned long b1 = 0L; // NON_COMPLIANT -unsigned long b2 = 0LL; // NON_COMPLIANT -unsigned long b3 = 0uL; // COMPLIANT -unsigned long b4 = 0Lu; // COMPLIANT -unsigned long b5 = 0LU; // COMPLIANT - -signed long c1 = 0L; // COMPLIANT -signed long c2 = 0LL; // COMPLIANT -signed long c3 = 0uL; // COMPLIANT -signed long c4 = 0Lu; // COMPLIANT -signed long c5 = 0LU; // COMPLIANT - -void f0(int a) {} - -void f1(unsigned int a) {} - -void f2() { - - f0(1); // COMPLIANT - f0(1U); // COMPLIANT - f0(0x01); // COMPLIANT - f0(0x01U); // COMPLIANT - f0(001); // COMPLIANT - f0(001U); // COMPLIANT - - f1(1); // NON_COMPLIANT - f1(1U); // COMPLIANT - f1(0x01); // NON_COMPLIANT - f1(0x01U); // COMPLIANT - f1(001); // NON_COMPLIANT - f1(001U); // COMPLIANT +// The type of an integer constant is determined by "6.4.4.1 Integer constants" +// in the C11 Standard. The principle is that any decimal integer constant will +// be signed, unless it has the `U` or `u` suffix. Any hexadecimal integer will +// depend on whether it is larger than the maximum value of the smallest signed +// integer value that can hold the value. So the signedness depends on the +// magnitude of the constant. + +void test_decimal_constants() { + 0; // COMPLIANT + 2147483648; // COMPLIANT - larger than int, but decimal constants never use + // unsigned without the suffix, so will be `long` + 4294967296; // COMPLIANT - larger than unsigned int, still `long` + 9223372036854775807; // COMPLIANT - max long int + // 9223372036854775808; Not a valid integer constant, out of signed range + 0U; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648U; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296U; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807U; // COMPLIANT - max long int + 9223372036854775808U; // COMPLIANT - explicitly unsigned, so can go large than + // max long int + 0u; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648u; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296u; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807u; // COMPLIANT - max long int + 9223372036854775808u; // COMPLIANT - explicitly unsigned, so can go large than + // max long int + + // l suffix + 0l; // COMPLIANT + 2147483648l; // COMPLIANT - within the range of long int + 4294967296l; // COMPLIANT - within the range of long int + 9223372036854775807l; // COMPLIANT - max long int + // 9223372036854775808l; Not a valid integer constant, out of signed range + 0lU; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648lU; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296lU; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807lU; // COMPLIANT - max long int + 9223372036854775808lU; // COMPLIANT - explicitly unsigned, so can go large + // than max long int + 0lu; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648lu; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296lu; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807lu; // COMPLIANT - max long int + 9223372036854775808lu; // COMPLIANT - explicitly unsigned, so can go large + // than max long int + + // L suffix + 0L; // COMPLIANT + 2147483648L; // COMPLIANT - within the range of long int + 4294967296L; // COMPLIANT - within the range of long int + 9223372036854775807L; // COMPLIANT - max long int + // 9223372036854775808L; Not a valid integer constant, out of signed range + 0LU; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648LU; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296LU; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807LU; // COMPLIANT - max long int + 9223372036854775808LU; // COMPLIANT - explicitly unsigned, so can go large + // than max long int + 0Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807Lu; // COMPLIANT - max long int + 9223372036854775808Lu; // COMPLIANT - explicitly unsigned, so can go large + // than max long int + + // ll suffix + 0ll; // COMPLIANT + 2147483648ll; // COMPLIANT - within the range of long long int + 4294967296ll; // COMPLIANT - within the range of long long int + 9223372036854775807ll; // COMPLIANT - max long long int + // 9223372036854775808ll; Not a valid integer constant, out of signed range + 0llU; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648llU; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296llU; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807llU; // COMPLIANT - max long long int + 9223372036854775808llU; // COMPLIANT - explicitly unsigned, so can go large + // than max long long int + 0llu; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648llu; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296llu; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807llu; // COMPLIANT - max long long int + 9223372036854775808llu; // COMPLIANT - explicitly unsigned, so can go large + // than max long long int + + // LL suffix + 0LL; // COMPLIANT + 2147483648LL; // COMPLIANT - within the range of long long int + 4294967296LL; // COMPLIANT - within the range of long long int + 9223372036854775807LL; // COMPLIANT - max long long int + // 9223372036854775808LL; Not a valid integer constant, out of signed range + 0LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807LLU; // COMPLIANT - max long long int + 9223372036854775808LLU; // COMPLIANT - explicitly unsigned, so can go large + // than max long long int + 0LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 2147483648LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 4294967296LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 9223372036854775807LLu; // COMPLIANT - max long long int + 9223372036854775808LLu; // COMPLIANT - explicitly unsigned, so can go large + // than max long long int } + +void test_hexadecimal_constants() { + 0x0; // COMPLIANT - uses signed int + 0x7FFFFFFF; // COMPLIANT - max value held by signed int + 0x80000000; // NON_COMPLIANT - larger than max signed int, so will be unsigned + // int + 0x100000000; // COMPLIANT - larger than unsigned int, but smaller than long + // int + 0x7FFFFFFFFFFFFFFF; // COMPLIANT - max long int + 0x8000000000000000; // NON_COMPLIANT - larger than long int, so will be + // unsigned long int + 0x0U; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000U; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000U; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000U; // COMPLIANT - unsigned, but uses the suffix correctly + 0x0u; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000u; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000u; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000u; // COMPLIANT - unsigned, but uses the suffix correctly + + // Use of the `l` suffix + 0x0l; // COMPLIANT - uses signed int + 0x7FFFFFFFl; // COMPLIANT - max value held by signed int + 0x80000000l; // COMPLIANT - larger than max signed int, but smaller than long + // int + 0x100000000l; // COMPLIANT - larger than unsigned int, but smaller than long + // int + 0x7FFFFFFFFFFFFFFFl; // COMPLIANT - max long int + 0x8000000000000000l; // NON_COMPLIANT - larger than long int, so will be + // unsigned long int + 0x0lU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFlU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000lU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000lU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFlU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000lU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x0lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFlu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFlu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000lu; // COMPLIANT - unsigned, but uses the suffix correctly + + // Use of the `L` suffix + 0x0L; // COMPLIANT - uses signed int + 0x7FFFFFFFL; // COMPLIANT - max value held by signed int + 0x80000000L; // COMPLIANT - larger than max signed int, but smaller than long + // int + 0x100000000L; // COMPLIANT - larger than unsigned int, but smaller than long + // int + 0x7FFFFFFFFFFFFFFFL; // COMPLIANT - max long int + 0x8000000000000000L; // NON_COMPLIANT - larger than long int, so will be + // unsigned long int + 0x0LU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000LU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000LU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000LU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x0Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000Lu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000Lu; // COMPLIANT - unsigned, but uses the suffix correctly + + // Use of the `ll` suffix + 0x0ll; // COMPLIANT - uses signed int + 0x7FFFFFFFll; // COMPLIANT - max value held by signed int + 0x80000000ll; // COMPLIANT - larger than max signed int, but smaller than long + // long int + 0x100000000ll; // COMPLIANT - larger than unsigned int, but smaller than long + // long int + 0x7FFFFFFFFFFFFFFFll; // COMPLIANT - max long long int + 0x8000000000000000ll; // NON_COMPLIANT - larger than long long int, so will be + // unsigned long long int + 0x0llU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFllU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000llU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000llU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFllU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000llU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x0llu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFllu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000llu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000llu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFllu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000llu; // COMPLIANT - unsigned, but uses the suffix correctly + + // Use of the `LL` suffix + 0x0LL; // COMPLIANT - uses signed int + 0x7FFFFFFFLL; // COMPLIANT - max value held by signed int + 0x80000000LL; // COMPLIANT - larger than max signed int, but smaller than long + // long int + 0x100000000LL; // COMPLIANT - larger than unsigned int, but smaller than long + // long int + 0x7FFFFFFFFFFFFFFFLL; // COMPLIANT - max long long int + 0x8000000000000000LL; // NON_COMPLIANT - larger than long long int, so will be + // unsigned long long int + 0x0LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFLLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFLLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000LLU; // COMPLIANT - unsigned, but uses the suffix correctly + 0x0LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFLLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x80000000LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x100000000LLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x7FFFFFFFFFFFFFFFLLu; // COMPLIANT - unsigned, but uses the suffix correctly + 0x8000000000000000LLu; // COMPLIANT - unsigned, but uses the suffix correctly +} + +#define COMPLIANT_VAL 0x80000000U +#define NON_COMPLIANT_VAL 0x80000000 + +void test_macro() { + COMPLIANT_VAL; // COMPLIANT + NON_COMPLIANT_VAL; // NON_COMPLIANT[FALSE_NEGATIVE] - cannot determine suffix + // in macro expansions +} \ No newline at end of file diff --git a/change_notes/2024-09-20-fix-7-2-fps.md b/change_notes/2024-09-20-fix-7-2-fps.md new file mode 100644 index 0000000000..897aebadb7 --- /dev/null +++ b/change_notes/2024-09-20-fix-7-2-fps.md @@ -0,0 +1,3 @@ + - `RULE-7-2` - `UOrUSuffixRepresentedInUnsignedType.ql` + - Remove false positives where integer constants are generated from macros. + - Remove false positives where a signed integer is implicitly converted to unsigned, which is permitted by the standard. \ No newline at end of file diff --git a/rule_packages/c/Syntax.json b/rule_packages/c/Syntax.json index b8899ccc97..99bcf8250e 100644 --- a/rule_packages/c/Syntax.json +++ b/rule_packages/c/Syntax.json @@ -121,7 +121,10 @@ "tags": [ "maintainability", "readability" - ] + ], + "implementation_scope": { + "description": "This implementation does not consider constants defined in macro bodies." + } } ], "title": "A 'U' or 'u' suffix shall be applied to all integer constants that are represented in an unsigned type"