From: Kristof Umann Date: Fri, 8 Mar 2019 16:26:29 +0000 (+0000) Subject: [analyzer] Fix infinite recursion in printing macros X-Git-Tag: llvmorg-10-init~10417 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=b4cdfe8e7fe6d04a9e5e9cb574314367e3c3ba6d;p=platform%2Fupstream%2Fllvm.git [analyzer] Fix infinite recursion in printing macros In the commited testfile, macro expansion (the one implemented for the plist output) runs into an infinite recursion. The issue originates from the algorithm being faulty, as in #define value REC_MACRO_FUNC(value) the "value" is being (or at least attempted) expanded from the same macro. The solved this issue by gathering already visited macros in a set, which does resolve the crash, but will result in an incorrect macro expansion, that would preferably be fixed down the line. Patch by Tibor Brunner! Differential Revision: https://reviews.llvm.org/D57891 llvm-svn: 355705 --- diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp index bda72e8..92d0720 100644 --- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -22,6 +22,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" @@ -776,10 +777,20 @@ public: /// 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 &AlreadyProcessedTokens); /// Retrieves the name of the macro and what it's arguments expand into /// at \p ExpanLoc. @@ -828,19 +839,35 @@ static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc, llvm::SmallString<200> ExpansionBuf; llvm::raw_svector_ostream OS(ExpansionBuf); TokenPrinter Printer(OS, PP); + llvm::SmallPtrSet AlreadyProcessedTokens; + std::string MacroName = - getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{}); + getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{}, + AlreadyProcessedTokens); return { MacroName, 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 &AlreadyProcessedTokens) { const SourceManager &SM = PP.getSourceManager(); MacroNameAndArgs Info = getMacroNameAndArgs(SM.getExpansionLoc(MacroLoc), PP); + IdentifierInfo* IDInfo = PP.getIdentifierInfo(Info.Name); + + // TODO: If the macro definition contains another symbol then this function is + // called recursively. In case this symbol is the one being defined, it will + // be an infinite recursion which is stopped by this "if" statement. However, + // in this case we don't get the full expansion text in the Plist file. See + // the test file where "value" is expanded to "garbage_" instead of + // "garbage_value". + if (AlreadyProcessedTokens.find(IDInfo) != AlreadyProcessedTokens.end()) + return Info.Name; + AlreadyProcessedTokens.insert(IDInfo); if (!Info.MI) return Info.Name; @@ -867,7 +894,8 @@ static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer, // 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 @@ -899,7 +927,7 @@ static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer, } getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP, - Info.Args); + Info.Args, AlreadyProcessedTokens); if (MI->getNumParams() != 0) ArgIt = getMatchingRParen(++ArgIt, ArgEnd); } @@ -911,6 +939,8 @@ static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer, Printer.printToken(T); } + AlreadyProcessedTokens.erase(IDInfo); + return Info.Name; } diff --git a/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist b/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist index 4d1d424..68f02a3 100644 --- a/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist +++ b/clang/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist @@ -5443,6 +5443,140 @@ + + path + + + kindcontrol + edges + + + start + + + line450 + col3 + file0 + + + line450 + col4 + file0 + + + end + + + line450 + col7 + file0 + + + line450 + col11 + file0 + + + + + + + kindevent + location + + line450 + col7 + file0 + + ranges + + + + line450 + col7 + file0 + + + line450 + col16 + file0 + + + + depth0 + extended_message + Assuming 'garbage_value' is equal to 0 + message + Assuming 'garbage_value' is equal to 0 + + + kindevent + location + + line451 + col7 + file0 + + ranges + + + + line451 + col5 + file0 + + + line451 + col13 + file0 + + + + depth0 + extended_message + Division by zero + message + Division by zero + + + macro_expansions + + + location + + line450 + col7 + file0 + + namevalue + expansiongarbage_ + + + descriptionDivision by zero + categoryLogic error + typeDivision by zero + check_namecore.DivideZero + + issue_hash_content_of_line_in_context1f3c94860e67b6b863e956bd67e49f1d + issue_context_kindfunction + issue_contextrecursiveMacroUser + issue_hash_function_offset2 + location + + line451 + col7 + file0 + + ExecutedLines + + 0 + + 449 + 450 + 451 + + + files diff --git a/clang/test/Analysis/plist-macros-with-expansion.cpp b/clang/test/Analysis/plist-macros-with-expansion.cpp index c3175a3..057a165 100644 --- a/clang/test/Analysis/plist-macros-with-expansion.cpp +++ b/clang/test/Analysis/plist-macros-with-expansion.cpp @@ -440,3 +440,14 @@ void test() { } // CHECK: nameYET_ANOTHER_SET_TO_NULL // CHECK-NEXT: expansionprint((void *)5); print((void *)"Remember the Vasa"); ptr = nullptr; + +int garbage_value; + +#define REC_MACRO_FUNC(REC_MACRO_PARAM) garbage_##REC_MACRO_PARAM +#define value REC_MACRO_FUNC(value) + +void recursiveMacroUser() { + if (value == 0) + 1 / value; // expected-warning{{Division by zero}} + // expected-warning@-1{{expression result unused}} +}