diff --git a/amendments.csv b/amendments.csv new file mode 100644 index 0000000000..ae6c85e7d1 --- /dev/null +++ b/amendments.csv @@ -0,0 +1,50 @@ +language,standard,amendment,rule_id,queryable,implementation_category,implemented,difficulty +c,misra-c-2012,Amendment3,DIR-4-6,Yes,Expand,No,Easy +c,misra-c-2012,Amendment3,DIR-4-9,Yes,Refine,No,Easy +c,misra-c-2012,Amendment3,DIR-4-11,Yes,Refine,No,Import +c,misra-c-2012,Amendment3,RULE-1-4,Yes,Replace,No,Easy +c,misra-c-2012,Amendment3,RULE-10-1,Yes,Replace,No,Easy +c,misra-c-2012,Amendment3,RULE-10-3,Yes,Refine,No,Easy +c,misra-c-2012,Amendment3,RULE-10-4,Yes,Refine,No,Import +c,misra-c-2012,Amendment3,RULE-10-5,Yes,Expand,No,Easy +c,misra-c-2012,Amendment3,RULE-10-7,Yes,Refine,No,Import +c,misra-c-2012,Amendment3,RULE-10-8,Yes,Refine,No,Import +c,misra-c-2012,Amendment3,RULE-21-11,Yes,Clarification,No,Import +c,misra-c-2012,Amendment3,RULE-21-12,Yes,Replace,No,Easy +c,misra-c-2012,Amendment4,RULE-11-3,Yes,Expand,No,Easy +c,misra-c-2012,Amendment4,RULE-11-8,Yes,Expand,No,Easy +c,misra-c-2012,Amendment4,RULE-13-2,Yes,Expand,No,Very Hard +c,misra-c-2012,Amendment4,RULE-18-6,Yes,Expand,No,Medium +c,misra-c-2012,Amendment4,RULE-18-8,Yes,Split,No,Easy +c,misra-c-2012,Corrigendum2,RULE-2-2,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-2-7,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-3-1,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-8-6,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-8-9,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-9-4,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-10-1,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-18-3,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-1-4,Yes,Replace,No,Easy +c,misra-c-2012,Corrigendum2,RULE-9-1,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-9-2,Yes,Refine,No,Import +c,misra-c-2012,Corrigendum2,DIR-4-10,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-7-4,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-8-2,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-8-3,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-8-7,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-10-1,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-10-2,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-10-3,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-11-3,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-11-6,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-13-2,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-13-6,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-14-3,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-15-7,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-17-4,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-17-5,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-18-1,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-20-14,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-21-19,Yes,Clarification,No,Import +c,misra-c-2012,Corrigendum2,RULE-21-20,Yes,Refine,No,Easy +c,misra-c-2012,Corrigendum2,RULE-22-9,Yes,Clarification,No,Import \ No newline at end of file diff --git a/c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql b/c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql index fa4da7e358..aaa64fc3c0 100644 --- a/c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql +++ b/c/misra/src/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.ql @@ -13,14 +13,73 @@ import cpp import codingstandards.c.misra +import codingstandards.cpp.Macro import codingstandards.cpp.Pointers -from CStyleCast cast, Type typeFrom, Type typeTo +MacroInvocation getAMacroInvocation(CStyleCast cast) { result.getAnExpandedElement() = cast } + +Macro getPrimaryMacro(CStyleCast cast) { + exists(MacroInvocation mi | + mi = getAMacroInvocation(cast) and + not exists(MacroInvocation otherMi | + otherMi = getAMacroInvocation(cast) and otherMi.getParentInvocation() = mi + ) and + result = mi.getMacro() + ) +} + +Macro getNonFunctionPrimaryMacro(CStyleCast cast) { + result = getPrimaryMacro(cast) and + not result instanceof FunctionLikeMacro +} + +from + Locatable primaryLocation, CStyleCast cast, Type typeFrom, Type typeTo, string message, + string extraMessage, Locatable optionalPlaceholderLocation, string optionalPlaceholderMessage where not isExcluded(cast, Pointers1Package::conversionBetweenPointerToObjectAndIntegerTypeQuery()) and typeFrom = cast.getExpr().getUnderlyingType() and typeTo = cast.getUnderlyingType() and - [typeFrom, typeTo] instanceof IntegralType and - [typeFrom, typeTo] instanceof PointerToObjectType and - not isNullPointerConstant(cast.getExpr()) -select cast, "Cast performed between a pointer to object type and a pointer to an integer type." + ( + typeFrom instanceof PointerToObjectType and + typeTo instanceof IntegralType and + message = + "Cast from pointer to object type '" + typeFrom + "' to integer type '" + typeTo + "'" + + extraMessage + "." + or + typeFrom instanceof IntegralType and + typeTo instanceof PointerToObjectType and + message = + "Cast from integer type '" + typeFrom + "' to pointer to object type '" + typeTo + "'" + + extraMessage + "." + ) and + not isNullPointerConstant(cast.getExpr()) and + // If this alert is arising through a non-function-like macro expansion, flag the macro instead, to + // help make the alerts more manageable. We only do this for non-function-like macros because they + // cannot be context specific. + if exists(getNonFunctionPrimaryMacro(cast)) + then + primaryLocation = getNonFunctionPrimaryMacro(cast) and + extraMessage = "" and + optionalPlaceholderLocation = primaryLocation and + optionalPlaceholderMessage = "" + else ( + primaryLocation = cast and + // If the cast is in a macro expansion which is context specific, we still report the original + // location, but also add a link to the most specific macro that contains the cast, to aid + // validation. + if exists(getPrimaryMacro(cast)) + then + extraMessage = " from expansion of macro $@" and + exists(Macro m | + m = getPrimaryMacro(cast) and + optionalPlaceholderLocation = m and + optionalPlaceholderMessage = m.getName() + ) + else ( + extraMessage = "" and + optionalPlaceholderLocation = cast and + optionalPlaceholderMessage = "" + ) + ) +select primaryLocation, message, optionalPlaceholderLocation, optionalPlaceholderMessage diff --git a/c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql b/c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql index 987d8a32bb..de75e9d37a 100644 --- a/c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql +++ b/c/misra/src/rules/RULE-11-6/CastBetweenPointerToVoidAndArithmeticType.ql @@ -22,5 +22,5 @@ where typeTo = cast.getUnderlyingType() and [typeFrom, typeTo] instanceof ArithmeticType and [typeFrom, typeTo] instanceof VoidPointerType and - not isNullPointerConstant(cast.getExpr()) + not cast.getExpr() instanceof Zero select cast, "Cast performed between a pointer to void type and an arithmetic type." diff --git a/c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql b/c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql index b002ceb4c2..a5c34fb747 100644 --- a/c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql +++ b/c/misra/src/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.ql @@ -18,15 +18,25 @@ import codingstandards.cpp.Type from Zero zero, Expr e, string type where not isExcluded(zero, Pointers1Package::macroNullNotUsedAsIntegerNullPointerConstantQuery()) and - // exclude the base-case (NULL macros and void pointer casts) - not isNullPointerConstant(zero) and + // Exclude the base-case (NULL macros and void pointer casts) + // Note: we cannot use the isNullPointerConstant predicate here because it permits + // the use of `0` without casting, which is prohibited here. + not ( + zero.findRootCause() instanceof NullMacro + or + // integer constant `0` explicitly cast to void pointer + exists(Conversion c | c = zero.getConversion() | + not c.isImplicit() and + c.getUnderlyingType() instanceof VoidPointerType + ) + ) and ( // ?: operator exists(ConditionalExpr parent | ( - parent.getThen().getAChild*() = zero and parent.getElse().getType() instanceof PointerType + parent.getThen() = zero and parent.getElse().getType() instanceof PointerType or - parent.getElse().getAChild*() = zero and parent.getThen().getType() instanceof PointerType + parent.getElse() = zero and parent.getThen().getType() instanceof PointerType ) and // exclude a common conditional pattern used in macros such as 'assert' not parent.isInMacroExpansion() and diff --git a/c/misra/src/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql b/c/misra/src/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql new file mode 100644 index 0000000000..5fcf938046 --- /dev/null +++ b/c/misra/src/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql @@ -0,0 +1,23 @@ +/** + * @id c/misra/bit-field-declared-as-member-of-a-union + * @name RULE-6-3: A bit field shall not be declared as a member of a union + * @description Type punning on a union with bit fields relies on implementation-specific alignment + * behavior. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-6-3 + * correctness + * external/misra/obligation/required + */ + +import cpp +import codingstandards.c.misra + +from BitField field, Union u +where + not isExcluded(field, BitfieldTypes2Package::bitFieldDeclaredAsMemberOfAUnionQuery()) and + u.getAField() = field +select field, + "Union member " + field.getName() + + " is declared as a bit field which relies on implementation-specific behavior." diff --git a/c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected b/c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected index ebe2c74742..0144180616 100644 --- a/c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected +++ b/c/misra/test/rules/RULE-11-1/ConversionBetweenFunctionPointerAndOtherType.expected @@ -1,7 +1,6 @@ | test.c:11:8:11:16 | (fp1 *)... | Cast performed between a function pointer and another type. | | test.c:11:8:11:16 | (fp1)... | Cast performed between a function pointer and another type. | | test.c:12:14:12:23 | (void *)... | Cast performed between a function pointer and another type. | -| test.c:14:8:14:15 | (fp2)... | Cast performed between a function pointer and another type. | | test.c:15:8:15:15 | (fp2)... | Cast performed between a function pointer and another type. | | test.c:22:12:22:13 | (fp1)... | Cast performed between a function pointer and another type. | | test.c:25:8:25:9 | (fp1)... | Cast performed between a function pointer and another type. | diff --git a/c/misra/test/rules/RULE-11-1/test.c b/c/misra/test/rules/RULE-11-1/test.c index 858c6e68a9..4fcabb0599 100644 --- a/c/misra/test/rules/RULE-11-1/test.c +++ b/c/misra/test/rules/RULE-11-1/test.c @@ -11,7 +11,7 @@ void f1(void) { v1 = (fp1 *)v2; // NON_COMPLIANT void *v3 = (void *)v1; // NON_COMPLIANT - v2 = (fp2 *)0; // NON_COMPLIANT + v2 = (fp2 *)0; // COMPLIANT - null pointer constant v2 = (fp2 *)1; // NON_COMPLIANT pfp2 v4; diff --git a/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected b/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected index 5fedfdcce4..44d5ca5943 100644 --- a/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected +++ b/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected @@ -1,6 +1,6 @@ -| test.c:5:21:5:42 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. | -| test.c:5:35:5:42 | (int *)... | Cast performed between a pointer to object type and a pointer to an integer type. | -| test.c:6:21:6:37 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. | -| test.c:8:8:8:24 | (unsigned int)... | Cast performed between a pointer to object type and a pointer to an integer type. | -| test.c:10:22:10:22 | (unsigned int *)... | Cast performed between a pointer to object type and a pointer to an integer type. | -| test.c:12:22:12:39 | (unsigned int *)... | Cast performed between a pointer to object type and a pointer to an integer type. | +| test.c:6:21:6:37 | (unsigned int)... | Cast from pointer to object type 'unsigned int *' to integer type 'unsigned int'. | test.c:6:21:6:37 | (unsigned int)... | | +| test.c:8:8:8:24 | (unsigned int)... | Cast from pointer to object type 'unsigned int *' to integer type 'unsigned int'. | test.c:8:8:8:24 | (unsigned int)... | | +| test.c:12:22:12:39 | (unsigned int *)... | Cast from integer type 'unsigned int' to pointer to object type 'unsigned int *'. | test.c:12:22:12:39 | (unsigned int *)... | | +| test.c:15:1:15:24 | #define FOO (int *)0x200 | Cast from integer type 'int' to pointer to object type 'int *'. | test.c:15:1:15:24 | #define FOO (int *)0x200 | | +| test.c:23:3:23:22 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:17:1:17:34 | #define FOO_FUNCTIONAL(x) (int *)x | FOO_FUNCTIONAL | +| test.c:24:14:24:25 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:18:1:18:23 | #define FOO_INSERT(x) x | FOO_INSERT | diff --git a/c/misra/test/rules/RULE-11-4/test.c b/c/misra/test/rules/RULE-11-4/test.c index 25e3f3c4b2..5a78387247 100644 --- a/c/misra/test/rules/RULE-11-4/test.c +++ b/c/misra/test/rules/RULE-11-4/test.c @@ -2,12 +2,24 @@ void f1(void) { unsigned int v1 = (unsigned int)(void *)0; // COMPLIANT - unsigned int v2 = (unsigned int)(int *)0; // NON_COMPLIANT + unsigned int v2 = (unsigned int)(int *)0; // COMPLIANT unsigned int v3 = (unsigned int)&v2; // NON_COMPLIANT v3 = v2; // COMPLIANT v3 = (unsigned int)&v2; // NON_COMPLIANT v3 = NULL; // COMPLIANT - unsigned int *v4 = 0; // NON_COMPLIANT + unsigned int *v4 = 0; // COMPLIANT unsigned int *v5 = NULL; // COMPLIANT unsigned int *v6 = (unsigned int *)v2; // NON_COMPLIANT +} + +#define FOO (int *)0x200 // NON_COMPLIANT +#define FOO_WRAPPER FOO; +#define FOO_FUNCTIONAL(x) (int *)x +#define FOO_INSERT(x) x + +void test_macros() { + FOO; // Issue is reported at the macro + FOO_WRAPPER; // Issue is reported at the macro + FOO_FUNCTIONAL(0x200); // NON_COMPLIANT + FOO_INSERT((int *)0x200); // NON_COMPLIANT } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.expected b/c/misra/test/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.expected index 8cdd34edd1..d854730296 100644 --- a/c/misra/test/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.expected +++ b/c/misra/test/rules/RULE-11-9/MacroNullNotUsedAsIntegerNullPointerConstant.expected @@ -1,4 +1,5 @@ | test.c:15:13:15:13 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:15:7:15:13 | ... == ... | Equality operator | | test.c:17:8:17:8 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:17:3:17:8 | ... = ... | Assignment to pointer | -| test.c:25:20:25:20 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:25:3:25:35 | ... ? ... : ... | Ternary operator | -| test.c:25:20:25:20 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:25:15:25:20 | ... = ... | Assignment to pointer | +| test.c:23:13:23:13 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:23:3:23:13 | ... ? ... : ... | Ternary operator | +| test.c:24:8:24:8 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:24:3:24:13 | ... ? ... : ... | Ternary operator | +| test.c:31:14:31:14 | 0 | $@ uses zero-value integer constant expression as null pointer constant. | test.c:31:9:31:14 | ... = ... | Assignment to pointer | diff --git a/c/misra/test/rules/RULE-11-9/test.c b/c/misra/test/rules/RULE-11-9/test.c index 216ea2b280..e87366d831 100644 --- a/c/misra/test/rules/RULE-11-9/test.c +++ b/c/misra/test/rules/RULE-11-9/test.c @@ -19,9 +19,16 @@ void *f1(void *p1, int p2) { p1 = NULL; // COMPLIANT if (p2 == 0) { // COMPLIANT return NULL; - } // COMPLIANT - (p1) ? (p1 = NULL) : (p1 = NULL); // COMPLIANT - (p2 > 0) ? (p1 = NULL) : (p1 = NULL); // COMPLIANT - (p2 > 0) ? (p1 = 0) : (p1 = NULL); // NON_COMPLIANT - return 0; // COMPLIANT + } + p2 ? p1 : 0; // NON_COMPLIANT + p2 ? 0 : p1; // NON_COMPLIANT + p2 ? (void *)0 : p1; // COMPLIANT + p2 ? p1 : (void *)0; // COMPLIANT + p2 ? p2 : 0; // COMPLIANT - p2 is not a pointer type + p2 ? 0 : p2; // COMPLIANT - p2 is not a pointer type + int x; + int *y; + p2 ? (p1 = 0) : p1; // NON_COMPLIANT - p1 is a pointer type + p2 ? (p2 = 0) : p1; // COMPLIANT - p2 is not a pointer type + return 0; // COMPLIANT } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.expected b/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.expected new file mode 100644 index 0000000000..7c39484796 --- /dev/null +++ b/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.expected @@ -0,0 +1,2 @@ +| test.c:7:7:7:7 | x | Union member x is declared as a bit field which relies on implementation-specific behavior. | +| test.c:20:7:20:7 | (unnamed bitfield) | Union member (unnamed bitfield) is declared as a bit field which relies on implementation-specific behavior. | diff --git a/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.qlref b/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.qlref new file mode 100644 index 0000000000..21c43d4826 --- /dev/null +++ b/c/misra/test/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.qlref @@ -0,0 +1 @@ +rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql \ No newline at end of file diff --git a/c/misra/test/rules/RULE-6-3/test.c b/c/misra/test/rules/RULE-6-3/test.c new file mode 100644 index 0000000000..1de648d294 --- /dev/null +++ b/c/misra/test/rules/RULE-6-3/test.c @@ -0,0 +1,21 @@ +union U1 { + int x; // COMPLIANT + char y; // COMPLIANT +}; + +union U2 { + int x : 2; // NON-COMPLIANT + char y; // COMPLIANT +}; + +union U3 { + struct str { + int x : 4; // COMPLIANT + int y : 2; // COMPLIANT + }; +}; + +union U4 { + char x; + int : 0; // NON-COMPLIANT +}; \ No newline at end of file diff --git a/change_notes/2023-07-28-rule-11-4-improvements.md b/change_notes/2023-07-28-rule-11-4-improvements.md new file mode 100644 index 0000000000..3c385359a8 --- /dev/null +++ b/change_notes/2023-07-28-rule-11-4-improvements.md @@ -0,0 +1,12 @@ + - `RULE-11-1` - `ConversionBetweenFunctionPointerAndOtherType.ql`: + - Fixed issue #331 - consider `0` a null pointer constant. + - `RULE-11-4` - `ConversionBetweenPointerToObjectAndIntegerType.ql`: + - Fixed issue #331 - consider `0` a null pointer constant. + - Improve reporting of the order of the cast and the actual types involved. + - Improve reporting where the result is expanded from a macro by either reporting the macro itself (if it is not dependent on the context) or by including a link to the macro in the alert message. + - `RULE-11-5` - `ConversionFromPointerToVoidIntoPointerToObject.ql`: + - Fixed issue #331 - consider `0` a null pointer constant. + - `RULE-11-6` - `CastBetweenPointerToVoidAndArithmeticType.ql`: + - Fixed issue #331 - accept integer constant expressions with value `0` instead of null pointer constants. + - `RULE-11-9` - `MacroNullNotUsedAsIntegerNullPointerConstant.ql`: + - Remove false positives in branches of ternary expressions, where `0` was used correctly. \ No newline at end of file diff --git a/change_notes/2024-09-19-fix-fp-665-M3-4-1.md b/change_notes/2024-09-19-fix-fp-665-M3-4-1.md new file mode 100644 index 0000000000..63c5f91b56 --- /dev/null +++ b/change_notes/2024-09-19-fix-fp-665-M3-4-1.md @@ -0,0 +1,2 @@ +- `M3-4-1` - `UnnecessaryExposedIdentifierDeclarationShared.qll`: + - Fixes #665. Exclude variables that are constexpr and coming from template instantiations. diff --git a/change_notes/2024-9-20-a1-1-2-improvements.md b/change_notes/2024-9-20-a1-1-2-improvements.md new file mode 100644 index 0000000000..25e393954b --- /dev/null +++ b/change_notes/2024-9-20-a1-1-2-improvements.md @@ -0,0 +1,2 @@ +- `A1-1-2` - `CompilerWarningLevelNotInCompliance.ql`: + - Report non-compliance for compilations that use the error-suppressing `-w` flag. diff --git a/cpp/autosar/src/rules/A1-1-2/CompilerWarningLevelNotInCompliance.ql b/cpp/autosar/src/rules/A1-1-2/CompilerWarningLevelNotInCompliance.ql index bd98ad9162..60efab251a 100644 --- a/cpp/autosar/src/rules/A1-1-2/CompilerWarningLevelNotInCompliance.ql +++ b/cpp/autosar/src/rules/A1-1-2/CompilerWarningLevelNotInCompliance.ql @@ -18,15 +18,17 @@ import cpp import codingstandards.cpp.autosar -predicate hasResponseFileArgument(Compilation c) { c.getAnArgument().matches("@%") } +class CompilationWithNoWarnings extends Compilation { + CompilationWithNoWarnings() { + getAnArgument() = "-w" or + not getAnArgument().regexpMatch("-W[\\w=-]+") + } +} -predicate hasWarningOption(Compilation c) { c.getAnArgument().regexpMatch("-W[\\w=-]+") } +predicate hasResponseFileArgument(Compilation c) { c.getAnArgument().matches("@%") } from File f where not isExcluded(f, ToolchainPackage::compilerWarningLevelNotInComplianceQuery()) and - exists(Compilation c | f = c.getAFileCompiled() | - not hasResponseFileArgument(c) and - not hasWarningOption(c) - ) + exists(CompilationWithNoWarnings c | f = c.getAFileCompiled() | not hasResponseFileArgument(c)) select f, "No warning-level options were used in the compilation of '" + f.getBaseName() + "'." diff --git a/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected b/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected index e69de29bb2..81a5c4327e 100644 --- a/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected +++ b/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected @@ -0,0 +1 @@ +| Wcast-function-type.cpp:0:0:0:0 | Wcast-function-type.cpp | No warning-level options were used in the compilation of 'Wcast-function-type.cpp'. | diff --git a/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.clang b/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.clang new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.gcc b/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.gcc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.qcc b/cpp/autosar/test/rules/A1-1-2.2/CompilerWarningLevelNotInCompliance.expected.qcc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2.2/Wcast-function-type.cpp b/cpp/autosar/test/rules/A1-1-2.2/Wcast-function-type.cpp index f405349bbb..bc48268931 100644 --- a/cpp/autosar/test/rules/A1-1-2.2/Wcast-function-type.cpp +++ b/cpp/autosar/test/rules/A1-1-2.2/Wcast-function-type.cpp @@ -1,2 +1,14 @@ // semmle-extractor-options: --clang -std=c++14 -Wcast-function-type -// COMPLIANT \ No newline at end of file +// COMPLIANT + +// NOTE: When tested with `codeql test run`, the test extractor provides `-w` +// which overrides `-Wcast-function-type` and causes this test case to be +// non-compliant. +// +// However, when tested with our compiler matrix tests, this test db is built +// via `codeql database create --command="..."`, and the `-w` flag will NOT be +// used. This means the `-Wcast-function-type` flag is active and the test case +// is compliant. +// +// Therefore, the .expected file for this test expects non-compliance, and the +// .expected.gcc and .expected.clang files expect this test to be compliant. diff --git a/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected b/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected index e69de29bb2..82ff1c0c36 100644 --- a/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected +++ b/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected @@ -0,0 +1 @@ +| Wall.cpp:0:0:0:0 | Wall.cpp | No warning-level options were used in the compilation of 'Wall.cpp'. | \ No newline at end of file diff --git a/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.clang b/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.clang new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.gcc b/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.gcc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.qcc b/cpp/autosar/test/rules/A1-1-2/CompilerWarningLevelNotInCompliance.expected.qcc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cpp/autosar/test/rules/A1-1-2/Wall.cpp b/cpp/autosar/test/rules/A1-1-2/Wall.cpp index cb21e0601e..b42189a8d1 100644 --- a/cpp/autosar/test/rules/A1-1-2/Wall.cpp +++ b/cpp/autosar/test/rules/A1-1-2/Wall.cpp @@ -1,2 +1,12 @@ // semmle-extractor-options: --clang -std=c++14 -Wall -// COMPLIANT \ No newline at end of file +// COMPLIANT + +// NOTE: When tested with `codeql test run`, the test extractor provides `-w` +// which overrides `-Wall` and causes this test case to be non-compliant. +// +// However, when tested with our compiler matrix tests, this test db is built +// via `codeql database create --command="..."`, and the `-w` flag will NOT be +// used. This means the `-Wall` flag is active and the test case is compliant. +// +// Therefore, the .expected file for this test expects non-compliance, and the +// .expected.gcc and .expected.clang files expect this test to be compliant. \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Pointers.qll b/cpp/common/src/codingstandards/cpp/Pointers.qll index 034080363e..22dcbd187b 100644 --- a/cpp/common/src/codingstandards/cpp/Pointers.qll +++ b/cpp/common/src/codingstandards/cpp/Pointers.qll @@ -62,12 +62,9 @@ class ArrayPointerArithmeticExpr extends PointerArithmeticExpr, ArrayExpr { predicate isNullPointerConstant(Expr e) { e.findRootCause() instanceof NullMacro or - exists(CStyleCast c | - not c.isImplicit() and - c.getExpr() = e and - e instanceof Zero and - c.getType() instanceof VoidPointerType - ) + // 8.11 Pointer type conversions states: + // A null pointer constant, i.e. the value 0, optionally cast to void *. + e instanceof Zero or isNullPointerConstant(e.(Conversion).getExpr()) } diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/BitfieldTypes2.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/BitfieldTypes2.qll new file mode 100644 index 0000000000..ca116bb51c --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/BitfieldTypes2.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype BitfieldTypes2Query = TBitFieldDeclaredAsMemberOfAUnionQuery() + +predicate isBitfieldTypes2QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `bitFieldDeclaredAsMemberOfAUnion` query + BitfieldTypes2Package::bitFieldDeclaredAsMemberOfAUnionQuery() and + queryId = + // `@id` for the `bitFieldDeclaredAsMemberOfAUnion` query + "c/misra/bit-field-declared-as-member-of-a-union" and + ruleId = "RULE-6-3" and + category = "required" +} + +module BitfieldTypes2Package { + Query bitFieldDeclaredAsMemberOfAUnionQuery() { + //autogenerate `Query` type + result = + // `Query` type for `bitFieldDeclaredAsMemberOfAUnion` query + TQueryC(TBitfieldTypes2PackageQuery(TBitFieldDeclaredAsMemberOfAUnionQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll index c2771f4171..2ddc5138b8 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/c/RuleMetadata.qll @@ -4,6 +4,7 @@ import codingstandards.cpp.exclusions.RuleMetadata //** Import packages for this language **/ import Banned import BitfieldTypes +import BitfieldTypes2 import Concurrency1 import Concurrency2 import Concurrency3 @@ -75,6 +76,7 @@ import Types1 newtype TCQuery = TBannedPackageQuery(BannedQuery q) or TBitfieldTypesPackageQuery(BitfieldTypesQuery q) or + TBitfieldTypes2PackageQuery(BitfieldTypes2Query q) or TConcurrency1PackageQuery(Concurrency1Query q) or TConcurrency2PackageQuery(Concurrency2Query q) or TConcurrency3PackageQuery(Concurrency3Query q) or @@ -146,6 +148,7 @@ newtype TCQuery = predicate isQueryMetadata(Query query, string queryId, string ruleId, string category) { isBannedQueryMetadata(query, queryId, ruleId, category) or isBitfieldTypesQueryMetadata(query, queryId, ruleId, category) or + isBitfieldTypes2QueryMetadata(query, queryId, ruleId, category) or isConcurrency1QueryMetadata(query, queryId, ruleId, category) or isConcurrency2QueryMetadata(query, queryId, ruleId, category) or isConcurrency3QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll index a18ab593bb..695a8740b6 100644 --- a/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll +++ b/cpp/common/src/codingstandards/cpp/rules/unnecessaryexposedidentifierdeclarationshared/UnnecessaryExposedIdentifierDeclarationShared.qll @@ -120,7 +120,10 @@ private predicate isTypeUse(Type t1, Type t2) { } newtype TDeclarationAccess = - ObjectAccess(Variable v, VariableAccess va) { va = v.getAnAccess() } or + ObjectAccess(Variable v, VariableAccess va) { + va = v.getAnAccess() or + v.(TemplateVariable).getAnInstantiation().getAnAccess() = va + } or /* Type access can be done in a declaration or an expression (e.g., static member function call) */ TypeAccess(Type t, Element access) { isTypeUse(access.(Variable).getUnspecifiedType(), t) @@ -205,9 +208,13 @@ class DeclarationAccess extends TDeclarationAccess { class CandidateDeclaration extends Declaration { CandidateDeclaration() { - this instanceof LocalVariable + this instanceof LocalVariable and + not this.(LocalVariable).isConstexpr() and + not this.isFromTemplateInstantiation(_) or - this instanceof GlobalOrNamespaceVariable + this instanceof GlobalOrNamespaceVariable and + not this.isFromTemplateInstantiation(_) and + not this.(GlobalOrNamespaceVariable).isConstexpr() or this instanceof Type and not this instanceof ClassTemplateInstantiation and @@ -229,7 +236,13 @@ Scope possibleScopesForDeclaration(CandidateDeclaration d) { result = scope.getStrictParent*() ) and // Limit the best scope to block statements and namespaces or control structures - (result instanceof BlockStmt or result instanceof Namespace) + ( + result instanceof BlockStmt and + // Template variables cannot be in block scope + not d instanceof TemplateVariable + or + result instanceof Namespace + ) } /* Gets the smallest scope that includes all the declaration accesses of declaration `d`. */ diff --git a/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp index ae3bb7b887..c4e01b8224 100644 --- a/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp +++ b/cpp/common/test/rules/unnecessaryexposedidentifierdeclarationshared/test.cpp @@ -136,4 +136,96 @@ void f17() { ptr = &i; } *ptr = 1; -} \ No newline at end of file +} + +namespace a_namespace { + +constexpr static unsigned int a_constexpr_var{ + 10U}; // COMPLIANT; used in + // a_namespace and + // another_namespace_function +static unsigned int + a_namespace_var[a_constexpr_var]{}; // COMPLIANT; used in + // a_namespace_function and + // another_namespace_function + +constexpr static unsigned int a_namespace_function(void) noexcept { + unsigned int a_return_value{0U}; + + for (auto loop_var : a_namespace_var) { // usage of a_namespace_var + a_return_value += loop_var; + } + return a_return_value; +} + +constexpr static unsigned int another_namespace_function(void) noexcept { + unsigned int a_return_value{0U}; + + for (unsigned int i{0U}; i < a_constexpr_var; + i++) { // usage of a_constexpr_var + a_return_value += a_namespace_var[i]; // usage of a_namespace_var + } + return a_return_value; +} +} // namespace a_namespace + +namespace parent_namespace { +namespace child_namespace { +template class a_class_in_child_namespace { +public: + template constexpr auto &&operator()(To &&val) const noexcept { + return static_cast(val); + } +}; // a_class_in_child_namespace end + +template +extern constexpr a_class_in_child_namespace + a_class_in_child_namespace_impl{}; + +} // namespace child_namespace + +template +static constexpr auto const &a_parent_namespace_variable = + child_namespace::a_class_in_child_namespace_impl< + From>; // COMPLIANT; used in child_namespace2::a_class::bar() and + // parent_namespace::another_class::foo() + +namespace child_namespace2 { +class a_class { +public: + int func(...) { return 0; } + void foo(int x) { x++; } + template constexpr auto bar(F(*func), int b) { + foo(func(a_parent_namespace_variable( + b))); // usage of a_parent_namespace_variable + } +}; // a_class +} // namespace child_namespace2 + +class another_class { + int a; + int b; + void bar(int param) { param++; } + + bool has_value() { return a == b; } + +public: + template int foo(F(*func), int b) { + if (has_value()) { + bar(func(a_parent_namespace_variable( + b))); // usage of a_parent_namespace_variable + } + return 0; + } +}; // another_class +} // namespace parent_namespace + +template T a_func(T v) { return v++; } + +int main() { + parent_namespace::child_namespace2::a_class a_class_obj; + a_class_obj.bar(a_func, 10); + parent_namespace::another_class another_class_obj; + another_class_obj.foo(a_func, 10); + return 0; +} diff --git a/rule_packages/c/BitfieldTypes2.json b/rule_packages/c/BitfieldTypes2.json new file mode 100644 index 0000000000..d916421b1f --- /dev/null +++ b/rule_packages/c/BitfieldTypes2.json @@ -0,0 +1,21 @@ +{ + "MISRA-C-2012": { + "RULE-6-3": { + "properties": { + "obligation": "required" + }, + "queries": [ + { + "description": "Type punning on a union with bit fields relies on implementation-specific alignment behavior.", + "kind": "problem", + "name": "A bit field shall not be declared as a member of a union", + "precision": "very-high", + "severity": "warning", + "short_name": "BitFieldDeclaredAsMemberOfAUnion", + "tags": ["correctness"] + } + ], + "title": "A bit field shall not be declared as a member of a union" + } + } +} \ No newline at end of file