From e0398761c37d2bbe9ee3f4ea91e7f1ce243d9f6a Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 2 Nov 2022 22:58:35 +0000 Subject: [PATCH 1/3] A5-2-2: Exclude c-style casts generated by library macros casts generated by library macros are not modifiable by the user, so can be excluded from the results of this rule. In addition, this commit improves the query message to specify whether the cast was generated due to the expansion of a macro, and to provide a link to the macro in question. This helps users identify the location in the code where the cast is actually written. --- .../A5-2-2/TraditionalCStyleCastsUsed.ql | 51 +++++++++++++++++-- .../TraditionalCStyleCastsUsed.expected | 10 ++-- cpp/autosar/test/rules/A5-2-2/test.cpp | 21 ++++++++ .../custom-library/macro_c_style_casts.h | 4 ++ cpp/options | 2 +- 5 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 cpp/common/test/includes/custom-library/macro_c_style_casts.h diff --git a/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql b/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql index 0b6962c06e..920da0137e 100644 --- a/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql +++ b/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql @@ -16,9 +16,54 @@ import cpp import codingstandards.cpp.autosar -from CStyleCast c +/** + * Gets the macro (if any) that generated the given `CStyleCast`. + * + * If there are nested macro invocations, we identify the most specific macro that generated the + * cast. + */ +Macro getGeneratedFrom(CStyleCast c) { + exists(MacroInvocation mi | + mi = result.getAnInvocation() and + mi.getAGeneratedElement() = c and + mi.getLocation().getStartColumn() = c.getLocation().getStartColumn() and + not exists(MacroInvocation child | + child.getParentInvocation() = mi and + child.getAGeneratedElement() = c + ) + ) +} + +/** A macro within the source location of this project. */ +class UserProvidedMacro extends Macro { + UserProvidedMacro() { exists(this.getFile().getRelativePath()) } +} + +/** A macro defined within a library used by this project. */ +class LibraryMacro extends Macro { + LibraryMacro() { not this instanceof UserProvidedMacro } +} + +from CStyleCast c, string extraMessage, Locatable l, string supplementary where not isExcluded(c, BannedSyntaxPackage::traditionalCStyleCastsUsedQuery()) and not c.isImplicit() and - not c.getType() instanceof UnknownType -select c, "Use of explicit C-style Cast" + not c.getType() instanceof UnknownType and + // Exclude casts created from macro invocations of macros defined by third parties + not getGeneratedFrom(c) instanceof LibraryMacro and + // If the cast was generated from a user-provided macro, then report the macro that generated the + // cast, as the macro itself may have generated the cast + if getGeneratedFrom(c) instanceof UserProvidedMacro + then + extraMessage = " generated from macro $@" and + // Add macro as explanatory link + l = getGeneratedFrom(c) and + supplementary = getGeneratedFrom(c).getName() + else ( + // No extra message required + extraMessage = "" and + // No explanatory link required, but we still need to set these to valid values + l = c and + supplementary = "" + ) +select c, "Use of explicit c-style cast" + extraMessage, l, supplementary diff --git a/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected b/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected index 1b349cea04..b9a305eb91 100644 --- a/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected +++ b/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected @@ -1,3 +1,7 @@ -| test.cpp:8:22:8:37 | (uint32_t)... | Use of explicit C-style Cast | -| test.cpp:9:22:9:32 | (unsigned int)... | Use of explicit C-style Cast | -| test.cpp:15:3:15:13 | (void)... | Use of explicit C-style Cast | +| test.cpp:8:22:8:37 | (uint32_t)... | Use of explicit c-style cast | test.cpp:8:22:8:37 | (uint32_t)... | | +| test.cpp:9:22:9:32 | (unsigned int)... | Use of explicit c-style cast | test.cpp:9:22:9:32 | (unsigned int)... | | +| test.cpp:15:3:15:13 | (void)... | Use of explicit c-style cast | test.cpp:15:3:15:13 | (void)... | | +| test.cpp:77:3:77:11 | (int)... | Use of explicit c-style cast generated from macro $@ | test.cpp:70:1:70:31 | #define ADD_ONE(x) ((int)x) + 1 | ADD_ONE | +| test.cpp:79:3:79:18 | (int)... | Use of explicit c-style cast generated from macro $@ | test.cpp:71:1:71:36 | #define NESTED_ADD_ONE(x) ADD_ONE(x) | NESTED_ADD_ONE | +| test.cpp:85:19:85:26 | (int)... | Use of explicit c-style cast | test.cpp:85:19:85:26 | (int)... | | +| test.cpp:86:27:86:34 | (int)... | Use of explicit c-style cast | test.cpp:86:27:86:34 | (int)... | | diff --git a/cpp/autosar/test/rules/A5-2-2/test.cpp b/cpp/autosar/test/rules/A5-2-2/test.cpp index 23820114cb..664a10f469 100644 --- a/cpp/autosar/test/rules/A5-2-2/test.cpp +++ b/cpp/autosar/test/rules/A5-2-2/test.cpp @@ -65,4 +65,25 @@ class A5_2_2 final { void a5_2_2_test() { A5_2_2 a; a.f(""); +} + +#define ADD_ONE(x) ((int)x) + 1 +#define NESTED_ADD_ONE(x) ADD_ONE(x) +#define NO_CAST_ADD_ONE(x) x + 1 + +#include "macro_c_style_casts.h" + +void test_macro_cast() { + ADD_ONE(1); // NON_COMPLIANT - expansion of user-defined macro creates + // c-style cast + NESTED_ADD_ONE(1); // NON_COMPLIANT - expansion of user-defined macro creates + // c-style cast + LIBRARY_ADD_TWO(1); // COMPLIANT - macro generating the cast is defined in a + // library, and is not modifiable by the user + LIBRARY_NESTED_ADD_TWO(1); // COMPLIANT - macro generating the cast is defined + // in a library, and is not modifiable by the user + NO_CAST_ADD_ONE((int)1.0); // NON_COMPLIANT - cast in argument to macro + LIBRARY_NO_CAST_ADD_TWO((int)1.0); // NON_COMPLIANT - library macro with + // c-style cast in argument, written by + // user so should be reported } \ No newline at end of file diff --git a/cpp/common/test/includes/custom-library/macro_c_style_casts.h b/cpp/common/test/includes/custom-library/macro_c_style_casts.h new file mode 100644 index 0000000000..31af2a4ead --- /dev/null +++ b/cpp/common/test/includes/custom-library/macro_c_style_casts.h @@ -0,0 +1,4 @@ +// Macros used in test for A5-2-2 +#define LIBRARY_ADD_TWO(x) ((int)x) + 2 +#define LIBRARY_NESTED_ADD_TWO(x) LIBRARY_ADD_TWO(x) +#define LIBRARY_NO_CAST_ADD_TWO(x) x + 1 \ No newline at end of file diff --git a/cpp/options b/cpp/options index f1c6ec672f..1f8961ecda 100644 --- a/cpp/options +++ b/cpp/options @@ -1 +1 @@ -semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library \ No newline at end of file +semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library -I../../../../common/test/includes/custom-library \ No newline at end of file From e185f3756edfd166343c09bba8af3a3f1fef94ce Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 2 Nov 2022 23:03:07 +0000 Subject: [PATCH 2/3] A5-2-2: Update message to cite the type Improve result message to cite the type being casted to, to help with validation and remediation. --- .../src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql | 3 ++- .../A5-2-2/TraditionalCStyleCastsUsed.expected | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql b/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql index 920da0137e..e7f6e96eb5 100644 --- a/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql +++ b/cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql @@ -66,4 +66,5 @@ where l = c and supplementary = "" ) -select c, "Use of explicit c-style cast" + extraMessage, l, supplementary +select c, "Use of explicit c-style cast to " + c.getType().getName() + extraMessage + ".", l, + supplementary diff --git a/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected b/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected index b9a305eb91..291eb53348 100644 --- a/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected +++ b/cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected @@ -1,7 +1,7 @@ -| test.cpp:8:22:8:37 | (uint32_t)... | Use of explicit c-style cast | test.cpp:8:22:8:37 | (uint32_t)... | | -| test.cpp:9:22:9:32 | (unsigned int)... | Use of explicit c-style cast | test.cpp:9:22:9:32 | (unsigned int)... | | -| test.cpp:15:3:15:13 | (void)... | Use of explicit c-style cast | test.cpp:15:3:15:13 | (void)... | | -| test.cpp:77:3:77:11 | (int)... | Use of explicit c-style cast generated from macro $@ | test.cpp:70:1:70:31 | #define ADD_ONE(x) ((int)x) + 1 | ADD_ONE | -| test.cpp:79:3:79:18 | (int)... | Use of explicit c-style cast generated from macro $@ | test.cpp:71:1:71:36 | #define NESTED_ADD_ONE(x) ADD_ONE(x) | NESTED_ADD_ONE | -| test.cpp:85:19:85:26 | (int)... | Use of explicit c-style cast | test.cpp:85:19:85:26 | (int)... | | -| test.cpp:86:27:86:34 | (int)... | Use of explicit c-style cast | test.cpp:86:27:86:34 | (int)... | | +| test.cpp:8:22:8:37 | (uint32_t)... | Use of explicit c-style cast to uint32_t. | test.cpp:8:22:8:37 | (uint32_t)... | | +| test.cpp:9:22:9:32 | (unsigned int)... | Use of explicit c-style cast to unsigned int. | test.cpp:9:22:9:32 | (unsigned int)... | | +| test.cpp:15:3:15:13 | (void)... | Use of explicit c-style cast to void. | test.cpp:15:3:15:13 | (void)... | | +| test.cpp:77:3:77:11 | (int)... | Use of explicit c-style cast to int generated from macro $@. | test.cpp:70:1:70:31 | #define ADD_ONE(x) ((int)x) + 1 | ADD_ONE | +| test.cpp:79:3:79:18 | (int)... | Use of explicit c-style cast to int generated from macro $@. | test.cpp:71:1:71:36 | #define NESTED_ADD_ONE(x) ADD_ONE(x) | NESTED_ADD_ONE | +| test.cpp:85:19:85:26 | (int)... | Use of explicit c-style cast to int. | test.cpp:85:19:85:26 | (int)... | | +| test.cpp:86:27:86:34 | (int)... | Use of explicit c-style cast to int. | test.cpp:86:27:86:34 | (int)... | | From 3f17a4695b8e2f4bc93a9650636724f8427f04e8 Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Wed, 2 Nov 2022 23:05:29 +0000 Subject: [PATCH 3/3] A5-2-2: Add changenote for c-style cast library macros --- change_notes/2022-11-02-c-style-casts-library-macros.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 change_notes/2022-11-02-c-style-casts-library-macros.md diff --git a/change_notes/2022-11-02-c-style-casts-library-macros.md b/change_notes/2022-11-02-c-style-casts-library-macros.md new file mode 100644 index 0000000000..92bfa4c435 --- /dev/null +++ b/change_notes/2022-11-02-c-style-casts-library-macros.md @@ -0,0 +1,4 @@ + - `A5-2-2` - `TraditionalCStyleCastsUsed.ql` + - Reduced false positives by excluding casts generated by library macros (i.e. macros defined outside the source location) + - Improved the message to cite the macro which generated the c-style cast, if any. + - Improved the message to cite the type being casted to, to aid with identification and remediation. \ No newline at end of file