vsavchenko created this revision.
vsavchenko added reviewers: NoQ, dcoughlin.
Herald added subscribers: cfe-commits, ASDenysPetrov, martong, Charusso,
dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, JDevlieghere,
szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.
We want to trust user type annotations and stop assuming pointers declared
as nonnull still can be null. This functionality is implemented as part
of NonNullParamChecker because it already checks parameter attributes.
Whenever we start analyzing a new function, we assume that all parameters
with 'nonnull' attribute are indeed non-null.
Depends on D77722 <https://reviews.llvm.org/D77722>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77806
Files:
clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
clang/test/Analysis/CheckNSError.m
clang/test/Analysis/nonnull.cpp
Index: clang/test/Analysis/nonnull.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/nonnull.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core -verify %s
+
+void nonnull [[gnu::nonnull]] (int *q);
+
+void f1(int *p) {
+ if (p)
+ return;
+ nonnull(p); //expected-warning{{nonnull}}
+}
+
+void f2(int *p) {
+ if (p)
+ return;
+ auto lambda = [](int *q) __attribute__((nonnull)){};
+ lambda(p); //expected-warning{{nonnull}}
+}
+
+template <class... ARGS>
+void variadicNonnull(ARGS... args) __attribute__((nonnull));
+
+void f3(int a, float b, int *p) {
+ if (p)
+ return;
+ variadicNonnull(a, b, p); //expected-warning{{nonnull}}
+}
Index: clang/test/Analysis/CheckNSError.m
===================================================================
--- clang/test/Analysis/CheckNSError.m
+++ clang/test/Analysis/CheckNSError.m
@@ -62,4 +62,17 @@
return 0;
}
+int __attribute__((nonnull)) f4(CFErrorRef *error) {
+ *error = 0; // no-warning
+ return 0;
+}
+int __attribute__((nonnull(1))) f5(int *x, CFErrorRef *error) {
+ *error = 0; // expected-warning {{Potential null dereference}}
+ return 0;
+}
+
+int __attribute__((nonnull(2))) f6(int *x, CFErrorRef *error) {
+ *error = 0; // no-warning
+ return 0;
+}
Index: clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -14,8 +14,9 @@
//
//===----------------------------------------------------------------------===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/AST/Attr.h"
+#include "clang/Analysis/AnyCall.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -28,44 +29,78 @@
namespace {
class NonNullParamChecker
- : public Checker< check::PreCall, EventDispatcher<ImplicitNullDerefEvent> > {
+ : public Checker<check::PreCall, check::BeginFunction,
+ EventDispatcher<ImplicitNullDerefEvent>> {
mutable std::unique_ptr<BugType> BTAttrNonNull;
mutable std::unique_ptr<BugType> BTNullRefArg;
public:
-
void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
+ void checkBeginFunction(CheckerContext &C) const;
std::unique_ptr<PathSensitiveBugReport>
- genReportNullAttrNonNull(const ExplodedNode *ErrorN,
- const Expr *ArgE,
+ genReportNullAttrNonNull(const ExplodedNode *ErrorN, const Expr *ArgE,
unsigned IdxOfArg) const;
std::unique_ptr<PathSensitiveBugReport>
genReportReferenceToNullPointer(const ExplodedNode *ErrorN,
const Expr *ArgE) const;
};
-} // end anonymous namespace
-/// \return Bitvector marking non-null attributes.
-static llvm::SmallBitVector getNonNullAttrs(const CallEvent &Call) {
+template <class CallType>
+void setBitsAccordingToFunctionAttributes(const CallType &Call,
+ llvm::SmallBitVector &AttrNonNull) {
const Decl *FD = Call.getDecl();
- unsigned NumArgs = Call.getNumArgs();
- llvm::SmallBitVector AttrNonNull(NumArgs);
+
for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
if (!NonNull->args_size()) {
- AttrNonNull.set(0, NumArgs);
+ // Lack of attribute parameters means that all of the parameters are
+ // implicitly marked as non-null.
+ AttrNonNull.set();
break;
}
+
for (const ParamIdx &Idx : NonNull->args()) {
+ // 'nonnull' attribute's parameters are 1-based and should be adjusted to
+ // match actual AST parameter/argument indices.
unsigned IdxAST = Idx.getASTIndex();
- if (IdxAST >= NumArgs)
+ if (IdxAST >= AttrNonNull.size())
continue;
AttrNonNull.set(IdxAST);
}
}
+}
+
+template <class CallType>
+void setBitsAccordingToParameterAttributes(const CallType &Call,
+ llvm::SmallBitVector &AttrNonNull) {
+ for (const ParmVarDecl *Parameter : Call.parameters()) {
+ if (Parameter->hasAttr<NonNullAttr>())
+ AttrNonNull.set(Parameter->getFunctionScopeIndex());
+ }
+}
+
+template <class CallType>
+llvm::SmallBitVector getNonNullAttrsImpl(const CallType &Call,
+ unsigned ExpectedSize) {
+ llvm::SmallBitVector AttrNonNull(ExpectedSize);
+
+ setBitsAccordingToFunctionAttributes(Call, AttrNonNull);
+ setBitsAccordingToParameterAttributes(Call, AttrNonNull);
+
return AttrNonNull;
}
+/// \return Bitvector marking non-null attributes.
+llvm::SmallBitVector getNonNullAttrs(const CallEvent &Call) {
+ return getNonNullAttrsImpl(Call, Call.getNumArgs());
+}
+
+/// \return Bitvector marking non-null attributes.
+llvm::SmallBitVector getNonNullAttrs(const AnyCall &Call) {
+ return getNonNullAttrsImpl(Call, Call.param_size());
+}
+} // end anonymous namespace
+
void NonNullParamChecker::checkPreCall(const CallEvent &Call,
CheckerContext &C) const {
if (!Call.getDecl())
@@ -75,7 +110,7 @@
unsigned NumArgs = Call.getNumArgs();
ProgramStateRef state = C.getState();
- ArrayRef<ParmVarDecl*> parms = Call.parameters();
+ ArrayRef<ParmVarDecl *> parms = Call.parameters();
for (unsigned idx = 0; idx < NumArgs; ++idx) {
// For vararg functions, a corresponding parameter decl may not exist.
@@ -83,15 +118,11 @@
// Check if the parameter is a reference. We want to report when reference
// to a null pointer is passed as a parameter.
- bool haveRefTypeParam =
+ bool HasRefTypeParam =
HasParam ? parms[idx]->getType()->isReferenceType() : false;
- bool haveAttrNonNull = AttrNonNull[idx];
-
- // Check if the parameter is also marked 'nonnull'.
- if (!haveAttrNonNull && HasParam)
- haveAttrNonNull = parms[idx]->hasAttr<NonNullAttr>();
+ bool ExpectedToBeNonNull = AttrNonNull.test(idx);
- if (!haveAttrNonNull && !haveRefTypeParam)
+ if (!ExpectedToBeNonNull && !HasRefTypeParam)
continue;
// If the value is unknown or undefined, we can't perform this check.
@@ -101,10 +132,10 @@
if (!DV)
continue;
- assert(!haveRefTypeParam || DV->getAs<Loc>());
+ assert(!HasRefTypeParam || DV->getAs<Loc>());
// Process the case when the argument is not a location.
- if (haveAttrNonNull && !DV->getAs<Loc>()) {
+ if (ExpectedToBeNonNull && !DV->getAs<Loc>()) {
// If the argument is a union type, we want to handle a potential
// transparent_union GCC extension.
if (!ArgE)
@@ -145,9 +176,9 @@
if (ExplodedNode *errorNode = C.generateErrorNode(stateNull)) {
std::unique_ptr<BugReport> R;
- if (haveAttrNonNull)
+ if (ExpectedToBeNonNull)
R = genReportNullAttrNonNull(errorNode, ArgE, idx + 1);
- else if (haveRefTypeParam)
+ else if (HasRefTypeParam)
R = genReportReferenceToNullPointer(errorNode, ArgE);
// Highlight the range of the argument that was null.
@@ -180,6 +211,54 @@
C.addTransition(state);
}
+/// We want to trust developer annotations and consider all 'nonnul' parameters
+/// as non-null indeed. Each marked parameter will get a corresponding
+/// constraint.
+///
+/// This approach will not only help us to get rid of some false positives, but
+/// remove duplicates and shorten warning traces as well.
+///
+/// \code
+/// void foo(int *x) [[gnu::nonnull]] {
+/// // . . .
+/// *x = 42; // we don't want to consider this as an error...
+/// // . . .
+/// }
+///
+/// foo(nullptr); // ...and report here instead
+/// \endcode
+void NonNullParamChecker::checkBeginFunction(CheckerContext &Context) const {
+ const LocationContext *LocContext = Context.getLocationContext();
+
+ const Decl *FD = LocContext->getDecl();
+ // AnyCall helps us here to avoid checking for FunctionDecl and ObjCMethodDecl
+ // separately and aggregates interfaces of these classes.
+ auto AbstractCall = AnyCall::forDecl(FD);
+ if (!AbstractCall)
+ return;
+
+ ProgramStateRef State = Context.getState();
+ llvm::SmallBitVector ParameterNonNullMarks = getNonNullAttrs(*AbstractCall);
+
+ for (const ParmVarDecl *Parameter : AbstractCall->parameters()) {
+ // 1. Check parameter if it is annotated as non-null
+ if (!ParameterNonNullMarks.test(Parameter->getFunctionScopeIndex()))
+ continue;
+
+ Loc ParameterLoc = State->getLValue(Parameter, LocContext);
+ auto StoredVal = State->getSVal(ParameterLoc).getAs<loc::MemRegionVal>();
+ if (!StoredVal)
+ continue;
+
+ // 2. Assume that it is indeed non-null
+ if (ProgramStateRef NewState = State->assume(*StoredVal, true)) {
+ State = NewState;
+ }
+ }
+
+ Context.addTransition(State);
+}
+
std::unique_ptr<PathSensitiveBugReport>
NonNullParamChecker::genReportNullAttrNonNull(const ExplodedNode *ErrorNode,
const Expr *ArgE,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
