From 9699ab445ae007a5a6b32981da73baed77600671 Mon Sep 17 00:00:00 2001 From: Fariborz Jahanian Date: Tue, 18 Nov 2014 21:57:54 +0000 Subject: [PATCH] [Sema] Patch to issue warning on comparing parameters with nonnull attribute when comparison is always true/false. Original patch by Steven Wu. I have added extra code to prevent issuing of warning when the nonnull parameter is modified prior to the comparison. This addition prevents false positives in the most obvious cases. There may still be false positive warnings in some cases (as one of my tests indicates), but benefit far outweighs such cases. rdar://18712242 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@222264 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 8 +++ include/clang/Sema/ScopeInfo.h | 4 ++ lib/Sema/ScopeInfo.cpp | 1 + lib/Sema/SemaChecking.cpp | 34 ++++++++++++- lib/Sema/SemaExpr.cpp | 20 ++++++++ test/Sema/nonnull.c | 58 ++++++++++++++++++++++ 6 files changed, 124 insertions(+), 1 deletion(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d01a2d62bd..aa93e63a4d 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2509,6 +2509,10 @@ def warn_impcast_pointer_to_bool : Warning< "address of%select{| function| array}0 '%1' will always evaluate to " "'true'">, InGroup; +def warn_cast_nonnull_to_bool : Warning< + "nonnull parameter '%0' will evaluate to " + "'true' on first encounter">, + InGroup; def warn_this_bool_conversion : Warning< "'this' pointer cannot be null in well-defined C++ code; pointer may be " "assumed to always convert to true">, InGroup; @@ -2521,6 +2525,10 @@ def warn_null_pointer_compare : Warning< "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">, + InGroup; def warn_this_null_compare : Warning< "'this' pointer cannot be null in well-defined C++ code; comparison may be " "assumed to always evaluate to %select{true|false}0">, diff --git a/include/clang/Sema/ScopeInfo.h b/include/clang/Sema/ScopeInfo.h index c0b6df40aa..d63b734a8d 100644 --- a/include/clang/Sema/ScopeInfo.h +++ b/include/clang/Sema/ScopeInfo.h @@ -144,6 +144,10 @@ public: /// current function scope. These diagnostics are vetted for reachability /// prior to being emitted. SmallVector PossiblyUnreachableDiags; + + /// \brief A list of parameters which have the nonnull attribute and are + /// modified in the function. + llvm::SmallPtrSet ModifiedNonNullParams; public: /// Represents a simple identification of a weak object. diff --git a/lib/Sema/ScopeInfo.cpp b/lib/Sema/ScopeInfo.cpp index c4bf67b76c..00d9982ac0 100644 --- a/lib/Sema/ScopeInfo.cpp +++ b/lib/Sema/ScopeInfo.cpp @@ -39,6 +39,7 @@ void FunctionScopeInfo::Clear() { ErrorTrap.reset(); PossiblyUnreachableDiags.clear(); WeakObjectUses.clear(); + ModifiedNonNullParams.clear(); } static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) { diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 72fc3e3749..aa6bf1760d 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -6739,7 +6739,39 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E, // 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); + 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); + } + } + if (!AttrNonNull.empty()) + for (unsigned i = 0; i < NumArgs; ++i) + if (FD->getParamDecl(i) == PV && AttrNonNull[i]) { + 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; + return; + } + } + } + QualType T = D->getType(); const bool IsArray = T->isArrayType(); const bool IsFunction = T->isFunctionType(); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 1429df778b..0104020bba 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -9121,6 +9121,24 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { return Context.getPointerType(op->getType()); } +static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) { + const DeclRefExpr *DRE = dyn_cast(Exp); + if (!DRE) + return; + const Decl *D = DRE->getDecl(); + if (!D) + return; + const ParmVarDecl *Param = dyn_cast(D); + if (!Param) + return; + if (const FunctionDecl* FD = dyn_cast(Param->getDeclContext())) + if (!FD->hasAttr()) + return; + if (FunctionScopeInfo *FD = S.getCurFunction()) + if (!FD->ModifiedNonNullParams.count(Param)) + FD->ModifiedNonNullParams.insert(Param); +} + /// CheckIndirectionOperand - Type check unary indirection (prefix '*'). static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK, SourceLocation OpLoc) { @@ -9360,6 +9378,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, } if (!ResultTy.isNull()) DiagnoseSelfAssignment(*this, LHS.get(), RHS.get(), OpLoc); + RecordModifiableNonNullParam(*this, LHS.get()); break; case BO_PtrMemD: case BO_PtrMemI: @@ -9833,6 +9852,7 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc, break; case UO_AddrOf: resultType = CheckAddressOfOperand(Input, OpLoc); + RecordModifiableNonNullParam(*this, InputExpr); break; case UO_Deref: { Input = DefaultFunctionArrayLvalueConversion(Input.get()); diff --git a/test/Sema/nonnull.c b/test/Sema/nonnull.c index 3b654a8890..de47e9664c 100644 --- a/test/Sema/nonnull.c +++ b/test/Sema/nonnull.c @@ -83,3 +83,61 @@ void redecl_test(void *p) { redecl(p, 0); // expected-warning{{null passed}} redecl(0, p); // expected-warning{{null passed}} } + +// rdar://18712242 +#define NULL (void*)0 +__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}} + return 0; + } else { + return *pointer; + } + + pointer = pv; + if (!pointer) + return 0; + else + return *pointer; + + 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}} + return 0; + } else { + return *pointer; + } + + set_param_to_null(&pointer); + if (!pointer) + return 0; + else + return *pointer; + + 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**); +extern void FOO(); +extern void FEE(); + +extern void *pv; +__attribute__((__nonnull__)) +void yet_another_evil_nonnull_func(int* pointer) +{ + while (pv) { + // This comparison will not be optimized away. + if (pointer) { // expected-warning {{nonnull parameter 'pointer' will evaluate to 'true' on first encounter}} + FOO(); + } else { + FEE(); + } + pointer = returns_null(&pv); + } +} + -- 2.40.0