Rule 5.4: Improve alert message, ensure efficient performance

Author: lcartey

c/misra/src/rules/RULE-5-4/MacroIdentifiersNotDistinct.ql

@@ -19,23 +19,52 @@ import codingstandards.cpp.Macro
 import codingstandards.cpp.Includes
 import codingstandards.cpp.PreprocessorDirective
 
+/**
+ * Gets a top level element that this macro is expanded to, e.g. an element which does not also have
+ * an enclosing element in the macro.
+ */
+Element getATopLevelElement(MacroInvocation mi) {
+  result = mi.getAnExpandedElement() and
+  not result.getEnclosingElement() = mi.getAnExpandedElement() and
+  not result instanceof Conversion
+}
+
 /**
  * Gets a link target that this macro is expanded in.
  */
 LinkTarget getALinkTarget(Macro m) {
-  exists(Element e | e = m.getAnInvocation().getAnAffectedElement() |
+  exists(MacroInvocation mi, Element e |
+    mi = m.getAnInvocation() and
+    e = getATopLevelElement(mi)
+  |
     result = e.(Expr).getEnclosingFunction().getALinkTarget()
     or
     result = e.(Stmt).getEnclosingFunction().getALinkTarget()
     or
     exists(GlobalOrNamespaceVariable g |
       result = g.getALinkTarget() and
-      g.getInitializer().getExpr().getAChild*() = e
+      g = e.(Expr).getEnclosingDeclaration()
     )
   )
 }
 
-from Macro m, Macro m2
+/**
+ * Holds if the m1 and m2 are unconditionally included from a common file.
+ *
+ * Extracted out for performance reasons - otherwise the call to determine the file path for the
+ * message was specializing the calls to `getAnUnconditionallyIncludedFile*(..)` and causing
+ * slow performance.
+ */
+bindingset[m1, m2]
+pragma[inline_late]
+private predicate isIncludedUnconditionallyFromCommonFile(Macro m1, Macro m2) {
+  exists(File f |
+    getAnUnconditionallyIncludedFile*(f) = m1.getFile() and
+    getAnUnconditionallyIncludedFile*(f) = m2.getFile()
+  )
+}
+
+from Macro m, Macro m2, string message
 where
   not isExcluded(m, Declarations1Package::macroIdentifiersNotDistinctQuery()) and
   not m = m2 and
@@ -44,9 +73,18 @@ where
     //C90 states the first 31 characters of macro identifiers are significant and is not currently considered by this rule
     //ie an identifier differing on the 32nd character would be indistinct for C90 but distinct for C99
     //and is currently not reported by this rule
-    if m.getName().length() >= 64
-    then m.getName().prefix(63) = m2.getName().prefix(63)
-    else m.getName() = m2.getName()
+    if m.getName().length() >= 64 and not m.getName() = m2.getName()
+    then (
+      m.getName().prefix(63) = m2.getName().prefix(63) and
+      message =
+        "Macro identifer " + m.getName() + " is nondistinct in first 63 characters, compared to $@."
+    ) else (
+      m.getName() = m2.getName() and
+      message =
+        "Definition of macro " + m.getName() +
+          " is not distinct from alternative definition of $@ in " +
+          m2.getLocation().getFile().getRelativePath() + "."
+    )
   ) and
   //reduce double report since both macros are in alert, arbitrary ordering
   m.getLocation().getStartLine() >= m2.getLocation().getStartLine() and
@@ -59,10 +97,7 @@ where
   ) and
   // Must be included unconditionally from the same file, otherwise m1 may not be defined
   // when m2 is defined
-  exists(File f |
-    getAnUnconditionallyIncludedFile*(f) = m.getFile() and
-    getAnUnconditionallyIncludedFile*(f) = m2.getFile()
-  ) and
+  isIncludedUnconditionallyFromCommonFile(m, m2) and
   // Macros can't be mutually exclusive
   not mutuallyExclusiveMacros(m, m2) and
   not mutuallyExclusiveMacros(m2, m) and
@@ -74,6 +109,4 @@ where
     // Must share a link target - e.g. must both be expanded in the same context
     getALinkTarget(m) = getALinkTarget(m2)
   )
-select m,
-  "Macro identifer " + m.getName() + " is nondistinct in first 63 characters, compared to $@.", m2,
-  m2.getName()
+select m, message, m2, m2.getName()

c/misra/test/rules/RULE-5-4/MacroIdentifiersNotDistinct.expected

@@ -1,4 +1,4 @@
-| header3.h:7:1:7:24 | #define MULTIPLE_INCLUDE | Macro identifer MULTIPLE_INCLUDE is nondistinct in first 63 characters, compared to $@. | header4.h:1:1:1:24 | #define MULTIPLE_INCLUDE | MULTIPLE_INCLUDE |
-| header3.h:14:1:14:21 | #define NOT_PROTECTED | Macro identifer NOT_PROTECTED is nondistinct in first 63 characters, compared to $@. | header4.h:12:1:12:23 | #define NOT_PROTECTED 1 | NOT_PROTECTED |
+| header3.h:7:1:7:24 | #define MULTIPLE_INCLUDE | Definition of macro MULTIPLE_INCLUDE is not distinct from alternative definition of $@ in rules/RULE-5-4/header4.h. | header4.h:1:1:1:24 | #define MULTIPLE_INCLUDE | MULTIPLE_INCLUDE |
+| header3.h:14:1:14:21 | #define NOT_PROTECTED | Definition of macro NOT_PROTECTED is not distinct from alternative definition of $@ in rules/RULE-5-4/header4.h. | header4.h:12:1:12:23 | #define NOT_PROTECTED 1 | NOT_PROTECTED |
 | test.c:2:1:2:72 | #define iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB | Macro identifer iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyB is nondistinct in first 63 characters, compared to $@. | test.c:1:1:1:72 | #define iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA | iltiqzxgfqsgigwfuyntzghvzltueatcxqnqofnnvjyszmcsylyohvqaosjbqyyA |
-| test.c:8:1:8:31 | #define FUNCTION_MACRO(X) X + 1 | Macro identifer FUNCTION_MACRO is nondistinct in first 63 characters, compared to $@. | test.c:7:1:7:57 | #define FUNCTION_MACRO(FUNCTION_MACRO) FUNCTION_MACRO + 1 | FUNCTION_MACRO |
+| test.c:8:1:8:31 | #define FUNCTION_MACRO(X) X + 1 | Definition of macro FUNCTION_MACRO is not distinct from alternative definition of $@ in rules/RULE-5-4/test.c. | test.c:7:1:7:57 | #define FUNCTION_MACRO(FUNCTION_MACRO) FUNCTION_MACRO + 1 | FUNCTION_MACRO |