From c353b289d4d23b7e4495b22073f3774a94e9792e Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 11 Oct 2024 09:43:11 +0100 Subject: [PATCH 1/4] RULE-11-*: Handle pointers to specified void types MISRA does not consider void* to be a _pointer-to-object_. However, we did not correctly strip specifiers on the base type when considering whether to exclude void* types, which incorrectly lead us to consider `const void*` as a pointer-to-object type. --- c/misra/test/rules/RULE-11-3/test.c | 4 ++++ ...onversionBetweenPointerToObjectAndIntegerType.expected | 6 +++--- c/misra/test/rules/RULE-11-4/test.c | 3 +++ c/misra/test/rules/RULE-11-5/test.c | 4 ++++ c/misra/test/rules/RULE-11-7/test.c | 8 ++++++++ change_notes/2024-10-11-specifiers-rule-11-misra-c.md | 2 ++ cpp/common/src/codingstandards/cpp/Pointers.qll | 6 +++--- 7 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 change_notes/2024-10-11-specifiers-rule-11-misra-c.md diff --git a/c/misra/test/rules/RULE-11-3/test.c b/c/misra/test/rules/RULE-11-3/test.c index 64ae688993..1f13899638 100644 --- a/c/misra/test/rules/RULE-11-3/test.c +++ b/c/misra/test/rules/RULE-11-3/test.c @@ -13,4 +13,8 @@ void f1(void) { int *v8 = (int *)0; // COMPLIANT v8 = v2; // NON_COMPLIANT v8 = (int *)(short *)v2; // NON_COMPLIANT + (const void *)v1; // COMPLIANT + const void *v9 = v1; // COMPLIANT + (int *)v9; // COMPLIANT - cast from void* + (const void *)v2; // COMPLIANT } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected b/c/misra/test/rules/RULE-11-4/ConversionBetweenPointerToObjectAndIntegerType.expected index 44d5ca5943..17a2fa223f 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: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 | +| test.c:18:1:18:24 | #define FOO (int *)0x200 | Cast from integer type 'int' to pointer to object type 'int *'. | test.c:18:1:18:24 | #define FOO (int *)0x200 | | +| test.c:26:3:26:22 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:20:1:20:34 | #define FOO_FUNCTIONAL(x) (int *)x | FOO_FUNCTIONAL | +| test.c:27:14:27:25 | (int *)... | Cast from integer type 'int' to pointer to object type 'int *' from expansion of macro $@. | test.c:21:1:21: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 5a78387247..283af5e560 100644 --- a/c/misra/test/rules/RULE-11-4/test.c +++ b/c/misra/test/rules/RULE-11-4/test.c @@ -10,6 +10,9 @@ void f1(void) { unsigned int *v4 = 0; // COMPLIANT unsigned int *v5 = NULL; // COMPLIANT unsigned int *v6 = (unsigned int *)v2; // NON_COMPLIANT + const void *v7 = 0; + (unsigned int)v7; // COMPLIANT - cast const void to int + (const void *)v1; // COMPLIANT - casting int to const void } #define FOO (int *)0x200 // NON_COMPLIANT diff --git a/c/misra/test/rules/RULE-11-5/test.c b/c/misra/test/rules/RULE-11-5/test.c index a7ffa4822e..b14333e536 100644 --- a/c/misra/test/rules/RULE-11-5/test.c +++ b/c/misra/test/rules/RULE-11-5/test.c @@ -7,4 +7,8 @@ void f1(void) { v2 = NULL; // COMPLIANT void *v3 = (void *)v1; // COMPLIANT v3 = (void *)v2; // COMPLIANT + const void *v4 = 0; + (int *)v4; // NON_COMPLIANT[FALSE_NEGATIVE] - const in type is irrelevant + (const void *)v1; // COMPLIANT - casting is from void to void, const addition + // should be irrelevant } \ No newline at end of file diff --git a/c/misra/test/rules/RULE-11-7/test.c b/c/misra/test/rules/RULE-11-7/test.c index b7dd989b00..4891aaae85 100644 --- a/c/misra/test/rules/RULE-11-7/test.c +++ b/c/misra/test/rules/RULE-11-7/test.c @@ -7,4 +7,12 @@ void f1(void) { float v4 = (float)(bool)v1; // NON_COMPLIANT v1 = (int *)v2; // NON_COMPLIANT v4 = (float)v3; // COMPLIANT + void *v5 = 0; + const void *v6 = 0; + // void pointers (regardless of specifier) are not pointers to object, so all + // these examples are compliant according to this rule + (bool)v5; // COMPLIANT + (bool)v6; // COMPLIANT + (void *)v2; // COMPLIANT + (const void *)v2; // COMPLIANT } \ No newline at end of file diff --git a/change_notes/2024-10-11-specifiers-rule-11-misra-c.md b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md new file mode 100644 index 0000000000..910a66ec71 --- /dev/null +++ b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md @@ -0,0 +1,2 @@ +- `RULE-11-3`, `RULE-11-4`, `RULE-11-5`, `RULE-11-7` - `CastBetweenObjectPointerAndDifferentObjectType.ql`, `ConversionBetweenPointerToObjectAndIntegerType.ql`, `ConversionFromPointerToVoidIntoPointerToObject.ql`, `CastBetweenPointerToObjectAndNonIntArithmeticType.ql`: + - Removed false positives where casts involved a specified void type pointer, e.g. `const void*`, which should not be considered as a pointer to object. diff --git a/cpp/common/src/codingstandards/cpp/Pointers.qll b/cpp/common/src/codingstandards/cpp/Pointers.qll index 22dcbd187b..8ed55b2bc0 100644 --- a/cpp/common/src/codingstandards/cpp/Pointers.qll +++ b/cpp/common/src/codingstandards/cpp/Pointers.qll @@ -80,9 +80,9 @@ predicate isCastNullPointerConstant(Cast c) { class PointerToObjectType extends PointerType { PointerToObjectType() { not ( - this.getUnderlyingType() instanceof FunctionPointerType or - this.getUnderlyingType() instanceof VoidPointerType or - this.getBaseType().getUnderlyingType() instanceof IncompleteType + this.getUnspecifiedType() instanceof FunctionPointerType or + this.getUnspecifiedType() instanceof VoidPointerType or + this.getBaseType().getUnspecifiedType() instanceof IncompleteType ) } } From d4ed99e93049a6eefb5bb2635a5bbe39fdcf24c0 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 11 Oct 2024 10:46:15 +0100 Subject: [PATCH 2/4] // Ignore realloc, as that memory may already be partially constructed RULE-11-5: Handle const pointers --- .../RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.ql | 2 +- .../ConversionFromPointerToVoidIntoPointerToObject.expected | 1 + c/misra/test/rules/RULE-11-5/test.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/c/misra/src/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.ql b/c/misra/src/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.ql index bdaebcbf54..0363c28c19 100644 --- a/c/misra/src/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.ql +++ b/c/misra/src/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.ql @@ -19,7 +19,7 @@ import codingstandards.cpp.Pointers from Cast cast, VoidPointerType type, PointerToObjectType newType where not isExcluded(cast, Pointers1Package::conversionFromPointerToVoidIntoPointerToObjectQuery()) and - type = cast.getExpr().getUnderlyingType() and + type = cast.getExpr().getUnspecifiedType() and newType = cast.getUnderlyingType() and not isNullPointerConstant(cast.getExpr()) select cast, diff --git a/c/misra/test/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.expected b/c/misra/test/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.expected index 5b4eec8d15..42cf288b34 100644 --- a/c/misra/test/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.expected +++ b/c/misra/test/rules/RULE-11-5/ConversionFromPointerToVoidIntoPointerToObject.expected @@ -1 +1,2 @@ | test.c:6:13:6:21 | (int *)... | Cast performed from a void pointer into a pointer to an object (int *). | +| test.c:11:3:11:11 | (int *)... | Cast performed from a void pointer into a pointer to an object (int *). | diff --git a/c/misra/test/rules/RULE-11-5/test.c b/c/misra/test/rules/RULE-11-5/test.c index b14333e536..5b5a5b3a52 100644 --- a/c/misra/test/rules/RULE-11-5/test.c +++ b/c/misra/test/rules/RULE-11-5/test.c @@ -8,7 +8,7 @@ void f1(void) { void *v3 = (void *)v1; // COMPLIANT v3 = (void *)v2; // COMPLIANT const void *v4 = 0; - (int *)v4; // NON_COMPLIANT[FALSE_NEGATIVE] - const in type is irrelevant + (int *)v4; // NON_COMPLIANT - const in type is irrelevant (const void *)v1; // COMPLIANT - casting is from void to void, const addition // should be irrelevant } \ No newline at end of file From 7504a96d6cfc098b187da4265d1fef302a5058bd Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 11 Oct 2024 10:47:31 +0100 Subject: [PATCH 3/4] Add extra test cases, update release note --- .../CastBetweenObjectPointerAndDifferentObjectType.expected | 4 ++++ c/misra/test/rules/RULE-11-3/test.c | 4 ++++ change_notes/2024-10-11-specifiers-rule-11-misra-c.md | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/c/misra/test/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.expected b/c/misra/test/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.expected index 91fd9f274a..94cf6ee635 100644 --- a/c/misra/test/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.expected +++ b/c/misra/test/rules/RULE-11-3/CastBetweenObjectPointerAndDifferentObjectType.expected @@ -2,3 +2,7 @@ | test.c:14:8:14:9 | (int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (int). | | test.c:15:8:15:25 | (int *)... | Cast performed between a pointer to object type (short) and a pointer to a different object type (int). | | test.c:15:15:15:25 | (short *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (short). | +| test.c:20:3:20:17 | (const int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (const int). | +| test.c:21:3:21:16 | (int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (int). | +| test.c:22:20:22:21 | (int *)... | Cast performed between a pointer to object type (char) and a pointer to a different object type (int). | +| test.c:23:3:23:18 | (long long *)... | Cast performed between a pointer to object type (int) and a pointer to a different object type (long long). | diff --git a/c/misra/test/rules/RULE-11-3/test.c b/c/misra/test/rules/RULE-11-3/test.c index 1f13899638..4730aeac03 100644 --- a/c/misra/test/rules/RULE-11-3/test.c +++ b/c/misra/test/rules/RULE-11-3/test.c @@ -17,4 +17,8 @@ void f1(void) { const void *v9 = v1; // COMPLIANT (int *)v9; // COMPLIANT - cast from void* (const void *)v2; // COMPLIANT + (const int *)v2; // NON_COMPLIANT + (int *const)v2; // NON_COMPLIANT + int *const v10 = v2; // NON_COMPLIANT + (long long *)v10; // NON_COMPLIANT } \ No newline at end of file diff --git a/change_notes/2024-10-11-specifiers-rule-11-misra-c.md b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md index 910a66ec71..bde621f220 100644 --- a/change_notes/2024-10-11-specifiers-rule-11-misra-c.md +++ b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md @@ -1,2 +1,2 @@ - `RULE-11-3`, `RULE-11-4`, `RULE-11-5`, `RULE-11-7` - `CastBetweenObjectPointerAndDifferentObjectType.ql`, `ConversionBetweenPointerToObjectAndIntegerType.ql`, `ConversionFromPointerToVoidIntoPointerToObject.ql`, `CastBetweenPointerToObjectAndNonIntArithmeticType.ql`: - - Removed false positives where casts involved a specified void type pointer, e.g. `const void*`, which should not be considered as a pointer to object. + - Removed false positives where casts involved a specified void type pointer, e.g. `const void*`, which should not be considered as a pointer to object, but should be considered a pointer-to-void. \ No newline at end of file From 6e930255395323629061b5a0f684869e011d773b Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 11 Oct 2024 10:56:30 +0100 Subject: [PATCH 4/4] Update release notes --- change_notes/2024-10-11-specifiers-rule-11-misra-c.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/change_notes/2024-10-11-specifiers-rule-11-misra-c.md b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md index bde621f220..5f74dc6b3f 100644 --- a/change_notes/2024-10-11-specifiers-rule-11-misra-c.md +++ b/change_notes/2024-10-11-specifiers-rule-11-misra-c.md @@ -1,2 +1,4 @@ - `RULE-11-3`, `RULE-11-4`, `RULE-11-5`, `RULE-11-7` - `CastBetweenObjectPointerAndDifferentObjectType.ql`, `ConversionBetweenPointerToObjectAndIntegerType.ql`, `ConversionFromPointerToVoidIntoPointerToObject.ql`, `CastBetweenPointerToObjectAndNonIntArithmeticType.ql`: - - Removed false positives where casts involved a specified void type pointer, e.g. `const void*`, which should not be considered as a pointer to object, but should be considered a pointer-to-void. \ No newline at end of file + - Removed false positives where casts involved a specified void type pointer, e.g. `const void*`, which should not be considered as a pointer to object. +- `RULE-11-5` - `ConversionFromPointerToVoidIntoPointerToObject.ql`: + - Addressed false negatives where the pointer-to-void was specified. \ No newline at end of file