r355705 - [analyzer] Fix infinite recursion in printing macros
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 8 08:26:29 PST 2019
Author: szelethus
Date: Fri Mar 8 08:26:29 2019
New Revision: 355705
URL: http://llvm.org/viewvc/llvm-project?rev=355705&view=rev
Log:
[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
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp?rev=355705&r1=355704&r2=355705&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp Fri Mar 8 08:26:29 2019
@@ -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<IdentifierInfo *, 8> &AlreadyProcessedTokens);
/// Retrieves the name of the macro and what it's arguments expand into
/// at \p ExpanLoc.
@@ -828,19 +839,35 @@ static ExpansionInfo getExpandedMacro(So
llvm::SmallString<200> ExpansionBuf;
llvm::raw_svector_ostream OS(ExpansionBuf);
TokenPrinter Printer(OS, PP);
+ llvm::SmallPtrSet<IdentifierInfo*, 8> 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<IdentifierInfo *, 8> &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 getMacroNameAndPrintE
// 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 getMacroNameAndPrintE
}
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 getMacroNameAndPrintE
Printer.printToken(T);
}
+ AlreadyProcessedTokens.erase(IDInfo);
+
return Info.Name;
}
Modified: cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist?rev=355705&r1=355704&r2=355705&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist (original)
+++ cfe/trunk/test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist Fri Mar 8 08:26:29 2019
@@ -5443,6 +5443,140 @@
</array>
</dict>
</dict>
+ <dict>
+ <key>path</key>
+ <array>
+ <dict>
+ <key>kind</key><string>control</string>
+ <key>edges</key>
+ <array>
+ <dict>
+ <key>start</key>
+ <array>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>3</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>4</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ <key>end</key>
+ <array>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>11</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </dict>
+ </array>
+ </dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>16</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>Assuming 'garbage_value' is equal to 0</string>
+ <key>message</key>
+ <string>Assuming 'garbage_value' is equal to 0</string>
+ </dict>
+ <dict>
+ <key>kind</key><string>event</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>451</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ranges</key>
+ <array>
+ <array>
+ <dict>
+ <key>line</key><integer>451</integer>
+ <key>col</key><integer>5</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <dict>
+ <key>line</key><integer>451</integer>
+ <key>col</key><integer>13</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ </array>
+ </array>
+ <key>depth</key><integer>0</integer>
+ <key>extended_message</key>
+ <string>Division by zero</string>
+ <key>message</key>
+ <string>Division by zero</string>
+ </dict>
+ </array>
+ <key>macro_expansions</key>
+ <array>
+ <dict>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>450</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>name</key><string>value</string>
+ <key>expansion</key><string>garbage_</string>
+ </dict>
+ </array>
+ <key>description</key><string>Division by zero</string>
+ <key>category</key><string>Logic error</string>
+ <key>type</key><string>Division by zero</string>
+ <key>check_name</key><string>core.DivideZero</string>
+ <!-- This hash is experimental and going to change! -->
+ <key>issue_hash_content_of_line_in_context</key><string>1f3c94860e67b6b863e956bd67e49f1d</string>
+ <key>issue_context_kind</key><string>function</string>
+ <key>issue_context</key><string>recursiveMacroUser</string>
+ <key>issue_hash_function_offset</key><string>2</string>
+ <key>location</key>
+ <dict>
+ <key>line</key><integer>451</integer>
+ <key>col</key><integer>7</integer>
+ <key>file</key><integer>0</integer>
+ </dict>
+ <key>ExecutedLines</key>
+ <dict>
+ <key>0</key>
+ <array>
+ <integer>449</integer>
+ <integer>450</integer>
+ <integer>451</integer>
+ </array>
+ </dict>
+ </dict>
</array>
<key>files</key>
<array>
Modified: cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp?rev=355705&r1=355704&r2=355705&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp (original)
+++ cfe/trunk/test/Analysis/plist-macros-with-expansion.cpp Fri Mar 8 08:26:29 2019
@@ -440,3 +440,14 @@ void test() {
}
// CHECK: <key>name</key><string>YET_ANOTHER_SET_TO_NULL</string>
// CHECK-NEXT: <key>expansion</key><string>print((void *)5); print((void *)"Remember the Vasa"); ptr = nullptr;</string>
+
+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 at -1{{expression result unused}}
+}
More information about the cfe-commits
mailing list