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

Anna Zaks via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 15 14:55:11 PST 2016


Author: zaks
Date: Thu Dec 15 16:55:11 2016
New Revision: 289884

URL: http://llvm.org/viewvc/llvm-project?rev=289884&view=rev
Log:
[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

Added:
    cfe/trunk/test/Analysis/diagnostics/macros.cpp
    cfe/trunk/test/Analysis/diagnostics/macros.m
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/test/Analysis/Inputs/system-header-simulator-objc.h
    cfe/trunk/test/Analysis/Inputs/system-header-simulator.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h?rev=289884&r1=289883&r2=289884&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h Thu Dec 15 16:55:11 2016
@@ -245,6 +245,7 @@ public:
                                               const ExplodedNode *N);
 
   bool patternMatch(const Expr *Ex,
+                    const Expr *ParentEx,
                     raw_ostream &Out,
                     BugReporterContext &BRC,
                     BugReport &R,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=289884&r1=289883&r2=289884&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Thu Dec 15 16:55:11 2016
@@ -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
   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(co
   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
   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;
   }

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator-objc.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator-objc.h?rev=289884&r1=289883&r2=289884&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator-objc.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator-objc.h Thu Dec 15 16:55:11 2016
@@ -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;
+ at 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;

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator.h?rev=289884&r1=289883&r2=289884&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator.h Thu Dec 15 16:55:11 2016
@@ -102,3 +102,11 @@ void exit(int status) __attribute__ ((__
 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

Added: cfe/trunk/test/Analysis/diagnostics/macros.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.cpp?rev=289884&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/macros.cpp (added)
+++ cfe/trunk/test/Analysis/diagnostics/macros.cpp Thu Dec 15 16:55:11 2016
@@ -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 at -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 at -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 at -1 {{Taking true branch}}
+    *p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+             // expected-note at -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 at -1 {{Taking true branch}}
+    *p = 7;  // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+             // expected-note at -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 at -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 at -1 {{Taking true branch}}
+    *p = 7;         // expected-warning {{Dereference of null pointer (loaded from variable 'p')}}
+                    // expected-note at -1 {{Dereference of null pointer (loaded from variable 'p')}}
+  }
+}

Added: cfe/trunk/test/Analysis/diagnostics/macros.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/macros.m?rev=289884&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/diagnostics/macros.m (added)
+++ cfe/trunk/test/Analysis/diagnostics/macros.m Thu Dec 15 16:55:11 2016
@@ -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"
+
+ at interface NSDictionary : NSObject
+- (NSUInteger)count;
+- (id)objectForKey:(id)aKey;
+- (NSEnumerator *)keyEnumerator;
+ at end
+ at interface NSMutableDictionary : NSDictionary
+- (void)setObject:(id)anObject forKey:(id <NSCopying>)aKey;
+ at end
+
+void testBOOLMacro(BOOL b) {
+  if (b == YES) { // expected-note {{Assuming 'b' is equal to YES}}
+                  // expected-note at -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 at -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 at -1 {{Taking true branch}}
+    [d setObject:o forKey:[o description]]; // expected-warning {{Key argument to 'setObject:forKey:' cannot be nil}}
+                                            // expected-note at -1 {{'description' not called because the receiver is nil}}
+                                            // expected-note at -2 {{Key argument to 'setObject:forKey:' cannot be nil}}
+
+  return;
+}




More information about the cfe-commits mailing list