Szelethus created this revision.
Szelethus added reviewers: NoQ, baloghadamsoftware, balazske, martong,
xazax.hun, dcoughlin, rnkovacs.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso,
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D76509: [analyzer][NFC] Move the text output
type to its own file, move code to PathDiagnosticConsumer creator functions.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D76605
Files:
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
clang/test/Analysis/incorrect-checker-names.mm
Index: clang/test/Analysis/incorrect-checker-names.mm
===================================================================
--- /dev/null
+++ 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;
+};
+
+@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]}}
+}
Index: clang/test/Analysis/incorrect-checker-names.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/incorrect-checker-names.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -verify %s -fblocks \
+// RUN: -analyzer-checker=core
+
+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]}}
+ // Just a regular compiler warning.
+ // expected-warning@-2{{address of stack memory associated with local variable 'x' returned}}
+}
+
Index: clang/test/Analysis/explain-svals.m
===================================================================
--- clang/test/Analysis/explain-svals.m
+++ 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"
Index: clang/test/Analysis/explain-svals.cpp
===================================================================
--- clang/test/Analysis/explain-svals.cpp
+++ 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;
Index: clang/test/Analysis/explain-svals.c
===================================================================
--- clang/test/Analysis/explain-svals.c
+++ 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;
Index: clang/test/Analysis/dispatch-once.m
===================================================================
--- clang/test/Analysis/dispatch-once.m
+++ 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"
Index: clang/test/Analysis/analyzer-config.c
===================================================================
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -51,6 +51,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
Index: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp
@@ -38,14 +38,15 @@
class TextDiagnostics : public PathDiagnosticConsumer {
DiagnosticsEngine &DiagEng;
const bool IncludePath = false, ShouldEmitAsError = false,
- FixitsAsRemarks = false;
+ FixitsAsRemarks = false, ShouldDisplayCheckerName = false;
public:
TextDiagnostics(DiagnosticsEngine &Diag, bool ShouldIncludePath,
const AnalyzerOptions &AnOpts)
: DiagEng(Diag), IncludePath(ShouldIncludePath),
ShouldEmitAsError(AnOpts.AnalyzerWerror),
- FixitsAsRemarks(AnOpts.ShouldEmitFixItHintsAsRemarks) {}
+ FixitsAsRemarks(AnOpts.ShouldEmitFixItHintsAsRemarks),
+ ShouldDisplayCheckerName(AnOpts.ShouldDisplayCheckerNameForText) {}
~TextDiagnostics() override {}
StringRef getName() const override { return "TextDiagnostics"; }
@@ -67,7 +68,7 @@
unsigned RemarkID =
DiagEng.getCustomDiagID(DiagnosticsEngine::Remark, "%0");
- auto reportPiece = [&](unsigned ID, SourceLocation Loc, StringRef String,
+ auto reportPiece = [&](unsigned ID, SourceLocation Loc, std::string String,
ArrayRef<SourceRange> Ranges,
ArrayRef<FixItHint> Fixits) {
if (!FixitsAsRemarks) {
@@ -94,9 +95,13 @@
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) {
@@ -104,7 +109,8 @@
continue;
reportPiece(NoteID, Piece->getLocation().asLocation(),
- Piece->getString(), Piece->getRanges(), Piece->getFixits());
+ Piece->getString().str(), Piece->getRanges(),
+ Piece->getFixits());
}
if (!IncludePath)
@@ -117,7 +123,8 @@
continue;
reportPiece(NoteID, Piece->getLocation().asLocation(),
- Piece->getString(), Piece->getRanges(), Piece->getFixits());
+ Piece->getString().str(), Piece->getRanges(),
+ Piece->getFixits());
}
}
}
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -310,6 +310,10 @@
"Emit fix-it hints as remarks for testing purposes",
false)
+ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
+ "Display the checker name for textual outputs",
+ true)
+
//===----------------------------------------------------------------------===//
// Unsigned analyzer options.
//===----------------------------------------------------------------------===//
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
