<div dir="ltr">Sorry for the duplicate email. Once more, with the list this time.<br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 23, 2014 at 12:00 PM, Fariborz Jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: fjahanian<br>
Date: Thu Oct 23 14:00:10 2014<br>
New Revision: 220496<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=220496&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=220496&view=rev</a><br>
Log:<br>
patch to issue warning on comparing parameters with<br>
nonnull attribute when comparison is always<br>
true/false. Patch by Steven Wu with few fixes and minor<br>
refactoring and adding tests by me. rdar://18712242<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
cfe/trunk/lib/Sema/SemaChecking.cpp<br>
cfe/trunk/test/Sema/nonnull.c<br>
<br>
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=220496&r1=220495&r2=220496&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=220496&r1=220495&r2=220496&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct 23 14:00:10 2014<br>
@@ -2500,6 +2500,10 @@ def warn_impcast_pointer_to_bool : Warni<br>
"address of%select{| function| array}0 '%1' will always evaluate to "<br>
"'true'">,<br>
InGroup<PointerBoolConversion>;<br>
+def warn_cast_nonnull_to_bool : Warning<<br>
+ "nonnull parameter '%0' will always evaluate to "<br>
+ "'true'">,<br>
+ InGroup<PointerBoolConversion>;<br>
def warn_this_bool_conversion : Warning<<br>
"'this' pointer cannot be null in well-defined C++ code; pointer may be "<br>
"assumed to always convert to true">, InGroup<UndefinedBoolConversion>;<br>
@@ -2512,6 +2516,10 @@ def warn_null_pointer_compare : Warning<<br>
"comparison of %select{address of|function|array}0 '%1' %select{not |}2"<br>
"equal to a null pointer is always %select{true|false}2">,<br>
InGroup<TautologicalPointerCompare>;<br>
+def warn_nonnull_parameter_compare : Warning<<br>
+ "comparison of nonnull parameter '%0' %select{not |}1"<br>
+ "equal to a null pointer is always %select{true|false}1">,<br>
+ InGroup<TautologicalPointerCompare>;<br>
def warn_this_null_compare : Warning<<br>
"'this' pointer cannot be null in well-defined C++ code; comparison may be "<br>
"assumed to always evaluate to %select{true|false}0">,<br>
<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Why are two new diagnostic messages added here? The text is nearly identical, so it should work by adding a new choice to the %select in the existing warning message.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=220496&r1=220495&r2=220496&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=220496&r1=220495&r2=220496&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Oct 23 14:00:10 2014<br>
@@ -6678,7 +6678,37 @@ void Sema::DiagnoseAlwaysNonNullPointer(<br>
// Weak Decls can be null.<br>
if (!D || D->isWeak())<br>
return;<br>
-<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Move the non-null parameter check to a helper function which takes a ParmVarDecl as a parameter and use early exit to prevent extra work. </span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ // Check for parameter decl with nonnull attribute<br>
+ if (ParmVarDecl* PV = dyn_cast<ParmVarDecl>(D)) {<br>
+ if (FunctionDecl* FD = dyn_cast<FunctionDecl>(PV->getDeclContext())) {<br>
+ unsigned NumArgs = FD->getNumParams();<br>
+ llvm::SmallBitVector AttrNonNull(NumArgs);<br></blockquote><div style="font-family:arial,sans-serif;font-size:13px">BitVector not needed in new function. Access to AttrNonNull are turned in returns. <span style="font-family:arial;font-size:small"> </span></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ for (const auto *NonNull : FD->specific_attrs<NonNullAttr>()) {<br>
+ if (!NonNull->args_size()) {<br>
+ AttrNonNull.set(0, NumArgs);<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Early exit here.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ break;<br>
+ }<br>
+ for (unsigned Val : NonNull->args()) {<br>
+ if (Val >= NumArgs)<br>
+ continue;<br>
+ AttrNonNull.set(Val);<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Use Val to get the ParmVarDecl from the FunctionDecl, which can be compared to ParmVarDecl and return on equality.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+ }<br>
+ }<br>
+ if (!AttrNonNull.empty())<br>
+ for (unsigned i = 0; i < NumArgs; ++i)<br>
+ if (FD->getParamDecl(i) == PV && AttrNonNull[i]) {<br>
+ std::string Str;<br>
+ llvm::raw_string_ostream S(Str);<br>
+ E->printPretty(S, nullptr, getPrintingPolicy());<br>
+ unsigned DiagID = IsCompare ? diag::warn_nonnull_parameter_compare<br>
+ : diag::warn_cast_nonnull_to_bool;<br>
+ Diag(E->getExprLoc(), DiagID) << S.str() << E->getSourceRange()<br>
+ << Range << IsEqual;<br>
+ return;<br>
+ }<br>
+ }<br>
+ }<br>
+<br>
QualType T = D->getType();<br>
const bool IsArray = T->isArrayType();<br>
const bool IsFunction = T->isFunctionType();<br>
<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Once the new warning and the old warning are folded together, use the current Diag call instead having a new one here.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Modified: cfe/trunk/test/Sema/nonnull.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=220496&r1=220495&r2=220496&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/nonnull.c?rev=220496&r1=220495&r2=220496&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/nonnull.c (original)<br>
+++ cfe/trunk/test/Sema/nonnull.c Thu Oct 23 14:00:10 2014<br>
@@ -83,3 +83,28 @@ void redecl_test(void *p) {<br>
redecl(p, 0); // expected-warning{{null passed}}<br>
redecl(0, p); // expected-warning{{null passed}}<br>
}<br>
+<br>
+// rdar://18712242<br>
+#define NULL (void*)0<br>
+__attribute__((__nonnull__))<br>
+int evil_nonnull_func(int* pointer, void * pv)<br>
+{<br>
+ if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is always false}}<br>
+ return 0;<br>
+ } else {<br>
+ return *pointer;<br>
+ }<br>
+<br>
+ if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is always false}}<br>
+}<br>
+<br>
+int another_evil_nonnull_func(int* pointer, char ch, void * pv) __attribute__((nonnull(1, 3)));<br>
+int another_evil_nonnull_func(int* pointer, char ch, void * pv) {<br>
+ if (pointer == NULL) { // expected-warning {{comparison of nonnull parameter 'pointer' equal to a null pointer is always false}}<br>
+ return 0;<br>
+ } else {<br>
+ return *pointer;<br>
+ }<br>
+<br>
+ if (pv == NULL) {} // expected-warning {{comparison of nonnull parameter 'pv' equal to a null pointer is always false}}<br>
+}<br>
<br></blockquote><div><span style="font-family:arial,sans-serif;font-size:13px">Add a test which has multiple pointer parameters, only a few which are marked non-null. Test that the warning only fires on the non-null pointer parameters.</span> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>