Skip to content

Commit

Permalink
[analyzer] Fix macro names in diagnostics within bigger macros.
Browse files Browse the repository at this point in the history
If macro "CHECK_X(x)" expands to something like "if (x != NULL) ...",
the "Assuming..." note no longer says "Assuming 'x' is equal to CHECK_X".

Differential Revision: https://reviews.llvm.org/D59121

llvm-svn: 359037
  • Loading branch information
haoNoQ committed Apr 23, 2019
1 parent ace7a08 commit a746f2b
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 37 deletions.
51 changes: 15 additions & 36 deletions clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2002,43 +2002,22 @@ bool ConditionBRVisitor::patternMatch(const Expr *Ex,
const Expr *OriginalExpr = Ex;
Ex = Ex->IgnoreParenCasts();

// Use heuristics to determine if Ex is a macro expending to a literal and
// if so, use the macro's name.
SourceLocation LocStart = Ex->getBeginLoc();
SourceLocation LocEnd = Ex->getEndLoc();
if (LocStart.isMacroID() && LocEnd.isMacroID() &&
(isa<GNUNullExpr>(Ex) ||
isa<ObjCBoolLiteralExpr>(Ex) ||
isa<CXXBoolLiteralExpr>(Ex) ||
isa<IntegerLiteral>(Ex) ||
isa<FloatingLiteral>(Ex))) {
StringRef StartName = Lexer::getImmediateMacroNameForDiagnostics(LocStart,
BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
StringRef EndName = Lexer::getImmediateMacroNameForDiagnostics(LocEnd,
BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
bool beginAndEndAreTheSameMacro = StartName.equals(EndName);

bool partOfParentMacro = false;
if (ParentEx->getBeginLoc().isMacroID()) {
StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
ParentEx->getBeginLoc(), BRC.getSourceManager(),
BRC.getASTContext().getLangOpts());
partOfParentMacro = PName.equals(StartName);
}

if (beginAndEndAreTheSameMacro && !partOfParentMacro ) {
// Get the location of the macro name as written by the caller.
SourceLocation Loc = LocStart;
while (LocStart.isMacroID()) {
Loc = LocStart;
LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart);
if (isa<GNUNullExpr>(Ex) || isa<ObjCBoolLiteralExpr>(Ex) ||
isa<CXXBoolLiteralExpr>(Ex) || isa<IntegerLiteral>(Ex) ||
isa<FloatingLiteral>(Ex)) {
// Use heuristics to determine if the expression is a macro
// expanding to a literal and if so, use the macro's name.
SourceLocation BeginLoc = OriginalExpr->getBeginLoc();
SourceLocation EndLoc = OriginalExpr->getEndLoc();
if (BeginLoc.isMacroID() && EndLoc.isMacroID()) {
SourceManager &SM = BRC.getSourceManager();
const LangOptions &LO = BRC.getASTContext().getLangOpts();
if (Lexer::isAtStartOfMacroExpansion(BeginLoc, SM, LO) &&
Lexer::isAtEndOfMacroExpansion(EndLoc, SM, LO)) {
CharSourceRange R = Lexer::getAsCharRange({BeginLoc, EndLoc}, SM, LO);
Out << Lexer::getSourceText(R, SM, LO);
return false;
}
StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());

// Return the macro name.
Out << MacroName;
return false;
}
}

Expand Down
27 changes: 26 additions & 1 deletion clang/test/Analysis/diagnostics/macros.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include "../Inputs/system-header-simulator.h"
#include "../Inputs/system-header-simulator-cxx.h"

void testIntMacro(unsigned int i) {
void testUnsignedIntMacro(unsigned int i) {
if (i == UINT32_MAX) { // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
// expected-note@-1 {{Taking true branch}}
char *p = NULL; // expected-note {{'p' initialized to a null pointer value}}
Expand All @@ -12,6 +12,20 @@ void testIntMacro(unsigned int i) {
}
}


// FIXME: 'i' can never be equal to UINT32_MAX - it doesn't even fit into its
// type ('int'). This should say "Assuming 'i' is equal to -1".
void testIntMacro(int i) {
if (i == UINT32_MAX) { // expected-note {{Assuming 'i' is equal to UINT32_MAX}}
// expected-note@-1 {{Taking true branch}}
char *p = NULL; // expected-note {{'p' initialized to a null pointer value}}
*p = 7; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
}
}



void testNULLMacro(int *p) {
if (p == NULL) { // expected-note {{Assuming 'p' is equal to NULL}}
// expected-note@-1 {{Taking true branch}}
Expand Down Expand Up @@ -47,3 +61,14 @@ void testboolMacro(bool b, int *p) {
// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
}
}

#define nested_null_split(x) if ((x) != UINT32_MAX) {}

void testNestedNullSplitMacro(int i, int *p) {
nested_null_split(i); // expected-note {{Assuming 'i' is equal to -1}}
// expected-note@-1 {{Taking false branch}}
if (!p) // expected-note {{Assuming 'p' is null}}
// expected-note@-1 {{Taking true branch}}
*p = 1; // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
// expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
}

0 comments on commit a746f2b

Please sign in to comment.