This is an archive of the discontinued LLVM Phabricator instance.

Differential D57891

[analyzer] Fix infinite recursion in printing macros
ClosedPublic

Authored by bruntib on Feb 7 2019, 7:09 AM.

Details

Reviewers
NoQ
george.karpenkov
Szelethus
xazax.hun
Commits
rGb4cdfe8e7fe6: [analyzer] Fix infinite recursion in printing macros
rC355705: [analyzer] Fix infinite recursion in printing macros
rL355705: [analyzer] Fix infinite recursion in printing macros
Summary

#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.

Diff Detail

Repository
rC Clang

Event Timeline

bruntib created this revision.Feb 7 2019, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2019, 7:09 AM
Herald added subscribers: cfe-commits, dkrupp, donat.nagy and 5 others. · View Herald Transcript
Szelethus added a comment.Feb 7 2019, 8:06 AM

Is there a chance of obtaining a testcase for this? I would very strongly prefer one.

Szelethus requested changes to this revision.Feb 7 2019, 8:08 AM
This revision now requires changes to proceed.Feb 7 2019, 8:08 AM
Szelethus added a child revision: D57893: [analyzer] Fix function macro crash.Feb 25 2019, 10:33 AM
bruntib updated this revision to Diff 189670.Mar 7 2019, 2:34 AM

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.

Herald added subscribers: Charusso, jdoerfert, whisperity. · View Herald TranscriptMar 7 2019, 2:34 AM
Szelethus accepted this revision.Mar 7 2019, 2:47 AM

Well, yea, getting rid of a crash is great, it's kind of bummer that can't expand the macro properly in the testfile. I really fear that we need a far greater overhaul of this entire effort to support them properly -- which should be the task of another patch.

Thanks!

This revision is now accepted and ready to land.Mar 7 2019, 2:47 AM
Closed by commit rL355705: [analyzer] Fix infinite recursion in printing macros (authored by Szelethus). · Explain WhyMar 8 2019, 8:25 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2019, 8:25 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
lib/
StaticAnalyzer/
Core/
52 lines
test/
Analysis/
Inputs/
expected-plists/
134 lines
11 lines

Diff 189670

lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

