[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, mikhail.ramalho, Szelethus, baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, jdoerfert, dkrupp, donat.nagy, a.sidorin, szepet.
Herald added a project: clang.

On the newly included test case the previous behavior was

  Line 53: note: Assuming 'i' is equal to nested_null_split

which is meh.

I tried to make this piece of logic slightly more correct, but it's most likely still completely incorrect.


Repository:
  rC Clang

https://reviews.llvm.org/D59121

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/test/Analysis/diagnostics/macros.cpp


Index: clang/test/Analysis/diagnostics/macros.cpp
===================================================================
--- clang/test/Analysis/diagnostics/macros.cpp
+++ clang/test/Analysis/diagnostics/macros.cpp
@@ -46,3 +46,14 @@
                     // expected-note at -1 {{Dereference of null pointer (loaded from variable 'p')}}
   }
 }
+
+#define nested_null_split(x) if ((x) != UINT32_MAX) {}
+
+void testNestedNullSplitMacro(int i, int *p) {
+  nested_null_split(i); // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
+                        // expected-note at -1 {{Taking false branch}}
+  if (!p) // expected-note {{Assuming 'p' is null}}
+          // expected-note at -1 {{Taking true branch}}
+    *p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+            // expected-note at -1 {{Dereference of null pointer (loaded from variable 'p')}}
+}
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1982,8 +1982,9 @@
     bool beginAndEndAreTheSameMacro = StartName.equals(EndName);
 
     bool partOfParentMacro = false;
+    StringRef PName = "";
     if (ParentEx->getBeginLoc().isMacroID()) {
-      StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
+      PName = Lexer::getImmediateMacroNameForDiagnostics(
           ParentEx->getBeginLoc(), BRC.getSourceManager(),
           BRC.getASTContext().getLangOpts());
       partOfParentMacro = PName.equals(StartName);
@@ -1991,13 +1992,20 @@
 
     if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
       // Get the location of the macro name as written by the caller.
-      SourceLocation Loc = LocStart;
-      while (LocStart.isMacroID()) {
-        Loc = LocStart;
+      StringRef MacroName = StartName;
+      while (true) {
         LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
+        if (!LocStart.isMacroID())
+          break;
+
+        StringRef NextMacroName = Lexer::getImmediateMacroNameForDiagnostics(
+            LocStart, BRC.getSourceManager(),
+            BRC.getASTContext().getLangOpts());
+        if (NextMacroName == PName)
+          break;
+
+        MacroName = NextMacroName;
       }
-      StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
-        Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
 
       // Return the macro name.
       Out << MacroName;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59121.189805.patch
Type: text/x-patch
Size: 2569 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190308/2ae06c08/attachment.bin>