[analyzer] Display the checker name in the text output
authorKirstóf Umann <dkszelethus@gmail.com>
Fri, 20 Mar 2020 15:45:53 +0000 (16:45 +0100)
committerKirstóf Umann <dkszelethus@gmail.com>
Thu, 9 Apr 2020 14:21:45 +0000 (16:21 +0200)
Exactly what it says on the tin! There is no reason I think not to have this.

Also, I added test files for checkers that emit warning under the wrong name.

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

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
clang/test/Analysis/analyzer-config.c
clang/test/Analysis/dispatch-once.m
clang/test/Analysis/explain-svals.c
clang/test/Analysis/explain-svals.cpp
clang/test/Analysis/explain-svals.m
clang/test/Analysis/incorrect-checker-names.cpp [new file with mode: 0644]
clang/test/Analysis/incorrect-checker-names.mm [new file with mode: 0644]

index ee67f60..597a65c 100644 (file)
@@ -310,6 +310,10 @@ ANALYZER_OPTION(bool, ShouldApplyFixIts, "apply-fixits",
                 "Apply the fix-it hints to the files",
                 false)
 
+ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
+                "Display the checker name for textual outputs",
+                true)
+
 //===----------------------------------------------------------------------===//
 // Unsigned analyzer options.
 //===----------------------------------------------------------------------===//
index 07e5685..f4c7e59 100644 (file)
@@ -35,17 +35,19 @@ namespace {
 /// diagnostics in textual format for the 'text' output type.
 class TextDiagnostics : public PathDiagnosticConsumer {
   DiagnosticsEngine &DiagEng;
-  LangOptions LO;
+  const LangOptions &LO;
   const bool IncludePath = false;
   const bool ShouldEmitAsError = false;
   const bool ApplyFixIts = false;
+  const bool ShouldDisplayCheckerName = false;
 
 public:
-  TextDiagnostics(DiagnosticsEngine &DiagEng, LangOptions LO,
+  TextDiagnostics(DiagnosticsEngine &DiagEng, const LangOptions &LO,
                   bool ShouldIncludePath, const AnalyzerOptions &AnOpts)
       : DiagEng(DiagEng), LO(LO), IncludePath(ShouldIncludePath),
         ShouldEmitAsError(AnOpts.AnalyzerWerror),
-        ApplyFixIts(AnOpts.ShouldApplyFixIts) {}
+        ApplyFixIts(AnOpts.ShouldApplyFixIts),
+        ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
   ~TextDiagnostics() override {}
 
   StringRef getName() const override { return "TextDiagnostics"; }
@@ -90,9 +92,13 @@ public:
          E = Diags.end();
          I != E; ++I) {
       const PathDiagnostic *PD = *I;
+      std::string WarningMsg =
+          (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
+              .str();
+
       reportPiece(WarnID, PD->getLocation().asLocation(),
-                  PD->getShortDescription(), PD->path.back()->getRanges(),
-                  PD->path.back()->getFixits());
+                  (PD->getShortDescription() + WarningMsg).str(),
+                  PD->path.back()->getRanges(), PD->path.back()->getFixits());
 
       // First, add extra notes, even if paths should not be included.
       for (const auto &Piece : PD->path) {
@@ -100,7 +106,8 @@ public:
           continue;
 
         reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+                    Piece->getString(), Piece->getRanges(),
+                    Piece->getFixits());
       }
 
       if (!IncludePath)
@@ -113,7 +120,8 @@ public:
           continue;
 
         reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+                    Piece->getString(), Piece->getRanges(),
+                    Piece->getFixits());
       }
     }
 
index 068930e..7069ee4 100644 (file)
@@ -52,6 +52,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false
+// CHECK-NEXT: display-checker-name = true
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 97
+// CHECK-NEXT: num-entries = 98
index 7314dc9..b8cf582 100644 (file)
@@ -1,5 +1,14 @@
-// RUN: %clang_analyze_cc1 -w -fblocks -analyzer-checker=core,osx.API,unix.Malloc -verify %s
-// RUN: %clang_analyze_cc1 -w -fblocks -fobjc-arc -analyzer-checker=core,osx.API,unix.Malloc -verify %s
+// RUN: %clang_analyze_cc1 -w -fblocks -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=osx.API \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config display-checker-name=false
+
+// RUN: %clang_analyze_cc1 -w -fblocks -fobjc-arc -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=osx.API \
+// RUN:   -analyzer-checker=unix.Malloc \
+// RUN:   -analyzer-config display-checker-name=false
 
 #include "Inputs/system-header-simulator-objc.h"
 