Show All 16 Lines
#include "clang/Lex/Preprocessor.h" #include "clang/Lex/Preprocessor.h"
#include "clang/Lex/TokenConcatenation.h" #include "clang/Lex/TokenConcatenation.h"
#include "clang/Rewrite/Core/HTMLRewrite.h" #include "clang/Rewrite/Core/HTMLRewrite.h"
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h" #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
#include "clang/StaticAnalyzer/Core/IssueHash.h" #include "clang/StaticAnalyzer/Core/IssueHash.h"
#include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h" #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
#include "llvm/ADT/Statistic.h" #include "llvm/ADT/Statistic.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Casting.h" #include "llvm/Support/Casting.h"
using namespace clang; using namespace clang;
using namespace ento; using namespace ento;
using namespace markup; using namespace markup;
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
▲ Show 20 Lines • Show All 738 Lines • ▼ Show 20 Lines
/// #define PRINT(x) print(x) /// #define PRINT(x) print(x)
/// #define DO_SOMETHING(str) PRINT(str) /// #define DO_SOMETHING(str) PRINT(str)
/// ///
/// DO_SOMETHING("Cute panda cubs."); /// DO_SOMETHING("Cute panda cubs.");
/// ///
/// As we expand the last line, we'll immediately replace PRINT(str) with /// 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 /// 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. /// is an information we have to forward, hence the argument \p PrevArgs.
static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer, ///
/// 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, SourceLocation MacroLoc,
const Preprocessor &PP, const Preprocessor &PP,
const MacroArgMap &PrevArgs); const MacroArgMap &PrevArgs,
llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens);
/// Retrieves the name of the macro and what it's arguments expand into /// Retrieves the name of the macro and what it's arguments expand into
/// at \p ExpanLoc. /// at \p ExpanLoc.
/// ///
/// For example, for the following macro expansion: /// For example, for the following macro expansion:
/// ///
/// #define SET_TO_NULL(x) x = 0 /// #define SET_TO_NULL(x) x = 0
/// #define NOT_SUSPICIOUS(a) \ /// #define NOT_SUSPICIOUS(a) \
Show All 32 Lines
//===----------------------------------------------------------------------===// //===----------------------------------------------------------------------===//
static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc, static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc,
const Preprocessor &PP) { const Preprocessor &PP) {
llvm::SmallString<200> ExpansionBuf; llvm::SmallString<200> ExpansionBuf;
llvm::raw_svector_ostream OS(ExpansionBuf); llvm::raw_svector_ostream OS(ExpansionBuf);
TokenPrinter Printer(OS, PP); TokenPrinter Printer(OS, PP);
llvm::SmallPtrSet<IdentifierInfo*, 8> AlreadyProcessedTokens;
std::string MacroName = std::string MacroName =
getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{}); getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{},
AlreadyProcessedTokens);
return { MacroName, OS.str() }; return { MacroName, OS.str() };
} }
static std::string getMacroNameAndPrintExpansion(TokenPrinter &Printer, static std::string getMacroNameAndPrintExpansion(
TokenPrinter &Printer,
SourceLocation MacroLoc, SourceLocation MacroLoc,
const Preprocessor &PP, const Preprocessor &PP,
const MacroArgMap &PrevArgs) { const MacroArgMap &PrevArgs,
llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens) {
const SourceManager &SM = PP.getSourceManager(); const SourceManager &SM = PP.getSourceManager();
MacroNameAndArgs Info = getMacroNameAndArgs(SM.getExpansionLoc(MacroLoc), PP); 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) if (!Info.MI)
return Info.Name; return Info.Name;
// Manually expand its arguments from the previous macro. // Manually expand its arguments from the previous macro.
Info.Args.expandFromPrevMacro(PrevArgs); Info.Args.expandFromPrevMacro(PrevArgs);
// Iterate over the macro's tokens and stringify them. // Iterate over the macro's tokens and stringify them.
Show All 10 Lines for (auto It = Info.MI->tokens_begin(), E = Info.MI->tokens_end(); It != E;
const auto *II = T.getIdentifierInfo(); const auto *II = T.getIdentifierInfo();
assert(II && assert(II &&
"This token is an identifier but has no IdentifierInfo!"); "This token is an identifier but has no IdentifierInfo!");
// If this token is a macro that should be expanded inside the current // If this token is a macro that should be expanded inside the current
// macro. // macro.
if (const MacroInfo *MI = if (const MacroInfo *MI =
getMacroInfoForLocation(PP, SM, II, T.getLocation())) { 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 // If this is a function-like macro, skip its arguments, as
// getExpandedMacro() already printed them. If this is the case, let's // getExpandedMacro() already printed them. If this is the case, let's
// first jump to the '(' token. // first jump to the '(' token.
if (MI->getNumParams() != 0) if (MI->getNumParams() != 0)
It = getMatchingRParen(++It, E); It = getMatchingRParen(++It, E);
continue; continue;
} }
Show All 15 Lines if (ArgMapIt != Info.Args.end()) {
const auto *MI = PP.getMacroInfo(ArgII); const auto *MI = PP.getMacroInfo(ArgII);
if (!MI) { if (!MI) {
Printer.printToken(*ArgIt); Printer.printToken(*ArgIt);
continue; continue;
} }
getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP, getMacroNameAndPrintExpansion(Printer, ArgIt->getLocation(), PP,
Info.Args); Info.Args, AlreadyProcessedTokens);
if (MI->getNumParams() != 0) if (MI->getNumParams() != 0)
ArgIt = getMatchingRParen(++ArgIt, ArgEnd); ArgIt = getMatchingRParen(++ArgIt, ArgEnd);
} }
continue; continue;
} }
// If control reached here, then this token isn't a macro identifier, nor an // If control reached here, then this token isn't a macro identifier, nor an
// unexpanded macro argument that we need to handle, print it. // unexpanded macro argument that we need to handle, print it.
Printer.printToken(T); Printer.printToken(T);
} }
AlreadyProcessedTokens.erase(IDInfo);
return Info.Name; return Info.Name;
} }
static MacroNameAndArgs getMacroNameAndArgs(SourceLocation ExpanLoc, static MacroNameAndArgs getMacroNameAndArgs(SourceLocation ExpanLoc,
const Preprocessor &PP) { const Preprocessor &PP) {
const SourceManager &SM = PP.getSourceManager(); const SourceManager &SM = PP.getSourceManager();
const LangOptions &LangOpts = PP.getLangOpts(); const LangOptions &LangOpts = PP.getLangOpts();
▲ Show 20 Lines • Show All 205 Lines • Show Last 20 Lines

test/Analysis/Inputs/expected-plists/plist-macros-with-expansion.cpp.plist

Show First 20 Lines • Show All 5,437 Lines • ▼ Show 20 Lines
<array> <array>
<integer>436</integer> <integer>436</integer>
<integer>437</integer> <integer>437</integer>
<integer>438</integer> <integer>438</integer>
<integer>439</integer> <integer>439</integer>
</array> </array>
</dict> </dict>
</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 &apos;garbage_value&apos; is equal to 0</string>
<key>message</key>
<string>Assuming &apos;garbage_value&apos; 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> </array>
<key>files</key> <key>files</key>
<array> <array>
<string>/home/szelethus/Documents/macro_expansion/clang/test/Analysis/plist-macros-with-expansion.cpp</string> <string>/home/szelethus/Documents/macro_expansion/clang/test/Analysis/plist-macros-with-expansion.cpp</string>
</array> </array>
</dict> </dict>
</plist> </plist>

test/Analysis/plist-macros-with-expansion.cpp

Show First 20 Lines • Show All 434 Lines • ▼ Show 20 Lines
void test() { void test() {
int *ptr; int *ptr;
YET_ANOTHER_SET_TO_NULL(5, DO_NOTHING2("Remember the Vasa"), ptr); YET_ANOTHER_SET_TO_NULL(5, DO_NOTHING2("Remember the Vasa"), ptr);
*ptr = 5; // expected-warning{{Dereference of null pointer}} *ptr = 5; // expected-warning{{Dereference of null pointer}}
} }
// CHECK: <key>name</key><string>YET_ANOTHER_SET_TO_NULL</string> // CHECK: <key>name</key><string>YET_ANOTHER_SET_TO_NULL</string>
// CHECK-NEXT: <key>expansion</key><string>print((void *)5); print((void *)&quot;Remember the Vasa&quot;); ptr = nullptr;</string> // CHECK-NEXT: <key>expansion</key><string>print((void *)5); print((void *)&quot;Remember the Vasa&quot;); 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}}
}