Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule 11.4 improvements #332

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 1 addition & 6 deletions c/common/src/codingstandards/c/Pointers.qll
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,7 @@ 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
)
e instanceof Zero
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self: this affects Rule 11.9, which is incorrect. 0 is a null pointer constant, but not permitted according to 11.9.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add this comment to the changenote (if it changes the result of Rule 11.9)

or
isNullPointerConstant(e.(Conversion).getExpr())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,73 @@

import cpp
import codingstandards.c.misra
import codingstandards.cpp.Macro
import codingstandards.c.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::castBetweenObjectPointerAndDifferentObjectTypeQuery()) 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
@@ -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
}
4 changes: 4 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,4 @@
- `RULE-11-4`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- `RULE-11-4`
- `RULE-11-4` - `ConversionBetweenPointerToObjectAndIntegerType.ql`:
- Fixed issue #331 - consider `0` a null pointer constant.

- Reduce false positives by considering `0` a null pointer constant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Reduce false positives by considering `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.