index f1540bb..204df7c 100644 (file)
@@ -1,4 +1,8 @@
-// RUN: %clang_cc1 -triple i386-apple-darwin10 -analyze -analyzer-checker=core.builtin,debug.ExprInspection,unix.cstring -verify %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -verify %s \
+// RUN:   -analyzer-checker=core.builtin \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=unix.cstring \
+// RUN:   -analyzer-config display-checker-name=false
 
 struct S {
   int z;
index 9c37642..f8ec234 100644 (file)
@@ -1,4 +1,8 @@
-// RUN: %clang_analyze_cc1 -std=c++14 -triple i386-apple-darwin10 -analyzer-checker=core.builtin,debug.ExprInspection,unix.cstring -verify %s
+// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -verify %s \
+// RUN:   -analyzer-checker=core.builtin \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=unix.cstring \
+// RUN:   -analyzer-config display-checker-name=false
 
 typedef unsigned long size_t;
 
index dd40946..b2efebf 100644 (file)
@@ -1,4 +1,7 @@
-// RUN: %clang_analyze_cc1 -w -triple i386-apple-darwin10 -fblocks -analyzer-checker=core.builtin,debug.ExprInspection -verify %s
+// RUN: %clang_analyze_cc1 -w -triple i386-apple-darwin10 -fblocks -verify %s \
+// RUN:   -analyzer-checker=core.builtin \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config display-checker-name=false
 
 #include "Inputs/system-header-simulator-objc.h"
 
diff --git a/clang/test/Analysis/incorrect-checker-names.cpp b/clang/test/Analysis/incorrect-checker-names.cpp
new file mode 100644 (file)
index 0000000..3519e15
--- /dev/null
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -verify %s -fblocks \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-output=text
+
+int* stack_addr_escape_base() {
+  int x = 0;
+  // FIXME: This shouldn't be tied to a modeling checker.
+  return &x; // expected-warning{{Address of stack memory associated with local variable 'x' returned to caller [core.StackAddrEscapeBase]}}
+  // expected-note-re@-1{{{{^Address of stack memory associated with local variable 'x' returned to caller$}}}}
+  // Just a regular compiler warning.
+  // expected-warning@-3{{address of stack memory associated with local variable 'x' returned}}
+}
+
diff --git a/clang/test/Analysis/incorrect-checker-names.mm b/clang/test/Analysis/incorrect-checker-names.mm
new file mode 100644 (file)
index 0000000..e81d5d3
--- /dev/null
@@ -0,0 +1,116 @@
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -Wno-objc-root-class \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=nullability \
+// RUN:   -analyzer-checker=osx
+
+#include "Inputs/system-header-simulator-for-nullability.h"
+#include "os_object_base.h"
+
+struct OSIterator : public OSObject {
+  static const OSMetaClass * const metaClass;
+};
+
+@interface TestObject : NSObject
+- (int *_Nonnull)returnsNonnull;
+- (int *_Nullable)returnsNullable;
+- (int *)returnsUnspecified;
+- (void)takesNonnull:(int *_Nonnull)p;
+- (void)takesNullable:(int *_Nullable)p;
+- (void)takesUnspecified:(int *)p;
+@property(readonly, strong) NSString *stuff;
+@end
+
+TestObject * getUnspecifiedTestObject();
+TestObject *_Nonnull getNonnullTestObject();
+TestObject *_Nullable getNullableTestObject();
+
+int getRandom();
+
+typedef struct Dummy { int val; } Dummy;
+
+void takesNullable(Dummy *_Nullable);
+void takesNonnull(Dummy *_Nonnull);
+void takesUnspecified(Dummy *);
+
+Dummy *_Nullable returnsNullable();
+Dummy *_Nonnull returnsNonnull();
+Dummy *returnsUnspecified();
+int *_Nullable returnsNullableInt();
+
+template <typename T> T *eraseNullab(T *p) { return p; }
+
+void takesAttrNonnull(Dummy *p) __attribute((nonnull(1)));
+
+void testBasicRules() {
+  // FIXME: None of these should be tied to a modeling checker.
+  Dummy *p = returnsNullable();
+  int *ptr = returnsNullableInt();
+  // Make every dereference a different path to avoid sinks after errors.
+  switch (getRandom()) {
+  case 0: {
+    Dummy &r = *p; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+  } break;
+  case 1: {
+    int b = p->val; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+  } break;
+  case 2: {
+    int stuff = *ptr; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+  } break;
+  case 3:
+    takesNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter [nullability.NullabilityBase]}}
+    break;
+  case 4: {
+    Dummy d;
+    takesNullable(&d);
+    Dummy dd(d);
+    break;
+  }
+  case 5: takesAttrNonnull(p); break; // expected-warning {{Nullable pointer is passed to a callee that requires a non-null [nullability.NullabilityBase]}}
+  default: { Dummy d = *p; } break; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+  }
+  if (p) {
+    takesNonnull(p);
+    if (getRandom()) {
+      Dummy &r = *p;
+    } else {
+      int b = p->val;
+    }
+  }
+  Dummy *q = 0;
+  if (getRandom()) {
+    takesNullable(q);
+  // FIXME: This shouldn't be tied to a modeling checker.
+    takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter [nullability.NullabilityBase]}}
+  }
+  Dummy a;
+  Dummy *_Nonnull nonnull = &a;
+  // FIXME: This shouldn't be tied to a modeling checker.
+  nonnull = q; // expected-warning {{Null assigned to a pointer which is expected to have non-null value [nullability.NullabilityBase]}}
+  q = &a;
+  takesNullable(q);
+  takesNonnull(q);
+}
+
+typedef int NSInteger;
+typedef struct _NSZone NSZone;
+@class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+@class NSDictionary;
+@interface NSError : NSObject <NSCopying, NSCoding> {}
++ (id)errorWithDomain:(NSString *)domain code:(NSInteger)code userInfo:(NSDictionary *)dict;
+@end
+
+struct __CFError {};
+typedef struct __CFError* CFErrorRef;
+
+void foo(CFErrorRef* error) { // expected-warning{{Function accepting CFErrorRef* should have a non-void return value to indicate whether or not an error occurred [osx.coreFoundation.CFError]}}
+  // FIXME: This shouldn't be tied to a modeling checker.
+  *error = 0;  // expected-warning {{Potential null dereference.  According to coding standards documented in CoreFoundation/CFError.h the parameter may be null [osx.NSOrCFErrorDerefChecker]}}
+}
+
+bool write_into_out_param_on_success(OS_RETURNS_RETAINED OSObject **obj);
+
+void use_out_param_leak() {
+  OSObject *obj;
+  // FIXME: This shouldn't be tied to a modeling checker.
+  write_into_out_param_on_success(&obj); // expected-warning{{Potential leak of an object stored into 'obj' [osx.cocoa.RetainCountBase]}}
+}