On Fri, Aug 17, 2012 at 3:33 PM, Nico Weber <[email protected]> wrote: > Should this really be on by default? On chrome, this triggers a single > time (linux-only): > > ../../third_party/tcmalloc/chromium/src/stack_trace_table.cc:138:16: > warning: expression which evaluates to zero treated as a null pointer > constant of type 'void *' [-Wnon-literal-null-conversion] > out[idx++] = static_cast<uintptr_t>(0); > ^~~~~~~~~~~~~~~~~~~~~~~~~ > > out is declared as `void** out = new void*[out_len];`. The warning > isn't wrong, but it looks rather pedantic to me. Should this be only > in -Wall (or maybe even in -pedantic)?
Might be a fair candidate for -Wall, though it did find some reasonable stuff in google. 18 cases overall with some fairly interesting ones (see b/6954211 for the ones that've been committed so far, or cl/32692314 for some of the remaining ones. The worst offenders are integer constants with value 0 that aren't at all intended to be pointers. (most easily occurred in function calls where the caller thought the argument was of one type but it's actually of a pointer type) I have some more once this warning opens up to cover comparisons, conditional operands, and return statements - there's a lot of confusing "cstr == '\0'" code where the user probably meant to deref the lhs but didn't. > > On Wed, Aug 8, 2012 at 10:33 AM, David Blaikie <[email protected]> wrote: >> Author: dblaikie >> Date: Wed Aug 8 12:33:31 2012 >> New Revision: 161501 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=161501&view=rev >> Log: >> Implement warning for integral null pointer constants other than the literal >> 0. >> >> This is effectively a warning for code that violates core issue 903 & thus >> will >> become standard error in the future, hopefully. It catches strange null >> pointers such as: '\0', 1 - 1, const int null = 0; etc... >> >> There's currently a flaw in this warning (& the warning for 'false' as a null >> pointer literal as well) where it doesn't trigger on comparisons (ptr == '\0' >> for example). Fix to come in a future patch. >> >> Also, due to this only being a warning, not an error, it triggers quite >> frequently on gtest code which tests expressions for null-pointer-ness in a >> SFINAE context (so it wouldn't be a problem if this was an error as in an >> actual implementation of core issue 903). To workaround this for now, the >> diagnostic does not fire in unevaluated contexts. >> >> Review by Sean Silva and Richard Smith. >> >> Modified: >> cfe/trunk/include/clang/AST/Expr.h >> cfe/trunk/include/clang/Basic/DiagnosticGroups.td >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/AST/Expr.cpp >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/lib/Sema/SemaOverload.cpp >> cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp >> cfe/trunk/test/SemaCXX/lambda-expressions.cpp >> cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp >> cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp >> >> Modified: cfe/trunk/include/clang/AST/Expr.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/AST/Expr.h (original) >> +++ cfe/trunk/include/clang/AST/Expr.h Wed Aug 8 12:33:31 2012 >> @@ -541,8 +541,15 @@ >> /// \brief Expression is not a Null pointer constant. >> NPCK_NotNull = 0, >> >> - /// \brief Expression is a Null pointer constant built from a zero >> integer. >> - NPCK_ZeroInteger, >> + /// \brief Expression is a Null pointer constant built from a zero >> integer >> + /// expression that is not a simple, possibly parenthesized, zero >> literal. >> + /// C++ Core Issue 903 will classify these expressions as "not pointers" >> + /// once it is adopted. >> + /// http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#903 >> + NPCK_ZeroExpression, >> + >> + /// \brief Expression is a Null pointer constant built from a literal >> zero. >> + NPCK_ZeroLiteral, >> >> /// \brief Expression is a C++0X nullptr. >> NPCK_CXX0X_nullptr, >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Aug 8 12:33:31 >> 2012 >> @@ -33,6 +33,7 @@ >> def SignConversion : DiagGroup<"sign-conversion">; >> def BoolConversion : DiagGroup<"bool-conversion">; >> def IntConversion : DiagGroup<"int-conversion">; >> +def NonLiteralNullConversion : DiagGroup<"non-literal-null-conversion">; >> def NullConversion : DiagGroup<"null-conversion">; >> def BuiltinRequiresHeader : DiagGroup<"builtin-requires-header">; >> def CXXCompat: DiagGroup<"c++-compat">; >> @@ -314,7 +315,8 @@ >> StringConversion, >> SignConversion, >> BoolConversion, >> - NullConversion, >> + NullConversion, // NULL->non-pointer >> + NonLiteralNullConversion, // (1-1)->pointer >> (etc) >> IntConversion]>, >> DiagCategory<"Value Conversion Issue">; >> >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug 8 12:33:31 >> 2012 >> @@ -1896,6 +1896,9 @@ >> def warn_impcast_bool_to_null_pointer : Warning< >> "initialization of pointer of type %0 to null from a constant boolean " >> "expression">, InGroup<BoolConversion>; >> +def warn_non_literal_null_pointer : Warning< >> + "expression which evaluates to zero treated as a null pointer constant >> of " >> + "type %0">, InGroup<NonLiteralNullConversion>; >> def warn_impcast_null_pointer_to_integer : Warning< >> "implicit conversion of NULL constant to %0">, >> InGroup<NullConversion>; >> >> Modified: cfe/trunk/lib/AST/Expr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/lib/AST/Expr.cpp (original) >> +++ cfe/trunk/lib/AST/Expr.cpp Wed Aug 8 12:33:31 2012 >> @@ -2903,7 +2903,7 @@ >> llvm_unreachable("Unexpected value dependent expression!"); >> case NPC_ValueDependentIsNull: >> if (isTypeDependent() || getType()->isIntegralType(Ctx)) >> - return NPCK_ZeroInteger; >> + return NPCK_ZeroExpression; >> else >> return NPCK_NotNull; >> >> @@ -2977,7 +2977,12 @@ >> return NPCK_NotNull; >> } >> >> - return (EvaluateKnownConstInt(Ctx) == 0) ? NPCK_ZeroInteger : >> NPCK_NotNull; >> + if (EvaluateKnownConstInt(Ctx) != 0) >> + return NPCK_NotNull; >> + >> + if (isa<IntegerLiteral>(this)) >> + return NPCK_ZeroLiteral; >> + return NPCK_ZeroExpression; >> } >> >> /// \brief If this expression is an l-value for an Objective C >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Aug 8 12:33:31 2012 >> @@ -4632,7 +4632,10 @@ >> if (NullKind == Expr::NPCK_NotNull) >> return false; >> >> - if (NullKind == Expr::NPCK_ZeroInteger) { >> + if (NullKind == Expr::NPCK_ZeroExpression) >> + return false; >> + >> + if (NullKind == Expr::NPCK_ZeroLiteral) { >> // In this case, check to make sure that we got here from a "NULL" >> // string in the source code. >> NullExpr = NullExpr->IgnoreParenImpCasts(); >> >> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Aug 8 12:33:31 2012 >> @@ -2562,13 +2562,17 @@ >> >> Kind = CK_BitCast; >> >> - if (!IsCStyleOrFunctionalCast && >> - Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy) && >> - From->isNullPointerConstant(Context, >> Expr::NPC_ValueDependentIsNotNull)) >> - DiagRuntimeBehavior(From->getExprLoc(), From, >> - PDiag(diag::warn_impcast_bool_to_null_pointer) >> - << ToType << From->getSourceRange()); >> - >> + if (!IsCStyleOrFunctionalCast && !FromType->isAnyPointerType() && >> + From->isNullPointerConstant(Context, >> Expr::NPC_ValueDependentIsNotNull) == >> + Expr::NPCK_ZeroExpression) { >> + if (Context.hasSameUnqualifiedType(From->getType(), Context.BoolTy)) >> + DiagRuntimeBehavior(From->getExprLoc(), From, >> + PDiag(diag::warn_impcast_bool_to_null_pointer) >> + << ToType << From->getSourceRange()); >> + else if (!isUnevaluatedContext()) >> + Diag(From->getExprLoc(), diag::warn_non_literal_null_pointer) >> + << ToType << From->getSourceRange(); >> + } >> if (const PointerType *ToPtrType = ToType->getAs<PointerType>()) { >> if (const PointerType *FromPtrType = FromType->getAs<PointerType>()) { >> QualType FromPointeeType = FromPtrType->getPointeeType(), >> >> Modified: cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp (original) >> +++ cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp Wed Aug 8 12:33:31 >> 2012 >> @@ -710,10 +710,16 @@ >> static_assert((Base2*)(Derived*)(Base*)pb1 == pok2, ""); >> static_assert((Derived*)(Base*)pb1 == (Derived*)pok2, ""); >> >> -constexpr Base *nullB = 42 - 6 * 7; >> +constexpr Base *nullB = 42 - 6 * 7; // expected-warning {{expression which >> evaluates to zero treated as a null pointer constant of type 'Class::Base >> *const'}} >> static_assert((Bottom*)nullB == 0, ""); >> static_assert((Derived*)nullB == 0, ""); >> static_assert((void*)(Bottom*)nullB == (void*)(Derived*)nullB, ""); >> +Base * nullB2 = '\0'; // expected-warning {{expression which evaluates to >> zero treated as a null pointer constant of type 'Class::Base *'}} >> +Base * nullB3 = (0); >> +// We suppress the warning in unevaluated contexts to workaround some gtest >> +// behavior. Once this becomes an error this isn't a problem anymore. >> +static_assert(nullB == (1 - 1), ""); >> + >> >> namespace ConversionOperators { >> >> >> Modified: cfe/trunk/test/SemaCXX/lambda-expressions.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/lambda-expressions.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/lambda-expressions.cpp (original) >> +++ cfe/trunk/test/SemaCXX/lambda-expressions.cpp Wed Aug 8 12:33:31 2012 >> @@ -117,16 +117,16 @@ >> >> const int m = 0; >> [=] { >> - int &k = f(m); // a null pointer constant >> + int &k = f(m); // expected-warning{{expression which evaluates to >> zero treated as a null pointer constant of type 'int *'}} >> } (); >> >> [=] () -> bool { >> - int &k = f(m); // a null pointer constant >> + int &k = f(m); // expected-warning{{expression which evaluates to >> zero treated as a null pointer constant of type 'int *'}} >> return &m == 0; >> } (); >> >> [m] { >> - int &k = f(m); // a null pointer constant >> + int &k = f(m); // expected-warning{{expression which evaluates to >> zero treated as a null pointer constant of type 'int *'}} >> } (); >> } >> } >> >> Modified: cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp (original) >> +++ cfe/trunk/test/SemaTemplate/explicit-instantiation.cpp Wed Aug 8 >> 12:33:31 2012 >> @@ -14,7 +14,7 @@ >> T f0(T x) { >> return x + 1; // expected-error{{invalid operands}} >> } >> - T* f0(T*, T*) { return T(); } >> + T* f0(T*, T*) { return T(); } // expected-warning{{expression which >> evaluates to zero treated as a null pointer constant of type 'int *'}} >> >> template<typename U> >> T f0(T, U) { return T(); } >> @@ -32,7 +32,7 @@ >> template NotDefaultConstructible X0<NotDefaultConstructible>::value; // >> expected-note{{instantiation}} >> >> template int X0<int>::f0(int); >> -template int* X0<int>::f0(int*, int*); >> +template int* X0<int>::f0(int*, int*); // expected-note{{in instantiation >> of member function 'X0<int>::f0' requested here}} >> template int X0<int>::f0(int, float); >> >> template int X0<int>::f0(int) const; // expected-error{{does not refer}} >> >> Modified: cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp?rev=161501&r1=161500&r2=161501&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp (original) >> +++ cfe/trunk/test/SemaTemplate/instantiate-member-class.cpp Wed Aug 8 >> 12:33:31 2012 >> @@ -124,19 +124,20 @@ >> { >> struct B >> { >> - struct C { C() { int *ptr = I; } }; // expected-error{{cannot >> initialize a variable of type 'int *' with an rvalue of type 'int'}} >> + struct C { C() { int *ptr = I; } }; // expected-error{{cannot >> initialize a variable of type 'int *' with an rvalue of type 'int'}} \ >> + expected-warning{{expression >> which evaluates to zero treated as a null pointer constant of type 'int *'}} >> }; >> }; >> >> template<int N> void foo() >> { >> - class A<N>::B::C X; // expected-note{{in instantiation of member >> function}} >> + class A<N>::B::C X; // expected-note 2 {{in instantiation of member >> function}} >> int A<N+1>::B::C::*member = 0; >> } >> >> void bar() >> { >> - foo<0>(); >> + foo<0>(); // expected-note{{in instantiation of function template}} >> foo<1>(); // expected-note{{in instantiation of function template}} >> } >> } >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
