r222264 - [Sema] Patch to issue warning on comparing parameters with
Fariborz Jahanian
fjahanian at apple.com
Tue Nov 18 13:57:54 PST 2014
Author: fjahanian
Date: Tue Nov 18 15:57:54 2014
New Revision: 222264
URL: http://llvm.org/viewvc/llvm-project?rev=222264&view=rev
Log:
[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
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/lib/Sema/ScopeInfo.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/nonnull.c
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Nov 18 15:57:54 2014
@@ -2509,6 +2509,10 @@ def warn_impcast_pointer_to_bool : Warni
"address of%select{| function| array}0 '%1' will always evaluate to "
"'true'">,
InGroup<PointerBoolConversion>;
+def warn_cast_nonnull_to_bool : Warning<
+ "nonnull parameter '%0' will evaluate to "
+ "'true' on first encounter">,
+ InGroup<PointerBoolConversion>;
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<UndefinedBoolConversion>;
@@ -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<TautologicalPointerCompare>;
+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<TautologicalPointerCompare>;
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">,
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Tue Nov 18 15:57:54 2014
@@ -144,6 +144,10 @@ public:
/// current function scope. These diagnostics are vetted for reachability
/// prior to being emitted.
SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags;
+
+ /// \brief A list of parameters which have the nonnull attribute and are
+ /// modified in the function.
+ llvm::SmallPtrSet<const ParmVarDecl*, 8> ModifiedNonNullParams;
public:
/// Represents a simple identification of a weak object.
Modified: cfe/trunk/lib/Sema/ScopeInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/ScopeInfo.cpp?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/ScopeInfo.cpp (original)
+++ cfe/trunk/lib/Sema/ScopeInfo.cpp Tue Nov 18 15:57:54 2014
@@ -39,6 +39,7 @@ void FunctionScopeInfo::Clear() {
ErrorTrap.reset();
PossiblyUnreachableDiags.clear();
WeakObjectUses.clear();
+ ModifiedNonNullParams.clear();
}
static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Nov 18 15:57:54 2014
@@ -6739,7 +6739,39 @@ void Sema::DiagnoseAlwaysNonNullPointer(
// Weak Decls can be null.
if (!D || D->isWeak())
return;
-
+
+ // Check for parameter decl with nonnull attribute
+ if (const ParmVarDecl* PV = dyn_cast<ParmVarDecl>(D)) {
+ if (getCurFunction() && !getCurFunction()->ModifiedNonNullParams.count(PV))
+ if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {
+ unsigned NumArgs = FD->getNumParams();
+ llvm::SmallBitVector AttrNonNull(NumArgs);
+ for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {
+ 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();
Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Nov 18 15:57:54 2014
@@ -9121,6 +9121,24 @@ QualType Sema::CheckAddressOfOperand(Exp
return Context.getPointerType(op->getType());
}
+static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) {
+ const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Exp);
+ if (!DRE)
+ return;
+ const Decl *D = DRE->getDecl();
+ if (!D)
+ return;
+ const ParmVarDecl *Param = dyn_cast<ParmVarDecl>(D);
+ if (!Param)
+ return;
+ if (const FunctionDecl* FD = dyn_cast<FunctionDecl>(Param->getDeclContext()))
+ if (!FD->hasAttr<NonNullAttr>())
+ 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(Sour
}
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(So
break;
case UO_AddrOf:
resultType = CheckAddressOfOperand(Input, OpLoc);
+ RecordModifiableNonNullParam(*this, InputExpr);
break;
case UO_Deref: {
Input = DefaultFunctionArrayLvalueConversion(Input.get());
Modified: cfe/trunk/test/Sema/nonnull.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=222264&r1=222263&r2=222264&view=diff
==============================================================================
--- cfe/trunk/test/Sema/nonnull.c (original)
+++ cfe/trunk/test/Sema/nonnull.c Tue Nov 18 15:57:54 2014
@@ -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);
+ }
+}
+
More information about the cfe-commits
mailing list