[clang] e4e1080 - [analyzer][Nullability] Don't emit under the checker name NullabilityBase
Kirstóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Tue May 19 08:04:21 PDT 2020
Author: Kirstóf Umann
Date: 2020-05-19T17:04:06+02:00
New Revision: e4e1080a5837e4a4f38c46b9974f2b2402bb021a
URL: https://github.com/llvm/llvm-project/commit/e4e1080a5837e4a4f38c46b9974f2b2402bb021a
DIFF: https://github.com/llvm/llvm-project/commit/e4e1080a5837e4a4f38c46b9974f2b2402bb021a.diff
LOG: [analyzer][Nullability] Don't emit under the checker name NullabilityBase
Differential Revision: https://reviews.llvm.org/D78122
Added:
Modified:
clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
clang/test/Analysis/incorrect-checker-names.mm
Removed:
################################################################################
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index 565d0eeef704..bc7a8a3b12a1 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -82,7 +82,6 @@ class NullabilityChecker
check::PostCall, check::PostStmt<ExplicitCastExpr>,
check::PostObjCMessage, check::DeadSymbols,
check::Location, check::Event<ImplicitNullDerefEvent>> {
- mutable std::unique_ptr<BugType> BT;
public:
// If true, the checker will not diagnose nullabilility issues for calls
@@ -107,21 +106,26 @@ class NullabilityChecker
void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
const char *Sep) const override;
- struct NullabilityChecksFilter {
- DefaultBool CheckNullPassedToNonnull;
- DefaultBool CheckNullReturnedFromNonnull;
- DefaultBool CheckNullableDereferenced;
- DefaultBool CheckNullablePassedToNonnull;
- DefaultBool CheckNullableReturnedFromNonnull;
-
- CheckerNameRef CheckNameNullPassedToNonnull;
- CheckerNameRef CheckNameNullReturnedFromNonnull;
- CheckerNameRef CheckNameNullableDereferenced;
- CheckerNameRef CheckNameNullablePassedToNonnull;
- CheckerNameRef CheckNameNullableReturnedFromNonnull;
+ enum CheckKind {
+ CK_NullPassedToNonnull,
+ CK_NullReturnedFromNonnull,
+ CK_NullableDereferenced,
+ CK_NullablePassedToNonnull,
+ CK_NullableReturnedFromNonnull,
+ CK_NumCheckKinds
};
- NullabilityChecksFilter Filter;
+ DefaultBool ChecksEnabled[CK_NumCheckKinds];
+ CheckerNameRef CheckNames[CK_NumCheckKinds];
+ mutable std::unique_ptr<BugType> BTs[CK_NumCheckKinds];
+
+ const std::unique_ptr<BugType> &getBugType(CheckKind Kind) const {
+ if (!BTs[Kind])
+ BTs[Kind].reset(new BugType(CheckNames[Kind], "Nullability",
+ categories::MemoryError));
+ return BTs[Kind];
+ }
+
// When set to false no nullability information will be tracked in
// NullabilityMap. It is possible to catch errors like passing a null pointer
// to a callee that expects nonnull argument without the information that is
@@ -153,18 +157,16 @@ class NullabilityChecker
///
/// When \p SuppressPath is set to true, no more bugs will be reported on this
/// path by this checker.
- void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error,
+ void reportBugIfInvariantHolds(StringRef Msg, ErrorKind Error, CheckKind CK,
ExplodedNode *N, const MemRegion *Region,
CheckerContext &C,
const Stmt *ValueExpr = nullptr,
- bool SuppressPath = false) const;
+ bool SuppressPath = false) const;
- void reportBug(StringRef Msg, ErrorKind Error, ExplodedNode *N,
+ void reportBug(StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
const MemRegion *Region, BugReporter &BR,
const Stmt *ValueExpr = nullptr) const {
- if (!BT)
- BT.reset(new BugType(this, "Nullability", categories::MemoryError));
-
+ const std::unique_ptr<BugType> &BT = getBugType(CK);
auto R = std::make_unique<PathSensitiveBugReport>(*BT, Msg, N);
if (Region) {
R->markInteresting(Region);
@@ -432,9 +434,10 @@ static bool checkInvariantViolation(ProgramStateRef State, ExplodedNode *N,
return false;
}
-void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg,
- ErrorKind Error, ExplodedNode *N, const MemRegion *Region,
- CheckerContext &C, const Stmt *ValueExpr, bool SuppressPath) const {
+void NullabilityChecker::reportBugIfInvariantHolds(
+ StringRef Msg, ErrorKind Error, CheckKind CK, ExplodedNode *N,
+ const MemRegion *Region, CheckerContext &C, const Stmt *ValueExpr,
+ bool SuppressPath) const {
ProgramStateRef OriginalState = N->getState();
if (checkInvariantViolation(OriginalState, N, C))
@@ -444,7 +447,7 @@ void NullabilityChecker::reportBugIfInvariantHolds(StringRef Msg,
N = C.addTransition(OriginalState, N);
}
- reportBug(Msg, Error, N, Region, C.getBugReporter(), ValueExpr);
+ reportBug(Msg, Error, CK, N, Region, C.getBugReporter(), ValueExpr);
}
/// Cleaning up the program state.
@@ -489,17 +492,19 @@ void NullabilityChecker::checkEvent(ImplicitNullDerefEvent Event) const {
if (!TrackedNullability)
return;
- if (Filter.CheckNullableDereferenced &&
+ if (ChecksEnabled[CK_NullableDereferenced] &&
TrackedNullability->getValue() == Nullability::Nullable) {
BugReporter &BR = *Event.BR;
// Do not suppress errors on defensive code paths, because dereferencing
// a nullable pointer is always an error.
if (Event.IsDirectDereference)
reportBug("Nullable pointer is dereferenced",
- ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR);
+ ErrorKind::NullableDereferenced, CK_NullableDereferenced,
+ Event.SinkNode, Region, BR);
else {
reportBug("Nullable pointer is passed to a callee that requires a "
- "non-null", ErrorKind::NullablePassedToNonnull,
+ "non-null",
+ ErrorKind::NullablePassedToNonnull, CK_NullableDereferenced,
Event.SinkNode, Region, BR);
}
}
@@ -614,11 +619,9 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
bool NullReturnedFromNonNull = (RequiredNullability == Nullability::Nonnull &&
Nullness == NullConstraint::IsNull);
- if (Filter.CheckNullReturnedFromNonnull &&
- NullReturnedFromNonNull &&
+ if (ChecksEnabled[CK_NullReturnedFromNonnull] && NullReturnedFromNonNull &&
RetExprTypeLevelNullability != Nullability::Nonnull &&
- !InSuppressedMethodFamily &&
- C.getLocationContext()->inTopFrame()) {
+ !InSuppressedMethodFamily && C.getLocationContext()->inTopFrame()) {
static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
ExplodedNode *N = C.generateErrorNode(State, &Tag);
if (!N)
@@ -629,8 +632,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
OS << (RetExpr->getType()->isObjCObjectPointerType() ? "nil" : "Null");
OS << " returned from a " << C.getDeclDescription(D) <<
" that is expected to return a non-null value";
- reportBugIfInvariantHolds(OS.str(),
- ErrorKind::NilReturnedToNonnull, N, nullptr, C,
+ reportBugIfInvariantHolds(OS.str(), ErrorKind::NilReturnedToNonnull,
+ CK_NullReturnedFromNonnull, N, nullptr, C,
RetExpr);
return;
}
@@ -651,7 +654,7 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
State->get<NullabilityMap>(Region);
if (TrackedNullability) {
Nullability TrackedNullabValue = TrackedNullability->getValue();
- if (Filter.CheckNullableReturnedFromNonnull &&
+ if (ChecksEnabled[CK_NullableReturnedFromNonnull] &&
Nullness != NullConstraint::IsNotNull &&
TrackedNullabValue == Nullability::Nullable &&
RequiredNullability == Nullability::Nonnull) {
@@ -663,9 +666,8 @@ void NullabilityChecker::checkPreStmt(const ReturnStmt *S,
OS << "Nullable pointer is returned from a " << C.getDeclDescription(D) <<
" that is expected to return a non-null value";
- reportBugIfInvariantHolds(OS.str(),
- ErrorKind::NullableReturnedToNonnull, N,
- Region, C);
+ reportBugIfInvariantHolds(OS.str(), ErrorKind::NullableReturnedToNonnull,
+ CK_NullableReturnedFromNonnull, N, Region, C);
}
return;
}
@@ -716,7 +718,8 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
unsigned ParamIdx = Param->getFunctionScopeIndex() + 1;
- if (Filter.CheckNullPassedToNonnull && Nullness == NullConstraint::IsNull &&
+ if (ChecksEnabled[CK_NullPassedToNonnull] &&
+ Nullness == NullConstraint::IsNull &&
ArgExprTypeLevelNullability != Nullability::Nonnull &&
RequiredNullability == Nullability::Nonnull &&
isDiagnosableCall(Call)) {
@@ -729,9 +732,9 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
OS << (Param->getType()->isObjCObjectPointerType() ? "nil" : "Null");
OS << " passed to a callee that requires a non-null " << ParamIdx
<< llvm::getOrdinalSuffix(ParamIdx) << " parameter";
- reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull, N,
- nullptr, C,
- ArgExpr, /*SuppressPath=*/false);
+ reportBugIfInvariantHolds(OS.str(), ErrorKind::NilPassedToNonnull,
+ CK_NullPassedToNonnull, N, nullptr, C, ArgExpr,
+ /*SuppressPath=*/false);
return;
}
@@ -747,7 +750,7 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
TrackedNullability->getValue() != Nullability::Nullable)
continue;
- if (Filter.CheckNullablePassedToNonnull &&
+ if (ChecksEnabled[CK_NullablePassedToNonnull] &&
RequiredNullability == Nullability::Nonnull &&
isDiagnosableCall(Call)) {
ExplodedNode *N = C.addTransition(State);
@@ -755,17 +758,18 @@ void NullabilityChecker::checkPreCall(const CallEvent &Call,
llvm::raw_svector_ostream OS(SBuf);
OS << "Nullable pointer is passed to a callee that requires a non-null "
<< ParamIdx << llvm::getOrdinalSuffix(ParamIdx) << " parameter";
- reportBugIfInvariantHolds(OS.str(),
- ErrorKind::NullablePassedToNonnull, N,
- Region, C, ArgExpr, /*SuppressPath=*/true);
+ reportBugIfInvariantHolds(OS.str(), ErrorKind::NullablePassedToNonnull,
+ CK_NullablePassedToNonnull, N, Region, C,
+ ArgExpr, /*SuppressPath=*/true);
return;
}
- if (Filter.CheckNullableDereferenced &&
+ if (ChecksEnabled[CK_NullableDereferenced] &&
Param->getType()->isReferenceType()) {
ExplodedNode *N = C.addTransition(State);
reportBugIfInvariantHolds("Nullable pointer is dereferenced",
- ErrorKind::NullableDereferenced, N, Region,
- C, ArgExpr, /*SuppressPath=*/true);
+ ErrorKind::NullableDereferenced,
+ CK_NullableDereferenced, N, Region, C,
+ ArgExpr, /*SuppressPath=*/true);
return;
}
continue;
@@ -1125,8 +1129,7 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
bool NullAssignedToNonNull = (LocNullability == Nullability::Nonnull &&
RhsNullness == NullConstraint::IsNull);
- if (Filter.CheckNullPassedToNonnull &&
- NullAssignedToNonNull &&
+ if (ChecksEnabled[CK_NullPassedToNonnull] && NullAssignedToNonNull &&
ValNullability != Nullability::Nonnull &&
ValueExprTypeLevelNullability != Nullability::Nonnull &&
!isARCNilInitializedLocal(C, S)) {
@@ -1144,9 +1147,8 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
llvm::raw_svector_ostream OS(SBuf);
OS << (LocType->isObjCObjectPointerType() ? "nil" : "Null");
OS << " assigned to a pointer which is expected to have non-null value";
- reportBugIfInvariantHolds(OS.str(),
- ErrorKind::NilAssignedToNonnull, N, nullptr, C,
- ValueStmt);
+ reportBugIfInvariantHolds(OS.str(), ErrorKind::NilAssignedToNonnull,
+ CK_NullPassedToNonnull, N, nullptr, C, ValueStmt);
return;
}
@@ -1172,14 +1174,14 @@ void NullabilityChecker::checkBind(SVal L, SVal V, const Stmt *S,
if (RhsNullness == NullConstraint::IsNotNull ||
TrackedNullability->getValue() != Nullability::Nullable)
return;
- if (Filter.CheckNullablePassedToNonnull &&
+ if (ChecksEnabled[CK_NullablePassedToNonnull] &&
LocNullability == Nullability::Nonnull) {
static CheckerProgramPointTag Tag(this, "NullablePassedToNonnull");
ExplodedNode *N = C.addTransition(State, C.getPredecessor(), &Tag);
reportBugIfInvariantHolds("Nullable pointer is assigned to a pointer "
"which is expected to have non-null value",
- ErrorKind::NullableAssignedToNonnull, N,
- ValueRegion, C);
+ ErrorKind::NullableAssignedToNonnull,
+ CK_NullablePassedToNonnull, N, ValueRegion, C);
}
return;
}
@@ -1237,8 +1239,9 @@ bool ento::shouldRegisterNullabilityBase(const CheckerManager &mgr) {
#define REGISTER_CHECKER(name, trackingRequired) \
void ento::register##name##Checker(CheckerManager &mgr) { \
NullabilityChecker *checker = mgr.getChecker<NullabilityChecker>(); \
- checker->Filter.Check##name = true; \
- checker->Filter.CheckName##name = mgr.getCurrentCheckerName(); \
+ checker->ChecksEnabled[NullabilityChecker::CK_##name] = true; \
+ checker->CheckNames[NullabilityChecker::CK_##name] = \
+ mgr.getCurrentCheckerName(); \
checker->NeedTracking = checker->NeedTracking || trackingRequired; \
checker->NoDiagnoseCallsToSystemHeaders = \
checker->NoDiagnoseCallsToSystemHeaders || \
@@ -1246,7 +1249,7 @@ bool ento::shouldRegisterNullabilityBase(const CheckerManager &mgr) {
checker, "NoDiagnoseCallsToSystemHeaders", true); \
} \
\
- bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \
+ bool ento::shouldRegister##name##Checker(const CheckerManager &mgr) { \
return true; \
}
diff --git a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
index 88cf2fa882ad..3f38abb6c920 100644
--- a/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
+++ b/clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
@@ -206,9 +206,9 @@
<key>description</key><string>Nullable pointer is passed to a callee that requires a non-null 1st parameter</string>
<key>category</key><string>Memory error</string>
<key>type</key><string>Nullability</string>
- <key>check_name</key><string>nullability.NullabilityBase</string>
+ <key>check_name</key><string>nullability.NullablePassedToNonnull</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>ff735bea0eb12d4d172b139143c32365</string>
+ <key>issue_hash_content_of_line_in_context</key><string>cff860dfd96389c0fc88f9c04b609f87</string>
<key>issue_context_kind</key><string>Objective-C method</string>
<key>issue_context</key><string>method</string>
<key>issue_hash_function_offset</key><string>6</string>
diff --git a/clang/test/Analysis/incorrect-checker-names.mm b/clang/test/Analysis/incorrect-checker-names.mm
index e81d5d39a041..f14eea9d9c63 100644
--- a/clang/test/Analysis/incorrect-checker-names.mm
+++ b/clang/test/Analysis/incorrect-checker-names.mm
@@ -48,16 +48,16 @@ void testBasicRules() {
// 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]}}
+ Dummy &r = *p; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}}
} break;
case 1: {
- int b = p->val; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+ int b = p->val; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}}
} break;
case 2: {
- int stuff = *ptr; // expected-warning {{Nullable pointer is dereferenced [nullability.NullabilityBase]}}
+ int stuff = *ptr; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}}
} break;
case 3:
- takesNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter [nullability.NullabilityBase]}}
+ takesNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null 1st parameter [nullability.NullablePassedToNonnull]}}
break;
case 4: {
Dummy d;
@@ -65,8 +65,10 @@ void testBasicRules() {
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]}}
+ case 5:
+ takesAttrNonnull(p); // expected-warning {{Nullable pointer is passed to a callee that requires a non-null [nullability.NullableDereferenced]}}
+ break;
+ default: { Dummy d = *p; } break; // expected-warning {{Nullable pointer is dereferenced [nullability.NullableDereferenced]}}
}
if (p) {
takesNonnull(p);
@@ -80,12 +82,12 @@ void testBasicRules() {
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]}}
+ takesNonnull(q); // expected-warning {{Null passed to a callee that requires a non-null 1st parameter [nullability.NullPassedToNonnull]}}
}
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]}}
+ nonnull = q; // expected-warning {{Null assigned to a pointer which is expected to have non-null value [nullability.NullPassedToNonnull]}}
q = &a;
takesNullable(q);
takesNonnull(q);
More information about the cfe-commits
mailing list