bruntib updated this revision to Diff 189670.
bruntib added a comment.
Herald added subscribers: Charusso, jdoerfert, whisperity.
I added a test case for this recursive case. There is also a TODO in the code
indicating the place where an additional fix will be required.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57891/new/
https://reviews.llvm.org/D57891
Files:
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
test/Analysis/plist-macros-with-expansion.cpp
Index: test/Analysis/plist-macros-with-expansion.cpp
===================================================================
--- test/Analysis/plist-macros-with-expansion.cpp
+++ test/Analysis/plist-macros-with-expansion.cpp
@@ -440,3 +440,14 @@
}
// 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@-1{{expression result unused}}
+}
Index: test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
===================================================================
--- test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
+++ test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist
@@ -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>
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ 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 @@
/// 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 @@
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 @@
// 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 @@
}
getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP,
- Info.Args);
+ Info.Args, AlreadyProcessedTokens);
if (MI->getNumParams() != 0)
ArgIt = getMatchingRParen(++ArgIt, ArgEnd);
}
@@ -911,6 +939,8 @@
Printer.printToken(T);
}
+ AlreadyProcessedTokens.erase(IDInfo);
+
return Info.Name;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
