[clang] a2b6ece - [analyzer] Display the checker name in the text output

Kirstóf Umann via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 9 07:22:10 PDT 2020


Author: Kirstóf Umann
Date: 2020-04-09T16:21:45+02:00
New Revision: a2b6ece1fd4e13f2f463c5075eb0e513af47e656

URL: https://github.com/llvm/llvm-project/commit/a2b6ece1fd4e13f2f463c5075eb0e513af47e656
DIFF: https://github.com/llvm/llvm-project/commit/a2b6ece1fd4e13f2f463c5075eb0e513af47e656.diff

LOG: [analyzer] Display the checker name in the text output

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

Added: 
    clang/test/Analysis/incorrect-checker-names.cpp
    clang/test/Analysis/incorrect-checker-names.mm

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index ee67f60df948..597a65c21318 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -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.
 //===----------------------------------------------------------------------===//

diff  --git a/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
index 07e5685e604c..f4c7e5978e19 100644
--- a/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
@@ -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 @@ class TextDiagnostics : public PathDiagnosticConsumer {
          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 @@ class TextDiagnostics : public PathDiagnosticConsumer {
           continue;
 
         reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+                    Piece->getString(), Piece->getRanges(),
+                    Piece->getFixits());
       }
 
       if (!IncludePath)
@@ -113,7 +120,8 @@ class TextDiagnostics : public PathDiagnosticConsumer {
           continue;
 
         reportPiece(NoteID, Piece->getLocation().asLocation(),
-                    Piece->getString(), Piece->getRanges(), Piece->getFixits());
+                    Piece->getString(), Piece->getRanges(),
+                    Piece->getFixits());
       }
     }
 

diff  --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 068930e6e57f..7069ee4a938d 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/clang/test/Analysis/analyzer-config.c
@@ -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
@@ -100,4 +101,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 97
+// CHECK-NEXT: num-entries = 98

diff  --git a/clang/test/Analysis/dispatch-once.m b/clang/test/Analysis/dispatch-once.m
index 7314dc96d541..b8cf582ba468 100644
--- a/clang/test/Analysis/dispatch-once.m
+++ b/clang/test/Analysis/dispatch-once.m
@@ -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"
 

diff  --git a/clang/test/Analysis/explain-svals.c b/clang/test/Analysis/explain-svals.c
index f1540bbe2de8..204df7cd5c0a 100644
--- a/clang/test/Analysis/explain-svals.c
+++ b/clang/test/Analysis/explain-svals.c
@@ -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;

diff  --git a/clang/test/Analysis/explain-svals.cpp b/clang/test/Analysis/explain-svals.cpp
index 9c37642758bb..f8ec23468af4 100644
--- a/clang/test/Analysis/explain-svals.cpp
+++ b/clang/test/Analysis/explain-svals.cpp
@@ -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;
 

diff  --git a/clang/test/Analysis/explain-svals.m b/clang/test/Analysis/explain-svals.m
index dd40946c8432..b2efebf71f11 100644
--- a/clang/test/Analysis/explain-svals.m
+++ b/clang/test/Analysis/explain-svals.m
@@ -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
index 000000000000..3519e1509d10
--- /dev/null
+++ b/clang/test/Analysis/incorrect-checker-names.cpp
@@ -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 at -1{{{{^Address of stack memory associated with local variable 'x' returned to caller$}}}}
+  // Just a regular compiler warning.
+  // expected-warning at -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
index 000000000000..e81d5d39a041
--- /dev/null
+++ b/clang/test/Analysis/incorrect-checker-names.mm
@@ -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;
+};
+
+ at 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;
+ at property(readonly, strong) NSString *stuff;
+ at 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 
diff erent 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;
+ at class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+ at class NSDictionary;
+ at interface NSError : NSObject <NSCopying, NSCoding> {}
++ (id)errorWithDomain:(NSString *)domain code:(NSInteger)code userInfo:(NSDictionary *)dict;
+ at 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]}}
+}


        


More information about the cfe-commits mailing list