r254981 - [diagnostics] Avoid crashes while printing macro backtraces

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 7 17:08:10 PST 2015


Author: rnk
Date: Mon Dec  7 19:08:09 2015
New Revision: 254981

URL: http://llvm.org/viewvc/llvm-project?rev=254981&view=rev
Log:
[diagnostics] Avoid crashes while printing macro backtraces

When attempting to map a source into a given level of macro expansion,
this code was ignoring the possibility that the start and end of the
range might take wildly different paths through the tree of macro
expansions. It was assuming that the begin spelling location would
always precede the end spelling location, which is false. A macro can
easily transpose its arguments.

This also fixes a related issue where there are extra macro arguments
between the begin location and the end location. In this situation, we
now highlight the entire macro invocation.

Pair programmed with Richard Smith.

Fixes PR12818.

Modified:
    cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
    cfe/trunk/test/Misc/caret-diags-macros.c

Modified: cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp?rev=254981&r1=254980&r2=254981&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp (original)
+++ cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp Mon Dec  7 19:08:09 2015
@@ -308,34 +308,77 @@ void DiagnosticRenderer::emitModuleBuild
 
 /// A recursive function to trace all possible backtrace locations
 /// to match the \p CaretLocFileID.
-static SourceLocation retrieveMacroLocation(SourceLocation Loc,
-                                            FileID MacroFileID,
-                                            FileID CaretFileID,
-                                            bool getBeginLoc,
-                                            const SourceManager *SM) {
-  if (MacroFileID == CaretFileID) return Loc;
-  if (!Loc.isMacroID()) return SourceLocation();
+static SourceLocation
+retrieveMacroLocation(SourceLocation Loc, FileID MacroFileID,
+                      FileID CaretFileID,
+                      const SmallVectorImpl<FileID> &CommonArgExpansions,
+                      bool IsBegin, const SourceManager *SM) {
+  assert(SM->getFileID(Loc) == MacroFileID);
+  if (MacroFileID == CaretFileID)
+    return Loc;
+  if (!Loc.isMacroID())
+    return SourceLocation();
 
   SourceLocation MacroLocation, MacroArgLocation;
 
   if (SM->isMacroArgExpansion(Loc)) {
-    MacroLocation = SM->getImmediateSpellingLoc(Loc);
-    MacroArgLocation = getBeginLoc ? SM->getImmediateExpansionRange(Loc).first
-                                   : SM->getImmediateExpansionRange(Loc).second;
+    // Only look at the immediate spelling location of this macro argument if
+    // the other location in the source range is also present in that expansion.
+    if (std::binary_search(CommonArgExpansions.begin(),
+                           CommonArgExpansions.end(), MacroFileID))
+      MacroLocation = SM->getImmediateSpellingLoc(Loc);
+    MacroArgLocation = IsBegin ? SM->getImmediateExpansionRange(Loc).first
+                               : SM->getImmediateExpansionRange(Loc).second;
   } else {
-    MacroLocation = getBeginLoc ? SM->getImmediateExpansionRange(Loc).first
-                                : SM->getImmediateExpansionRange(Loc).second;
+    MacroLocation = IsBegin ? SM->getImmediateExpansionRange(Loc).first
+                            : SM->getImmediateExpansionRange(Loc).second;
     MacroArgLocation = SM->getImmediateSpellingLoc(Loc);
   }
 
-  MacroFileID = SM->getFileID(MacroLocation);
-  MacroLocation = retrieveMacroLocation(MacroLocation, MacroFileID, CaretFileID,
-                                        getBeginLoc, SM);
-  if (MacroLocation.isValid()) return MacroLocation;
+  if (MacroLocation.isValid()) {
+    MacroFileID = SM->getFileID(MacroLocation);
+    MacroLocation =
+        retrieveMacroLocation(MacroLocation, MacroFileID, CaretFileID,
+                              CommonArgExpansions, IsBegin, SM);
+    if (MacroLocation.isValid())
+      return MacroLocation;
+  }
 
   MacroFileID = SM->getFileID(MacroArgLocation);
   return retrieveMacroLocation(MacroArgLocation, MacroFileID, CaretFileID,
-                               getBeginLoc, SM);
+                               CommonArgExpansions, IsBegin, SM);
+}
+
+/// Walk up the chain of macro expansions and collect the FileIDs identifying the
+/// expansions.
+static void getMacroArgExpansionFileIDs(SourceLocation Loc,
+                                        SmallVectorImpl<FileID> &IDs,
+                                        bool IsBegin, const SourceManager *SM) {
+  while (Loc.isMacroID()) {
+    if (SM->isMacroArgExpansion(Loc)) {
+      IDs.push_back(SM->getFileID(Loc));
+      Loc = SM->getImmediateSpellingLoc(Loc);
+    } else {
+      auto ExpRange = SM->getImmediateExpansionRange(Loc);
+      Loc = IsBegin ? ExpRange.first : ExpRange.second;
+    }
+  }
+}
+
+/// Collect the expansions of the begin and end locations and compute the set
+/// intersection. Produces a sorted vector of FileIDs in CommonArgExpansions.
+static void computeCommonMacroArgExpansionFileIDs(
+    SourceLocation Begin, SourceLocation End, const SourceManager *SM,
+    SmallVectorImpl<FileID> &CommonArgExpansions) {
+  SmallVector<FileID, 4> BeginArgExpansions;
+  SmallVector<FileID, 4> EndArgExpansions;
+  getMacroArgExpansionFileIDs(Begin, BeginArgExpansions, /*IsBegin=*/true, SM);
+  getMacroArgExpansionFileIDs(End, EndArgExpansions, /*IsBegin=*/false, SM);
+  std::sort(BeginArgExpansions.begin(), BeginArgExpansions.end());
+  std::sort(EndArgExpansions.begin(), EndArgExpansions.end());
+  std::set_intersection(BeginArgExpansions.begin(), BeginArgExpansions.end(),
+                        EndArgExpansions.begin(), EndArgExpansions.end(),
+                        std::back_inserter(CommonArgExpansions));
 }
 
 // Helper function to fix up source ranges.  It takes in an array of ranges,
