From 04c8ba2af1e7c828153dd8db2d6fc95573456963 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 13:06:54 +0000 Subject: [PATCH 01/11] Rule 2.4: Fix test case compiler issues This test case had some syntactically invalid C code, and a name clash, which have now been addressed. --- c/misra/test/rules/RULE-2-4/test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/c/misra/test/rules/RULE-2-4/test.c b/c/misra/test/rules/RULE-2-4/test.c index ae73b17a6e..db2199f302 100644 --- a/c/misra/test/rules/RULE-2-4/test.c +++ b/c/misra/test/rules/RULE-2-4/test.c @@ -29,10 +29,10 @@ void test() { int x = state1; // enum access on E1 enum E2 e2; struct S7 { // NON_COMPLIANT - int x + int x; } s7; struct S8 { // COMPLIANT - int x + int x; } s8; struct S8 s8_2; @@ -55,7 +55,7 @@ struct S10 { // NON_COMPLIANT struct S12 { // COMPLIANT int x; -} foo(struct S12 s); +} foo2(struct S12 s); #define STRUCT_MACRO \ struct S13 { \ From 79569b664ecaa000501e5b38980be4a409d752e6 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 13:43:51 +0000 Subject: [PATCH 02/11] Rule 2.4: Exclude template parameters --- c/misra/src/rules/RULE-2-4/UnusedTagDeclaration.ql | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/c/misra/src/rules/RULE-2-4/UnusedTagDeclaration.ql b/c/misra/src/rules/RULE-2-4/UnusedTagDeclaration.ql index e3d0d74c31..565b9fb407 100644 --- a/c/misra/src/rules/RULE-2-4/UnusedTagDeclaration.ql +++ b/c/misra/src/rules/RULE-2-4/UnusedTagDeclaration.ql @@ -29,5 +29,7 @@ where // expansions of the same macro. // Note: due to a bug in the CodeQL CLI version 2.9.4, this will currently have no effect, because // `isInMacroExpansion` is broken for `UserType`s. - not s.isInMacroExpansion() + not s.isInMacroExpansion() and + // Exclude template parameters, in case this is run on C++ code. + not s instanceof TemplateParameter select s, "struct " + s.getName() + " has an unused tag." From 5b7118ea0ab3dc663bb2f3f8ae7a62d091dbd137 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 13:49:45 +0000 Subject: [PATCH 03/11] Rule 2.4: Add a missing test case. --- c/misra/test/rules/RULE-2-4/test.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/c/misra/test/rules/RULE-2-4/test.c b/c/misra/test/rules/RULE-2-4/test.c index db2199f302..64d05a1cc2 100644 --- a/c/misra/test/rules/RULE-2-4/test.c +++ b/c/misra/test/rules/RULE-2-4/test.c @@ -77,4 +77,8 @@ void testMacroNameNotUsed() { { int x; } struct s14 PARTIAL; // NON_COMPLIANT - affected by macro, but not fully - // generated, so fair to report as unused \ No newline at end of file + // generated, so fair to report as unused + +typedef struct { + int x; +} S15; // COMPLIANT - not a tag \ No newline at end of file From 16601af5525a37a08c42e802f076c666bcdb3979 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 13:52:51 +0000 Subject: [PATCH 04/11] Rule 2.6: Address compiler syntax errors Labels cannot be before declarations, only before expressions or statements. --- c/misra/test/rules/RULE-2-6/test.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/c/misra/test/rules/RULE-2-6/test.c b/c/misra/test/rules/RULE-2-6/test.c index d3a643c1f1..e358cdcb07 100644 --- a/c/misra/test/rules/RULE-2-6/test.c +++ b/c/misra/test/rules/RULE-2-6/test.c @@ -1,12 +1,12 @@ void test1(int p1) { dead_label_1: // NON_COMPLIANT live_label_1: // COMPLIANT - int x = 0; + p1 + 1; live_label_2: // COMPLIANT dead_label_2: // NON_COMPLIANT - int y = 0; + p1 + 2; dead_label_3: // NON_COMPLIANT - int z = 0; + p1 + 3; if (p1 > 1) { goto live_label_1; From ed8ccda82e35c139b87c632be87ecf43e00b1182 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 13:57:22 +0000 Subject: [PATCH 05/11] Rule 21.1: Make compiler compatibile Compliers already exclude the #define defined case. --- ...DefineAndUndefUsedOnReservedIdentifierOrMacroName.expected | 1 - c/misra/test/rules/RULE-21-1/test.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/c/misra/test/rules/RULE-21-1/DefineAndUndefUsedOnReservedIdentifierOrMacroName.expected b/c/misra/test/rules/RULE-21-1/DefineAndUndefUsedOnReservedIdentifierOrMacroName.expected index 299626d6fc..ef9700d8d3 100644 --- a/c/misra/test/rules/RULE-21-1/DefineAndUndefUsedOnReservedIdentifierOrMacroName.expected +++ b/c/misra/test/rules/RULE-21-1/DefineAndUndefUsedOnReservedIdentifierOrMacroName.expected @@ -1,4 +1,3 @@ | test.c:1:1:1:17 | #define _NOT_OKAY | Reserved identifier '_NOT_OKAY' has been undefined or redefined. | | test.c:2:1:2:16 | #undef _NOT_OKAY | Reserved identifier '_NOT_OKAY' has been undefined or redefined. | -| test.c:4:1:4:15 | #define defined | Reserved identifier 'defined' has been undefined or redefined. | | test.c:5:1:5:13 | #define errno | Reserved identifier 'errno' has been undefined or redefined. | diff --git a/c/misra/test/rules/RULE-21-1/test.c b/c/misra/test/rules/RULE-21-1/test.c index 380679d84a..dc709ca220 100644 --- a/c/misra/test/rules/RULE-21-1/test.c +++ b/c/misra/test/rules/RULE-21-1/test.c @@ -1,7 +1,7 @@ #define _NOT_OKAY // NON_COMPLIANT #undef _NOT_OKAY // NON_COMPLIANT -#define defined // NON_COMPLIANT -#define errno // NON_COMPLIANT +// #define defined // NON_COMPILABLE +#define errno // NON_COMPLIANT #define NDEBUG 1 // COMPLIANT \ No newline at end of file From ff57bb447107bf8f1c05d3d6e290a35a4ffd0255 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 14:09:33 +0000 Subject: [PATCH 06/11] DCL41-C: Fix test compiler syntax issue Cases are labels, and cannot be before declarations. --- c/cert/test/rules/DCL41-C/test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/c/cert/test/rules/DCL41-C/test.c b/c/cert/test/rules/DCL41-C/test.c index 2500c982f3..6b982c6d0b 100644 --- a/c/cert/test/rules/DCL41-C/test.c +++ b/c/cert/test/rules/DCL41-C/test.c @@ -23,6 +23,8 @@ void f1(int expr) { void f2(int expr) { switch (expr) { case 0: + 0; // Note: required because a "case" is a label, and not permitted on a + // declaration, so we need a no-op statement int i = 4; // COMPLIANT case 1: i = 6; // COMPLIANT From c3e650ad3bdaa331dd1110999a512acd5fa551fb Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 14:10:07 +0000 Subject: [PATCH 07/11] Rule 8.2: Address clang specific error This rule has a clang specific error, not permitting certain function declarations. Update test case and expected results to reflect this. --- .../FunctionTypesNotInPrototypeForm.expected.clang | 3 +++ c/misra/test/rules/RULE-8-2/test.c.clang | 9 +++++++++ 2 files changed, 12 insertions(+) create mode 100644 c/misra/test/rules/RULE-8-2/FunctionTypesNotInPrototypeForm.expected.clang create mode 100644 c/misra/test/rules/RULE-8-2/test.c.clang diff --git a/c/misra/test/rules/RULE-8-2/FunctionTypesNotInPrototypeForm.expected.clang b/c/misra/test/rules/RULE-8-2/FunctionTypesNotInPrototypeForm.expected.clang new file mode 100644 index 0000000000..8d933c8b4d --- /dev/null +++ b/c/misra/test/rules/RULE-8-2/FunctionTypesNotInPrototypeForm.expected.clang @@ -0,0 +1,3 @@ +| test.c:3:6:3:7 | f1 | Function f1 declares parameter that is unnamed. | +| test.c:4:6:4:7 | f2 | Function f2 does not specifiy void for no parameters present. | +| test.c:7:5:7:6 | f5 | Function f5 declares parameter in unsupported declaration list. | diff --git a/c/misra/test/rules/RULE-8-2/test.c.clang b/c/misra/test/rules/RULE-8-2/test.c.clang new file mode 100644 index 0000000000..f4cfb7da14 --- /dev/null +++ b/c/misra/test/rules/RULE-8-2/test.c.clang @@ -0,0 +1,9 @@ +void f(int x); // COMPLIANT +void f0(void); // COMPLIANT +void f1(int); // NON_COMPLIANT +void f2(); // NON_COMPLIANT +// void f3(x); // NON_COMPILABLE +void f4(const x); // NON_COMPLIANT[FALSE_NEGATIVE] +int f5(x) // NON_COMPLIANT + int x; +{ return 1; } \ No newline at end of file From 71c5ae56f2975887e7a2cc44e55483a71ffbe07f Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 14:17:25 +0000 Subject: [PATCH 08/11] STR34-C: Fix compiler compatability issues For this test case our standard library stubs (MUSL) use a different form of macro for these standard library functions that gcc and clang, so update the expected result files. --- ...arBeforeConvertingToLargerSizes.expected.clang | 15 +++++++++++++++ ...CharBeforeConvertingToLargerSizes.expected.gcc | 15 +++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.clang create mode 100644 c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.gcc diff --git a/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.clang b/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.clang new file mode 100644 index 0000000000..1cf143a196 --- /dev/null +++ b/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.clang @@ -0,0 +1,15 @@ +| test.c:7:7:7:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:28:3:28:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:29:3:29:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:31:3:31:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:32:3:32:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:33:3:33:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:34:3:34:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:35:3:35:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:36:3:36:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:37:3:37:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:38:3:38:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:39:3:39:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:40:3:40:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:42:11:42:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:43:11:43:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | diff --git a/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.gcc b/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.gcc new file mode 100644 index 0000000000..1cf143a196 --- /dev/null +++ b/c/cert/test/rules/STR34-C/CastCharBeforeConvertingToLargerSizes.expected.gcc @@ -0,0 +1,15 @@ +| test.c:7:7:7:14 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:28:3:28:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:29:3:29:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:31:3:31:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:32:3:32:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:33:3:33:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:34:3:34:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:35:3:35:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:36:3:36:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:37:3:37:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:38:3:38:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:39:3:39:13 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:40:3:40:14 | (...) | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:42:11:42:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | +| test.c:43:11:43:12 | * ... | Expression not converted to `unsigned char` before converting to a larger integer type. | From dab222264ce5fc0bee92f965d4cbafdb48cf3242 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Fri, 10 Mar 2023 23:59:39 +0000 Subject: [PATCH 09/11] STR37-C: Handle macros in The commonly implements its APIs using either macros or functions or some combination of the two. Our query only assumed functions were used, whereas macros are practically used by both gcc and clang, and these can vary depending on compiler flags. The CharFunctions.qll library now provides a unified interface from which to get a unique expression for each use of an API in the library, hopefully regardless of whether it is a macro or a function. To do this we have had to hard code assumptions about the structure of the macros, however our matrix compiler testing should flag if these assumptions are broken with a particular version of a supported compiler. --- ...erHandlingFunctionsRepresentableAsUChar.ql | 17 ++- ...lingFunctionsRepresentableAsUChar.expected | 40 +++--- .../src/codingstandards/cpp/CharFunctions.qll | 118 ++++++++++++++---- 3 files changed, 124 insertions(+), 51 deletions(-) diff --git a/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql index cb742859cc..8dda9012d2 100644 --- a/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql +++ b/c/cert/src/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.ql @@ -16,14 +16,11 @@ import cpp import codingstandards.c.cert import codingstandards.cpp.CharFunctions -from FunctionCall fc, Expr arg +from UseOfToOrIsChar useOfCharAPI, Expr arg where - not isExcluded(fc, Strings2Package::toCharacterHandlingFunctionsRepresentableAsUCharQuery()) and - // examine all impacted functions - fc.getTarget() instanceof CToOrIsCharFunction and - arg = fc.getArgument(0).getFullyConverted() and - // report on cases where either the explicit or implicit cast - // on the parameter type is not unsigned - not arg.(CStyleCast).getExpr().getType() instanceof UnsignedCharType -select fc, "$@ to character-handling function may not be representable as an unsigned char.", arg, - "Argument" + not isExcluded(useOfCharAPI, + Strings2Package::toCharacterHandlingFunctionsRepresentableAsUCharQuery()) and + arg = useOfCharAPI.getConvertedArgument() and + not arg.getType() instanceof UnsignedCharType +select useOfCharAPI, + "$@ to character-handling function may not be representable as an unsigned char.", arg, "Argument" diff --git a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected index b655289f4e..43008e02d0 100644 --- a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected +++ b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected @@ -1,28 +1,28 @@ -| test.c:7:3:7:9 | call to isalnum | $@ to character-handling function may not be representable as an unsigned char. | test.c:7:11:7:12 | (int)... | Argument | -| test.c:8:3:8:13 | call to isalpha | $@ to character-handling function may not be representable as an unsigned char. | test.c:8:11:8:12 | (int)... | Argument | -| test.c:10:3:10:9 | call to isblank | $@ to character-handling function may not be representable as an unsigned char. | test.c:10:11:10:12 | (int)... | Argument | -| test.c:11:3:11:9 | call to iscntrl | $@ to character-handling function may not be representable as an unsigned char. | test.c:11:11:11:12 | (int)... | Argument | -| test.c:12:3:12:13 | call to isdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:12:11:12:12 | (int)... | Argument | -| test.c:13:3:13:13 | call to isgraph | $@ to character-handling function may not be representable as an unsigned char. | test.c:13:11:13:12 | (int)... | Argument | -| test.c:14:3:14:13 | call to islower | $@ to character-handling function may not be representable as an unsigned char. | test.c:14:11:14:12 | (int)... | Argument | -| test.c:15:3:15:13 | call to isprint | $@ to character-handling function may not be representable as an unsigned char. | test.c:15:11:15:12 | (int)... | Argument | -| test.c:16:3:16:9 | call to ispunct | $@ to character-handling function may not be representable as an unsigned char. | test.c:16:11:16:12 | (int)... | Argument | -| test.c:17:3:17:13 | call to __isspace | $@ to character-handling function may not be representable as an unsigned char. | test.c:17:11:17:12 | (int)... | Argument | -| test.c:18:3:18:13 | call to isupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:18:11:18:12 | (int)... | Argument | -| test.c:19:3:19:10 | call to isxdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:19:12:19:13 | (int)... | Argument | -| test.c:21:3:21:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:21:11:21:12 | (int)... | Argument | -| test.c:22:3:22:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:22:11:22:12 | (int)... | Argument | +| test.c:7:3:7:9 | call to isalnum | $@ to character-handling function may not be representable as an unsigned char. | test.c:7:11:7:12 | * ... | Argument | +| test.c:8:3:8:13 | isalpha(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:8:11:8:12 | * ... | Argument | +| test.c:10:3:10:9 | call to isblank | $@ to character-handling function may not be representable as an unsigned char. | test.c:10:11:10:12 | * ... | Argument | +| test.c:11:3:11:9 | call to iscntrl | $@ to character-handling function may not be representable as an unsigned char. | test.c:11:11:11:12 | * ... | Argument | +| test.c:12:3:12:13 | isdigit(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:12:3:12:13 | (...) | Argument | +| test.c:13:3:13:13 | isgraph(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:13:3:13:13 | (...) | Argument | +| test.c:14:3:14:13 | islower(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:14:3:14:13 | (...) | Argument | +| test.c:15:3:15:13 | isprint(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:15:3:15:13 | (...) | Argument | +| test.c:16:3:16:9 | call to ispunct | $@ to character-handling function may not be representable as an unsigned char. | test.c:16:11:16:12 | * ... | Argument | +| test.c:17:3:17:13 | call to __isspace | $@ to character-handling function may not be representable as an unsigned char. | test.c:17:11:17:12 | * ... | Argument | +| test.c:18:3:18:13 | isupper(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:18:3:18:13 | (...) | Argument | +| test.c:19:3:19:10 | call to isxdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:19:12:19:13 | * ... | Argument | +| test.c:21:3:21:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:21:11:21:12 | * ... | Argument | +| test.c:22:3:22:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:22:11:22:12 | * ... | Argument | | test.c:70:3:70:9 | call to isalnum | $@ to character-handling function may not be representable as an unsigned char. | test.c:70:11:70:11 | t | Argument | -| test.c:71:3:71:12 | call to isalpha | $@ to character-handling function may not be representable as an unsigned char. | test.c:71:11:71:11 | t | Argument | +| test.c:71:3:71:12 | isalpha(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:71:11:71:11 | t | Argument | | test.c:73:3:73:9 | call to isblank | $@ to character-handling function may not be representable as an unsigned char. | test.c:73:11:73:11 | t | Argument | | test.c:74:3:74:9 | call to iscntrl | $@ to character-handling function may not be representable as an unsigned char. | test.c:74:11:74:11 | t | Argument | -| test.c:75:3:75:12 | call to isdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:75:11:75:11 | t | Argument | -| test.c:76:3:76:12 | call to isgraph | $@ to character-handling function may not be representable as an unsigned char. | test.c:76:11:76:11 | t | Argument | -| test.c:77:3:77:12 | call to islower | $@ to character-handling function may not be representable as an unsigned char. | test.c:77:11:77:11 | t | Argument | -| test.c:78:3:78:12 | call to isprint | $@ to character-handling function may not be representable as an unsigned char. | test.c:78:11:78:11 | t | Argument | +| test.c:75:3:75:12 | isdigit(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:75:3:75:12 | (...) | Argument | +| test.c:76:3:76:12 | isgraph(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:76:3:76:12 | (...) | Argument | +| test.c:77:3:77:12 | islower(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:77:3:77:12 | (...) | Argument | +| test.c:78:3:78:12 | isprint(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:78:3:78:12 | (...) | Argument | | test.c:79:3:79:9 | call to ispunct | $@ to character-handling function may not be representable as an unsigned char. | test.c:79:11:79:11 | t | Argument | | test.c:80:3:80:12 | call to __isspace | $@ to character-handling function may not be representable as an unsigned char. | test.c:80:11:80:11 | t | Argument | -| test.c:81:3:81:12 | call to isupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:81:11:81:11 | t | Argument | +| test.c:81:3:81:12 | isupper(a) | $@ to character-handling function may not be representable as an unsigned char. | test.c:81:3:81:12 | (...) | Argument | | test.c:82:3:82:10 | call to isxdigit | $@ to character-handling function may not be representable as an unsigned char. | test.c:82:12:82:12 | t | Argument | | test.c:84:3:84:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:84:11:84:11 | t | Argument | | test.c:85:3:85:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:85:11:85:11 | t | Argument | diff --git a/cpp/common/src/codingstandards/cpp/CharFunctions.qll b/cpp/common/src/codingstandards/cpp/CharFunctions.qll index 352f61858c..7f69c353e5 100644 --- a/cpp/common/src/codingstandards/cpp/CharFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/CharFunctions.qll @@ -1,31 +1,107 @@ import cpp -/** - * Models a class of functions that are either testers of characters - * or standard library conversion functions. - */ -class CToOrIsCharFunction extends Function { - CToOrIsCharFunction() { - this instanceof CIsCharFunction or - this instanceof CToCharFunction - } +private string getCToOrIsName() { + result = + [ + "isalnum", "isalpha", "isascii", "isblank", "iscntrl", "isdigit", "isgraph", "islower", + "isprint", "ispunct", "isspace", "isupper", "isxdigit", "__isspace", "toascii", "toupper", + "tolower" + ] } /** - * Models a class of functions that test characters. + * A use of one of the APIs in the `` header that test or convert characters. + * + * Note: these operations are commonly implemented as either function or a macro. This class + * abstracts away from those details, providing a `getConvertedArgument` predicate to get the + * argument after any conversions specified by the user, excluding any conversions induced by + * the structure of the macro, or */ -class CIsCharFunction extends Function { - CIsCharFunction() { - getName() in [ - "isalnum", "isalpha", "isascii", "isblank", "iscntrl", "isdigit", "isgraph", "islower", - "isprint", "ispunct", "isspace", "isupper", "isxdigit", "__isspace" - ] +abstract class UseOfToOrIsChar extends Element { + /** */ + abstract Expr getConvertedArgument(); +} + +private class CToOrIsCharFunctionCall extends FunctionCall, UseOfToOrIsChar { + CToOrIsCharFunctionCall() { + getTarget().getName() = getCToOrIsName() and + // Some library implementations, such as musl, include a "dead" call to the same function + // that has also been implemented as a macro, in order to retain the right types. We exclude + // this call because it does not appear in the control flow or data flow graph. However, + // isspace directly calls __isspace, which is allowed + ( + getTarget().getName() = "__isspace" or + not any(CToOrIsCharMacroInvocation mi).getAnExpandedElement() = this + ) } + + override Expr getConvertedArgument() { result = getArgument(0).getExplicitlyConverted() } } -/** - * Models a class of functions convert characters. - */ -class CToCharFunction extends Function { - CToCharFunction() { getName() in ["toascii", "toupper", "tolower"] } +private class CToOrIsCharMacroInvocation extends MacroInvocation, UseOfToOrIsChar { + CToOrIsCharMacroInvocation() { getMacroName() = getCToOrIsName() } + + override Expr getConvertedArgument() { + /* + * There is no common approach to how the macros are defined, so we handle + * each compiler/library case individually. Fortunately, there's no conflict + * between different compilers. + */ + + // For the "is" APIs, if clang and gcc use a macro, then it expands to an + // array access on the left hand side of an & + exists(ArrayExpr ae | ae = getExpr().(BitwiseAndExpr).getLeftOperand() | + // Casted to an explicit (int), so we want unwind only a single conversion + result = ae.getArrayOffset().getFullyConverted().(Conversion).getExpr() + ) + or + // For the tolower/toupper cases, a secondary macro is expanded + exists(MacroInvocation mi | + mi.getParentInvocation() = this and + mi.getMacroName() = "__tobody" + | + /* + * tolower and toupper can be defined by macros which: + * - if the size of the type is greater than 1 + * - then check if it's a compile time constant + * - then use c < -128 || c > 255 ? c : (a)[c] + * - else call the function + * - else (a)[c] + */ + + exists(ArrayExpr ae | + ae = mi.getAnExpandedElement() and + result = ae.getArrayOffset() and + // There are two array access, but only one should be reachable + result.getBasicBlock().isReachable() + ) + or + exists(ConditionalExpr ce | + ce = mi.getAnExpandedElement() and + result = ce.getThen() and + result.getBasicBlock().isReachable() + ) + ) + or + // musl uses a conditional expression as the expansion + exists(ConditionalExpr ce | ce = getExpr() | + // for most macro expansions, the else is a subtraction inside a `<` + exists(SubExpr s | + not getMacroName() = "isalpha" and + s = ce.getElse().(LTExpr).getLeftOperand() and + // Casted to an explicit (int), so we want unwind only a single conversion + result = s.getLeftOperand().getFullyConverted().(Conversion).getExpr() + ) + or + // for isalpha, the else is a bitwise or inside a subtraction inside a `<` + exists(BitwiseOrExpr bo | + // Casted to an explicit (unsigned) + getMacroName() = "isalpha" and + bo = ce.getElse().(LTExpr).getLeftOperand().(SubExpr).getLeftOperand() and + // Casted to an explicit (int), so we want unwind only a single conversion + result = + bo.getLeftOperand().getFullyConverted().(Conversion).getExpr().(ParenthesisExpr).getExpr() + ) + ) + } } From d5b08ca9d8c5656c0596077a1335a5c3a35d06de Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 13 Mar 2023 10:19:50 +0000 Subject: [PATCH 10/11] Add change note. --- change_notes/2023-03-13-ctype-improvements.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 change_notes/2023-03-13-ctype-improvements.md diff --git a/change_notes/2023-03-13-ctype-improvements.md b/change_notes/2023-03-13-ctype-improvements.md new file mode 100644 index 0000000000..c6eb55c56d --- /dev/null +++ b/change_notes/2023-03-13-ctype-improvements.md @@ -0,0 +1 @@ + * `STR37-C` - reduce false negatives by improving detection when the `` functions are implemented using macros. \ No newline at end of file From 1abf1bc0a7ef4d0993ec5391a8fdb53a4392ff03 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Mon, 13 Mar 2023 10:40:54 +0000 Subject: [PATCH 11/11] STR37-C: Update compiler specific expected results --- ...nctionsRepresentableAsUChar.expected.clang | 28 +++++++++++++++++++ ...FunctionsRepresentableAsUChar.expected.gcc | 28 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.clang create mode 100644 c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.gcc diff --git a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.clang b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.clang new file mode 100644 index 0000000000..3ad77b8c58 --- /dev/null +++ b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.clang @@ -0,0 +1,28 @@ +| test.c:7:3:7:13 | isalnum(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:7:3:7:13 | (...) | Argument | +| test.c:8:3:8:13 | isalpha(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:8:3:8:13 | (...) | Argument | +| test.c:10:3:10:13 | isblank(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:10:3:10:13 | (...) | Argument | +| test.c:11:3:11:13 | iscntrl(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:11:3:11:13 | (...) | Argument | +| test.c:12:3:12:13 | isdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:12:3:12:13 | (...) | Argument | +| test.c:13:3:13:13 | isgraph(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:13:3:13:13 | (...) | Argument | +| test.c:14:3:14:13 | islower(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:14:3:14:13 | (...) | Argument | +| test.c:15:3:15:13 | isprint(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:15:3:15:13 | (...) | Argument | +| test.c:16:3:16:13 | ispunct(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:16:3:16:13 | (...) | Argument | +| test.c:17:3:17:13 | isspace(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:17:3:17:13 | (...) | Argument | +| test.c:18:3:18:13 | isupper(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:18:3:18:13 | (...) | Argument | +| test.c:19:3:19:14 | isxdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:19:3:19:14 | (...) | Argument | +| test.c:21:3:21:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:21:11:21:12 | * ... | Argument | +| test.c:22:3:22:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:22:11:22:12 | * ... | Argument | +| test.c:70:3:70:12 | isalnum(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:70:3:70:12 | (...) | Argument | +| test.c:71:3:71:12 | isalpha(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:71:3:71:12 | (...) | Argument | +| test.c:73:3:73:12 | isblank(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:73:3:73:12 | (...) | Argument | +| test.c:74:3:74:12 | iscntrl(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:74:3:74:12 | (...) | Argument | +| test.c:75:3:75:12 | isdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:75:3:75:12 | (...) | Argument | +| test.c:76:3:76:12 | isgraph(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:76:3:76:12 | (...) | Argument | +| test.c:77:3:77:12 | islower(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:77:3:77:12 | (...) | Argument | +| test.c:78:3:78:12 | isprint(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:78:3:78:12 | (...) | Argument | +| test.c:79:3:79:12 | ispunct(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:79:3:79:12 | (...) | Argument | +| test.c:80:3:80:12 | isspace(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:80:3:80:12 | (...) | Argument | +| test.c:81:3:81:12 | isupper(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:81:3:81:12 | (...) | Argument | +| test.c:82:3:82:13 | isxdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:82:3:82:13 | (...) | Argument | +| test.c:84:3:84:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:84:11:84:11 | t | Argument | +| test.c:85:3:85:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:85:11:85:11 | t | Argument | diff --git a/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.gcc b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.gcc new file mode 100644 index 0000000000..3ad77b8c58 --- /dev/null +++ b/c/cert/test/rules/STR37-C/ToCharacterHandlingFunctionsRepresentableAsUChar.expected.gcc @@ -0,0 +1,28 @@ +| test.c:7:3:7:13 | isalnum(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:7:3:7:13 | (...) | Argument | +| test.c:8:3:8:13 | isalpha(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:8:3:8:13 | (...) | Argument | +| test.c:10:3:10:13 | isblank(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:10:3:10:13 | (...) | Argument | +| test.c:11:3:11:13 | iscntrl(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:11:3:11:13 | (...) | Argument | +| test.c:12:3:12:13 | isdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:12:3:12:13 | (...) | Argument | +| test.c:13:3:13:13 | isgraph(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:13:3:13:13 | (...) | Argument | +| test.c:14:3:14:13 | islower(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:14:3:14:13 | (...) | Argument | +| test.c:15:3:15:13 | isprint(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:15:3:15:13 | (...) | Argument | +| test.c:16:3:16:13 | ispunct(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:16:3:16:13 | (...) | Argument | +| test.c:17:3:17:13 | isspace(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:17:3:17:13 | (...) | Argument | +| test.c:18:3:18:13 | isupper(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:18:3:18:13 | (...) | Argument | +| test.c:19:3:19:14 | isxdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:19:3:19:14 | (...) | Argument | +| test.c:21:3:21:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:21:11:21:12 | * ... | Argument | +| test.c:22:3:22:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:22:11:22:12 | * ... | Argument | +| test.c:70:3:70:12 | isalnum(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:70:3:70:12 | (...) | Argument | +| test.c:71:3:71:12 | isalpha(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:71:3:71:12 | (...) | Argument | +| test.c:73:3:73:12 | isblank(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:73:3:73:12 | (...) | Argument | +| test.c:74:3:74:12 | iscntrl(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:74:3:74:12 | (...) | Argument | +| test.c:75:3:75:12 | isdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:75:3:75:12 | (...) | Argument | +| test.c:76:3:76:12 | isgraph(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:76:3:76:12 | (...) | Argument | +| test.c:77:3:77:12 | islower(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:77:3:77:12 | (...) | Argument | +| test.c:78:3:78:12 | isprint(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:78:3:78:12 | (...) | Argument | +| test.c:79:3:79:12 | ispunct(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:79:3:79:12 | (...) | Argument | +| test.c:80:3:80:12 | isspace(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:80:3:80:12 | (...) | Argument | +| test.c:81:3:81:12 | isupper(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:81:3:81:12 | (...) | Argument | +| test.c:82:3:82:13 | isxdigit(c) | $@ to character-handling function may not be representable as an unsigned char. | test.c:82:3:82:13 | (...) | Argument | +| test.c:84:3:84:9 | call to toupper | $@ to character-handling function may not be representable as an unsigned char. | test.c:84:11:84:11 | t | Argument | +| test.c:85:3:85:9 | call to tolower | $@ to character-handling function may not be representable as an unsigned char. | test.c:85:11:85:11 | t | Argument |