Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td @@ -2727,7 +2727,7 @@ "'true'">, InGroup; def warn_cast_nonnull_to_bool : Warning< - "nonnull parameter '%0' will evaluate to " + "nonnull %select{function call|parameter}0 '%1' will evaluate to " "'true' on first encounter">, InGroup; def warn_this_bool_conversion : Warning< @@ -2742,9 +2742,10 @@ "comparison of %select{address of|function|array}0 '%1' %select{not |}2" "equal to a null pointer is always %select{true|false}2">, InGroup; -def warn_nonnull_parameter_compare : Warning< - "comparison of nonnull parameter '%0' %select{not |}1" - "equal to a null pointer is %select{true|false}1 on first encounter">, +def warn_nonnull_expr_compare : Warning< + "comparison of nonnull %select{function call|parameter}0 '%1' " + "%select{not |}2equal to a null pointer is '%select{true|false}2' on first " + "encounter">, InGroup; def warn_this_null_compare : Warning< "'this' pointer cannot be null in well-defined C++ code; comparison may be " Index: cfe/trunk/include/clang/Sema/ScopeInfo.h =================================================================== --- cfe/trunk/include/clang/Sema/ScopeInfo.h +++ cfe/trunk/include/clang/Sema/ScopeInfo.h @@ -164,7 +164,7 @@ /// \brief A list of parameters which have the nonnull attribute and are /// modified in the function. - llvm::SmallPtrSet ModifiedNonNullParams; + llvm::SmallPtrSet ModifiedNonNullParams; public: /// Represents a simple identification of a weak object. Index: cfe/trunk/lib/Sema/SemaChecking.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -1180,8 +1180,7 @@ /// Checks if a the given expression evaluates to null. /// /// \brief Returns true if the value evaluates to null. -static bool CheckNonNullExpr(Sema &S, - const Expr *Expr) { +static bool CheckNonNullExpr(Sema &S, const Expr *Expr) { // If the expression has non-null type, it doesn't evaluate to null. if (auto nullability = Expr->IgnoreImplicit()->getType()->getNullability(S.Context)) { @@ -7666,6 +7665,26 @@ } } + auto ComplainAboutNonnullParamOrCall = [&](bool IsParam) { + std::string Str; + llvm::raw_string_ostream S(Str); + E->printPretty(S, nullptr, getPrintingPolicy()); + unsigned DiagID = IsCompare ? diag::warn_nonnull_expr_compare + : diag::warn_cast_nonnull_to_bool; + Diag(E->getExprLoc(), DiagID) << IsParam << S.str() + << E->getSourceRange() << Range << IsEqual; + }; + + // If we have a CallExpr that is tagged with returns_nonnull, we can complain. + if (auto *Call = dyn_cast(E->IgnoreParenImpCasts())) { + if (auto *Callee = Call->getDirectCallee()) { + if (Callee->hasAttr()) { + ComplainAboutNonnullParamOrCall(false); + return; + } + } + } + // Expect to find a single Decl. Skip anything more complicated. ValueDecl *D = nullptr; if (DeclRefExpr *R = dyn_cast(E)) { @@ -7677,40 +7696,38 @@ // Weak Decls can be null. if (!D || D->isWeak()) return; - + // Check for parameter decl with nonnull attribute - if (const ParmVarDecl* PV = dyn_cast(D)) { - if (getCurFunction() && !getCurFunction()->ModifiedNonNullParams.count(PV)) - if (const FunctionDecl* FD = dyn_cast(PV->getDeclContext())) { - unsigned NumArgs = FD->getNumParams(); - llvm::SmallBitVector AttrNonNull(NumArgs); + if (const auto* PV = dyn_cast(D)) { + if (getCurFunction() && + !getCurFunction()->ModifiedNonNullParams.count(PV)) { + if (PV->hasAttr()) { + ComplainAboutNonnullParamOrCall(true); + return; + } + + if (const auto *FD = dyn_cast(PV->getDeclContext())) { + auto ParamIter = std::find(FD->param_begin(), FD->param_end(), PV); + assert(ParamIter != FD->param_end()); + unsigned ParamNo = std::distance(FD->param_begin(), ParamIter); + for (const auto *NonNull : FD->specific_attrs()) { if (!NonNull->args_size()) { - AttrNonNull.set(0, NumArgs); - break; - } - for (unsigned Val : NonNull->args()) { - if (Val >= NumArgs) - continue; - AttrNonNull.set(Val); + ComplainAboutNonnullParamOrCall(true); + return; } - } - if (!AttrNonNull.empty()) - for (unsigned i = 0; i < NumArgs; ++i) - if (FD->getParamDecl(i) == PV && - (AttrNonNull[i] || PV->hasAttr())) { - std::string Str; - llvm::raw_string_ostream S(Str); - E->printPretty(S, nullptr, getPrintingPolicy()); - unsigned DiagID = IsCompare ? diag::warn_nonnull_parameter_compare - : diag::warn_cast_nonnull_to_bool; - Diag(E->getExprLoc(), DiagID) << S.str() << E->getSourceRange() - << Range << IsEqual; + + for (unsigned ArgNo : NonNull->args()) { + if (ArgNo == ParamNo) { + ComplainAboutNonnullParamOrCall(true); return; } + } + } } } - + } + QualType T = D->getType(); const bool IsArray = T->isArrayType(); const bool IsFunction = T->isFunctionType(); Index: cfe/trunk/test/Sema/nonnull.c =================================================================== --- cfe/trunk/test/Sema/nonnull.c +++ cfe/trunk/test/Sema/nonnull.c @@ -89,7 +89,7 @@ __attribute__((__nonnull__)) int evil_nonnull_func(int* pointer, void * pv) { - if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is false on first encounter}} + if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is 'false' on first encounter}} return 0; } else { return *pointer; @@ -101,13 +101,13 @@ else return *pointer; - if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is false on first encounter}} + if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is 'false' on first encounter}} } void set_param_to_null(int**); int another_evil_nonnull_func(int* pointer, char ch, void * pv) __attribute__((nonnull(1, 3))); int another_evil_nonnull_func(int* pointer, char ch, void * pv) { - if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is false on first encounter}} + if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is 'false' on first encounter}} return 0; } else { return *pointer; @@ -119,7 +119,7 @@ else return *pointer; - if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is false on first encounter}} + if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is 'false' on first encounter}} } extern void *returns_null(void**); @@ -153,3 +153,17 @@ if (p) // No warning ; } + +__attribute__((returns_nonnull)) void *returns_nonnull_whee(); + +void returns_nonnull_warning_tests() { + if (returns_nonnull_whee() == NULL) {} // expected-warning {{comparison of nonnull function call 'returns_nonnull_whee()' equal to a null pointer is 'false' on first encounter}} + + if (returns_nonnull_whee() != NULL) {} // expected-warning {{comparison of nonnull function call 'returns_nonnull_whee()' not equal to a null pointer is 'true' on first encounter}} + + if (returns_nonnull_whee()) {} // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}} + if (!returns_nonnull_whee()) {} // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}} + + int and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}} + and_again = !returns_nonnull_whee(); // expected-warning {{nonnull function call 'returns_nonnull_whee()' will evaluate to 'true' on first encounter}} +}