[PATCH] D57891: [analyzer] Fix infinite recursion in printing macros

[Tibor Brunner via Phabricator via cfe-commits]

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

#define f(y) x
#define x f(x)
int main() { x; }

This example results a compilation error since "x" in the first line was not defined earlier. However, the macro expression printer goes to an infinite recursion on this example.


Repository:
  rC Clang

https://reviews.llvm.org/D57891

Files:
  lib/StaticAnalyzer/Core/PlistDiagnostics.cpp


Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -23,6 +23,7 @@
 #include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/Statistic.h"
+#include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 
@@ -734,10 +735,20 @@
 /// As we expand the last line, we'll immediately replace PRINT(str) with
 /// print(x). The information that both 'str' and 'x' refers to the same string
 /// is an information we have to forward, hence the argument \p PrevArgs.
-static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer,
-                                                 SourceLocation MacroLoc,
-                                                 const Preprocessor &PP,
-                                                 const MacroArgMap &PrevArgs);
+///
+/// To avoid infinite recursion we maintain the already processed tokens in
+/// a set. This is carried as a parameter through the recursive calls. The set
+/// is extended with the currently processed token and after processing it, the
+/// token is removed. If the token is already in the set, then recursion stops:
+///
+/// #define f(y) x
+/// #define x f(x)
+static std::string getMacroNameAndPrintExpansion(
+    TokenPrinter &Printer,
+    SourceLocation MacroLoc,
+    const Preprocessor &PP,
+    const MacroArgMap &PrevArgs,
+    llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens);
 
 /// Retrieves the name of the macro and what it's arguments expand into
 /// at \p ExpanLoc.
@@ -786,19 +797,28 @@
   llvm::SmallString<200> ExpansionBuf;
   llvm::raw_svector_ostream OS(ExpansionBuf);
   TokenPrinter Printer(OS, PP);
+  llvm::SmallPtrSet<IdentifierInfo*, 8> AlreadyProcessedTokens;
 
-  return { getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{}),
+  return { getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{},
+                                         AlreadyProcessedTokens),
            OS.str() };
 }
 
-static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer,
-                                                 SourceLocation MacroLoc,
-                                                 const Preprocessor &PP,
-                                                 const MacroArgMap &PrevArgs) {
+static std::string getMacroNameAndPrintExpansion(
+    TokenPrinter &Printer,
+    SourceLocation MacroLoc,
+    const Preprocessor &PP,
+    const MacroArgMap &PrevArgs,
+    llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens) {
 
   const SourceManager &SM = PP.getSourceManager();
 
   MacroNameAndArgs Info = getMacroNameAndArgs(SM.getExpansionLoc(MacroLoc), PP);
+  IdentifierInfo* IDInfo = PP.getIdentifierInfo(Info.Name);
+
+  if (AlreadyProcessedTokens.find(IDInfo) != AlreadyProcessedTokens.end())
+    return Info.Name;
+  AlreadyProcessedTokens.insert(IDInfo);
 
   // Manually expand its arguments from the previous macro.
   Info.Args.expandFromPrevMacro(PrevArgs);
@@ -822,7 +842,8 @@
     // macro.
     if (const MacroInfo *MI =
                          getMacroInfoForLocation(PP, SM, II, T.getLocation())) {
-      getMacroNameAndPrintExpansion(Printer, T.getLocation(), PP, Info.Args);
+      getMacroNameAndPrintExpansion(Printer, T.getLocation(), PP, Info.Args,
+                                    AlreadyProcessedTokens);
 
       // If this is a function-like macro, skip its arguments, as
       // getExpandedMacro() already printed them. If this is the case, let's
@@ -854,7 +875,7 @@
         }
 
         getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP,
-                                      Info.Args);
+                                      Info.Args, AlreadyProcessedTokens);
         if (MI->getNumParams() != 0)
           ArgIt = getMatchingRParen(++ArgIt, ArgEnd);
       }
@@ -866,6 +887,8 @@
     Printer.printToken(T);
   }
 
+  AlreadyProcessedTokens.erase(IDInfo);
+
   return Info.Name;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57891.185755.patch
Type: text/x-patch
Size: 4186 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190207/cb4e690d/attachment.bin>