Merge pull request #116 from lcartey/a5-2-2/issue-32/c-style-casts-from-macros

A5-2-2: Exclude c-style casts generated from library macros, and improve message

Author: lcartey

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.

cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql

@@ -16,9 +16,55 @@
 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 to " + c.getType().getName() + extraMessage + ".", l,
+  supplementary

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 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)... |  |

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
 }

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

cpp/options

@@ -1 +1 @@
-semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library
+semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library -I../../../../common/test/includes/custom-library