Skip to content

Commit

Permalink
Merge branch 'main' into lcartey/dead-code-improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
lcartey committed Sep 26, 2024
2 parents 785974b + 3d43eac commit 6370722
Show file tree
Hide file tree
Showing 34 changed files with 425 additions and 46 deletions.
50 changes: 50 additions & 0 deletions amendments.csv
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions c/misra/src/rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql
Original file line number Diff line number Diff line change
@@ -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."
Original file line number Diff line number Diff line change
@@ -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. |
Expand Down
2 changes: 1 addition & 1 deletion c/misra/test/rules/RULE-11-1/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 |
16 changes: 14 additions & 2 deletions c/misra/test/rules/RULE-11-4/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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 |
17 changes: 12 additions & 5 deletions c/misra/test/rules/RULE-11-9/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
@@ -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. |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
rules/RULE-6-3/BitFieldDeclaredAsMemberOfAUnion.ql
21 changes: 21 additions & 0 deletions c/misra/test/rules/RULE-6-3/test.c
Original file line number Diff line number Diff line change
@@ -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
};
12 changes: 12 additions & 0 deletions change_notes/2023-07-28-rule-11-4-improvements.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions change_notes/2024-09-19-fix-fp-665-M3-4-1.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `M3-4-1` - `UnnecessaryExposedIdentifierDeclarationShared.qll`:
- Fixes #665. Exclude variables that are constexpr and coming from template instantiations.
2 changes: 2 additions & 0 deletions change_notes/2024-9-20-a1-1-2-improvements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `A1-1-2` - `CompilerWarningLevelNotInCompliance.ql`:
- Report non-compliance for compilations that use the error-suppressing `-w` flag.
Original file line number Diff line number Diff line change
Expand Up @@ -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() + "'."
Original file line number Diff line number Diff line change
@@ -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'. |
Empty file.
Empty file.
Empty file.
14 changes: 13 additions & 1 deletion cpp/autosar/test/rules/A1-1-2.2/Wcast-function-type.cpp
Original file line number Diff line number Diff line change
@@ -1,2 +1,14 @@
// semmle-extractor-options: --clang -std=c++14 -Wcast-function-type
// COMPLIANT
// 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.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| Wall.cpp:0:0:0:0 | Wall.cpp | No warning-level options were used in the compilation of 'Wall.cpp'. |
Empty file.
Empty file.
Empty file.
12 changes: 11 additions & 1 deletion cpp/autosar/test/rules/A1-1-2/Wall.cpp
Original file line number Diff line number Diff line change
@@ -1,2 +1,12 @@
// semmle-extractor-options: --clang -std=c++14 -Wall
// COMPLIANT
// 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.
Loading

0 comments on commit 6370722

Please sign in to comment.