[analyzer] Refer to macro names in diagnostics for macros representing a literal

When a macro expending to a literal is used in a comparison, use the macro name
in the diagnostic rather than the literal. This improves readability of path
notes.

Added tests for various macro literals that could occur. Only BOOl, Int, and
NULL tests have changed behavior with this patch.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@289884 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
index fdff74792f5db6bd342e54c69a4f96d6df146375..8c3a1d0d4b4084d3ecb774358843efce1b2fcbd7 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
+++ b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
@@ -245,6 +245,7 @@ public:
                                               const ExplodedNode *N);
 
   bool patternMatch(const Expr *Ex,
+                    const Expr *ParentEx,
                     raw_ostream &Out,
                     BugReporterContext &BRC,
                     BugReport &R,

diff --git a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 7045d8bb94c9f79099aeb331fd3bbeb3b59860e6..7f20f0d7703ebb0eb632a17d4548bb370ef236cd 100644 (file)
--- a/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Analysis/CFGStmtMap.h"
+#include "clang/Lex/Lexer.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
@@ -1372,7 +1373,9 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond,
   return Event;
 }
 
-bool ConditionBRVisitor::patternMatch(const Expr *Ex, raw_ostream &Out,
+bool ConditionBRVisitor::patternMatch(const Expr *Ex,
+                                      const Expr *ParentEx,
+                                      raw_ostream &Out,
                                       BugReporterContext &BRC,
                                       BugReport &report,
                                       const ExplodedNode *N,
@@ -1380,6 +1383,47 @@ bool ConditionBRVisitor::patternMatch(const Expr *Ex, raw_ostream &Out,
   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->getLocStart();
+  SourceLocation LocEnd = Ex->getLocEnd();
+  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->getLocStart().isMacroID()) {
+      StringRef PName = Lexer::getImmediateMacroNameForDiagnostics(
+        ParentEx->getLocStart(), 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);
+      }
+      StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
+        Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts());
+
+      // Return the macro name.
+      Out << MacroName;
+      return false;
+    }
+  }
+
   if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Ex)) {
     const bool quotes = isa<VarDecl>(DR->getDecl());
     if (quotes) {
@@ -1440,10 +1484,10 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond,
   SmallString<128> LhsString, RhsString;
   {
     llvm::raw_svector_ostream OutLHS(LhsString), OutRHS(RhsString);
-    const bool isVarLHS = patternMatch(BExpr->getLHS(), OutLHS, BRC, R, N,
-                                       shouldPrune);
-    const bool isVarRHS = patternMatch(BExpr->getRHS(), OutRHS, BRC, R, N,
-                                       shouldPrune);
+    const bool isVarLHS = patternMatch(BExpr->getLHS(), BExpr, OutLHS,
+                                       BRC, R, N, shouldPrune);
+    const bool isVarRHS = patternMatch(BExpr->getRHS(), BExpr, OutRHS,
+                                       BRC, R, N, shouldPrune);
 
     shouldInvert = !isVarLHS && isVarRHS;
   }

diff --git a/test/Analysis/Inputs/system-header-simulator-objc.h b/test/Analysis/Inputs/system-header-simulator-objc.h
index 9563fb1f81f9552f3158f275c9746c00e57cf294..df751d03e642afef6840526b7f0d6dfc2cf22613 100644 (file)
--- a/test/Analysis/Inputs/system-header-simulator-objc.h
+++ b/test/Analysis/Inputs/system-header-simulator-objc.h
@@ -17,7 +17,11 @@ typedef unsigned long NSUInteger;
 typedef unsigned short unichar;
 typedef UInt16 UniChar;
 
-#define NULL ((void *)0)
+#ifndef NULL
+#define __DARWIN_NULL ((void *)0)
+#define NULL __DARWIN_NULL
+#endif
+
 #define nil ((id)0)
 
 enum {
@@ -54,6 +58,7 @@ typedef struct _NSZone NSZone;
 - (oneway void)release;
 - (id)autorelease;
 - (id)init;
+@property (readonly, copy) NSString *description;
 @end  @protocol NSCopying  - (id)copyWithZone:(NSZone *)zone;
 @end  @protocol NSMutableCopying  - (id)mutableCopyWithZone:(NSZone *)zone;
 @end  @protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder;

diff --git a/test/Analysis/Inputs/system-header-simulator.h b/test/Analysis/Inputs/system-header-simulator.h
index 889e23398f03a750d400631f5c0df170873dac27..2e6f1e7d4a96f59b94cff896bc9ba95ddeea90e9 100644 (file)
--- a/test/Analysis/Inputs/system-header-simulator.h
+++ b/test/Analysis/Inputs/system-header-simulator.h
@@ -102,3 +102,11 @@ void exit(int status) __attribute__ ((__noreturn__));
 void _exit(int status) __attribute__ ((__noreturn__));
 void _Exit(int status) __attribute__ ((__noreturn__));
 
+#define UINT32_MAX        4294967295U
+#define INT64_MIN        (-INT64_MAX-1)
+#define __DBL_MAX__ 1.7976931348623157e+308
+#define DBL_MAX __DBL_MAX__
+#ifndef NULL
+#define __DARWIN_NULL 0
+#define NULL __DARWIN_NULL
+#endif
\ No newline at end of file

diff --git a/test/Analysis/diagnostics/macros.cpp b/test/Analysis/diagnostics/macros.cpp
new file mode 100644 (file)
index 0000000..8d7fccd
--- /dev/null
+++ b/test/Analysis/diagnostics/macros.cpp
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -analyze -std=c++11 -analyzer-checker=core,osx -analyzer-output=text -verify %s
+
+#include "../Inputs/system-header-simulator.h"
+#include "../Inputs/system-header-simulator-cxx.h"
+
+void testIntMacro(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}}
+    *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}}
+    *p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+             // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+void testnullptrMacro(int *p) {
+  if (p == nullptr) { // expected-note {{Assuming pointer value is null}}
+                      // expected-note@-1 {{Taking true branch}}
+    *p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+             // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}
+
+// There are no path notes on the comparison to float types.
+void testDoubleMacro(double d) {
+  if (d == DBL_MAX) { // expected-note {{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 testboolMacro(bool b, int *p) {
+  p = nullptr;      // expected-note {{Null pointer value stored to 'p'}}
+  if (b == false) { // expected-note {{Assuming the condition is true}}
+                    // expected-note@-1 {{Taking true branch}}
+    *p = 7;         // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+                    // expected-note@-1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}

diff --git a/test/Analysis/diagnostics/macros.m b/test/Analysis/diagnostics/macros.m
new file mode 100644 (file)
index 0000000..7ef80b3
--- /dev/null
+++ b/test/Analysis/diagnostics/macros.m
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,osx -fblocks -analyzer-output=text -verify %s
+
+#include "../Inputs/system-header-simulator-objc.h"
+
+@interface NSDictionary : NSObject
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
+@end
+@interface NSMutableDictionary : NSDictionary
+- (void)setObject:(id)anObject forKey:(id <NSCopying>)aKey;
+@end
+
+void testBOOLMacro(BOOL b) {
+  if (b == YES) { // expected-note {{Assuming 'b' is equal to YES}}
+                  // 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 testNilMacro(NSMutableDictionary *d, NSObject *o) {
+  if (o == nil) // expected-note {{Assuming 'o' is equal to nil}}
+                // expected-note@-1 {{Taking true branch}}
+    [d setObject:o forKey:[o description]]; // expected-warning {{Key argument to 'setObject:forKey:' cannot be nil}}
+                                            // expected-note@-1 {{'description' not called because the receiver is nil}}
+                                            // expected-note@-2 {{Key argument to 'setObject:forKey:' cannot be nil}}
+
+  return;
+}