@@ -387,10 +430,12 @@ static void mapDiagnosticRanges(
     }
 
     // Do the backtracking.
+    SmallVector<FileID, 4> CommonArgExpansions;
+    computeCommonMacroArgExpansionFileIDs(Begin, End, SM, CommonArgExpansions);
     Begin = retrieveMacroLocation(Begin, BeginFileID, CaretLocFileID,
-                                  true /*getBeginLoc*/, SM);
+                                  CommonArgExpansions, /*IsBegin=*/true, SM);
     End = retrieveMacroLocation(End, BeginFileID, CaretLocFileID,
-                                false /*getBeginLoc*/, SM);
+                                CommonArgExpansions, /*IsBegin=*/false, SM);
     if (Begin.isInvalid() || End.isInvalid()) continue;
 
     // Return the spelling location of the beginning and end of the range.

Modified: cfe/trunk/test/Misc/caret-diags-macros.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/caret-diags-macros.c?rev=254981&r1=254980&r2=254981&view=diff
==============================================================================
--- cfe/trunk/test/Misc/caret-diags-macros.c (original)
+++ cfe/trunk/test/Misc/caret-diags-macros.c Mon Dec  7 19:08:09 2015
@@ -220,3 +220,29 @@ Csprintf(pMsgBuf,"\nEnter minimum anagra
 // CHECK-NEXT:    {{.*}}:206:56: note: expanded from macro 'sprintf2'
 // CHECK-NEXT:      __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
 // CHECK-NEXT: {{^                                                       \^~~~~~~~~~~}}
+
+#define SWAP_AND_APPLY(arg, macro) macro arg
+#define APPLY(macro, arg) macro arg
+#define DECLARE_HELPER() __builtin_printf("%d\n", mylong);
+void use_evil_macros(long mylong) {
+  SWAP_AND_APPLY((), DECLARE_HELPER)
+  APPLY(DECLARE_HELPER, ())
+}
+// CHECK:      {{.*}}:228:22: warning: format specifies type 'int' but the argument has type 'long'
+// CHECK-NEXT:   SWAP_AND_APPLY((), DECLARE_HELPER)
+// CHECK-NEXT:   ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
+// CHECK-NEXT: {{.*}}:224:36: note: expanded from macro 'SWAP_AND_APPLY'
+// CHECK-NEXT: #define SWAP_AND_APPLY(arg, macro) macro arg
+// CHECK-NEXT:                                    ^~~~~~~~~
+// CHECK-NEXT: {{.*}}:226:51: note: expanded from macro 'DECLARE_HELPER'
+// CHECK-NEXT: #define DECLARE_HELPER() __builtin_printf("%d\n", mylong);
+// CHECK-NEXT:                                            ~~     ^~~~~~
+// CHECK-NEXT: {{.*}}:229:9: warning: format specifies type 'int' but the argument has type 'long'
+// CHECK-NEXT:   APPLY(DECLARE_HELPER, ())
+// CHECK-NEXT:   ~~~~~~^~~~~~~~~~~~~~~~~~~
+// CHECK-NEXT: {{.*}}:225:27: note: expanded from macro 'APPLY'
+// CHECK-NEXT: #define APPLY(macro, arg) macro arg
+// CHECK-NEXT:                           ^~~~~~~~~
+// CHECK-NEXT: {{.*}}:226:51: note: expanded from macro 'DECLARE_HELPER'
+// CHECK-NEXT: #define DECLARE_HELPER() __builtin_printf("%d\n", mylong);
+// CHECK-NEXT:                                            ~~     ^~~~~~




More information about the cfe-commits mailing list