r210372 - Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings
Nico Weber
thakis at chromium.org
Tue Aug 5 15:36:40 PDT 2014
This is a very useful warning, thanks.
One issue I have with it is that it's disabled by Wno-tautological-compare.
>From the warning name that makes sense – but Wtautological-compare used to
be a fairly harmless warning, so we (chromium) disabled it for a bunch of
third-party libraries. When Wtautological-undefined-compare arrived, we
fixed all instances of this in our code and updated our compiler to a clang
that optimizes away comparisons of references with NULL (since we had fixed
all of these comparisons, we thought!)
I recently realized that we didn't see Wtautological-undefined-compare
warnings for all the third-party libraries that we're building with
Wno-tautological-compare.
Do you think Wtautological-undefined-compare should be in its own warning
group (and maybe have a different name to make that clear), so that folks
who disabled Wtautological-compare still get this warning? The reasoning is
that this warning is much more serious that what Wtautological-compare used
to warn about.
Nico
On Fri, Jun 6, 2014 at 2:39 PM, Richard Trieu <rtrieu at google.com> wrote:
> Author: rtrieu
> Date: Fri Jun 6 16:39:26 2014
> New Revision: 210372
>
> URL: http://llvm.org/viewvc/llvm-project?rev=210372&view=rev
> Log:
> Add -Wtautological-undefined-compare and -Wundefined-bool-conversion
> warnings
> to detect underfined behavior involving pointers.
>
> Added:
> cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
> cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/Analysis/inlining/path-notes.cpp
> cfe/trunk/test/Analysis/misc-ps-region-store.cpp
> cfe/trunk/test/Analysis/reference.cpp
> cfe/trunk/test/Analysis/stack-addr-ps.cpp
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Jun 6 16:39:26
> 2014
> @@ -38,7 +38,9 @@ def LiteralConversion : DiagGroup<"liter
> def StringConversion : DiagGroup<"string-conversion">;
> def SignConversion : DiagGroup<"sign-conversion">;
> def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;
> -def BoolConversion : DiagGroup<"bool-conversion", [ PointerBoolConversion
> ] >;
> +def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;
> +def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
> +
> UndefinedBoolConversion]>;
> def IntConversion : DiagGroup<"int-conversion">;
> def EnumConversion : DiagGroup<"enum-conversion">;
> def FloatConversion : DiagGroup<"float-conversion">;
> @@ -298,10 +300,12 @@ def StrncatSize : DiagGroup<"strncat-siz
> def TautologicalOutOfRangeCompare :
> DiagGroup<"tautological-constant-out-of-range-compare">;
> def TautologicalPointerCompare :
> DiagGroup<"tautological-pointer-compare">;
> def TautologicalOverlapCompare :
> DiagGroup<"tautological-overlap-compare">;
> +def TautologicalUndefinedCompare :
> DiagGroup<"tautological-undefined-compare">;
> def TautologicalCompare : DiagGroup<"tautological-compare",
> [TautologicalOutOfRangeCompare,
> TautologicalPointerCompare,
> - TautologicalOverlapCompare]>;
> + TautologicalOverlapCompare,
> + TautologicalUndefinedCompare]>;
> def HeaderHygiene : DiagGroup<"header-hygiene">;
> def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
> def CompareDistinctPointerType :
> DiagGroup<"compare-distinct-pointer-types">;
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 6
> 16:39:26 2014
> @@ -2400,10 +2400,27 @@ def warn_impcast_pointer_to_bool : Warni
> "address of%select{| function| array}0 '%1' will always evaluate to "
> "'true'">,
> InGroup<PointerBoolConversion>;
> +def warn_this_bool_conversion : Warning<
> + "'this' pointer cannot be null in well-defined C++ code; pointer may be
> "
> + "assumed always converted to true">, InGroup<UndefinedBoolConversion>;
> +def warn_address_of_reference_bool_conversion : Warning<
> + "reference cannot be bound to dereferenced null pointer in well-defined
> C++ "
> + "code; pointer may be assumed always converted to true">,
> + InGroup<UndefinedBoolConversion>;
> +
> 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_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">,
> + InGroup<TautologicalUndefinedCompare>;
> +def warn_address_of_reference_null_compare : Warning<
> + "reference cannot be bound to dereferenced null pointer in well-defined
> C++ "
> + "code; comparison may be assumed to always evaluate to "
> + "%select{true|false}0">,
> + InGroup<TautologicalUndefinedCompare>;
>
> def note_function_warning_silence : Note<
> "prefix with the address-of operator to silence this warning">;
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jun 6 16:39:26 2014
> @@ -6189,6 +6189,13 @@ void Sema::DiagnoseAlwaysNonNullPointer(
>
> const bool IsCompare = NullKind != Expr::NPCK_NotNull;
>
> + if (isa<CXXThisExpr>(E)) {
> + unsigned DiagID = IsCompare ? diag::warn_this_null_compare
> + : diag::warn_this_bool_conversion;
> + Diag(E->getExprLoc(), DiagID) << E->getSourceRange() << Range <<
> IsEqual;
> + return;
> + }
> +
> bool IsAddressOf = false;
>
> if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {
> @@ -6218,9 +6225,14 @@ void Sema::DiagnoseAlwaysNonNullPointer(
> // Address of function is used to silence the function warning.
> if (IsFunction)
> return;
> - // Address of reference can be null.
> - if (T->isReferenceType())
> +
> + if (T->isReferenceType()) {
> + unsigned DiagID = IsCompare
> + ? diag::warn_address_of_reference_null_compare
> + :
> diag::warn_address_of_reference_bool_conversion;
> + Diag(E->getExprLoc(), DiagID) << E->getSourceRange() << Range <<
> IsEqual;
> return;
> + }
> }
>
> // Found nothing.
>
> Modified: cfe/trunk/test/Analysis/inlining/path-notes.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.cpp?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/path-notes.cpp (original)
> +++ cfe/trunk/test/Analysis/inlining/path-notes.cpp Fri Jun 6 16:39:26
> 2014
> @@ -1,5 +1,5 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text
> -analyzer-config c++-inlining=destructors -std=c++11 -verify %s
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core
> -analyzer-output=plist-multi-file -analyzer-config c++-inlining=destructors
> -std=c++11 -analyzer-config path-diagnostics-alternate=false %s -o %t.plist
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text
> -analyzer-config c++-inlining=destructors -std=c++11 -verify
> -Wno-tautological-undefined-compare %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core
> -analyzer-output=plist-multi-file -analyzer-config c++-inlining=destructors
> -std=c++11 -analyzer-config path-diagnostics-alternate=false %s -o %t.plist
> -Wno-tautological-undefined-compare
> // RUN: FileCheck --input-file=%t.plist %s
>
> class Foo {
>
> Modified: cfe/trunk/test/Analysis/misc-ps-region-store.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.cpp?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/misc-ps-region-store.cpp (original)
> +++ cfe/trunk/test/Analysis/misc-ps-region-store.cpp Fri Jun 6 16:39:26
> 2014
> @@ -1,5 +1,5 @@
> -// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks
> %s -fexceptions -fcxx-exceptions
> -// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks
> %s -fexceptions -fcxx-exceptions
> +// RUN: %clang_cc1 -triple i386-apple-darwin9 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks
> %s -fexceptions -fcxx-exceptions -Wno-tautological-undefined-compare
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -verify -fblocks -analyzer-opt-analyze-nested-blocks
> %s -fexceptions -fcxx-exceptions -Wno-tautological-undefined-compare
>
> void clang_analyzer_warnIfReached();
>
>
> Modified: cfe/trunk/test/Analysis/reference.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/reference.cpp (original)
> +++ cfe/trunk/test/Analysis/reference.cpp Fri Jun 6 16:39:26 2014
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -analyzer-constraints=range -verify
> -Wno-null-dereference %s
> +// RUN: %clang_cc1 -analyze
> -analyzer-checker=core,alpha.core,debug.ExprInspection
> -analyzer-store=region -analyzer-constraints=range -verify
> -Wno-null-dereference -Wno-tautological-undefined-compare %s
>
> void clang_analyzer_eval(bool);
>
>
> Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=210372&r1=210371&r2=210372&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)
> +++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Jun 6 16:39:26 2014
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region
> -verify %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region
> -verify %s -Wno-undefined-bool-conversion
>
> typedef __INTPTR_TYPE__ intptr_t;
>
>
> Added: cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=210372&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp Fri Jun
> 6 16:39:26 2014
> @@ -0,0 +1,34 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-undefined-compare
> %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-tautological-compare
> -Wtautological-undefined-compare %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare %s
> +
> +void test1(int &x) {
> + if (x == 1) { }
> + if (&x == 0) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced null
> pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to false}}
> + if (&x != 0) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced null
> pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to true}}
> +}
> +
> +class test2 {
> + test2() : x(y) {}
> +
> + void foo() {
> + if (this == 0) { }
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> false}}
> + if (this != 0) { }
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; comparison may be assumed to always evaluate to
> true}}
> + }
> +
> + void bar() {
> + if (x == 1) { }
> + if (&x == 0) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to false}}
> + if (&x != 0) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; comparison may be assumed to always
> evaluate to true}}
> + }
> +
> + int &x;
> + int y;
> +};
>
> Added: cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=210372&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp (added)
> +++ cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp Fri Jun 6
> 16:39:26 2014
> @@ -0,0 +1,37 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wundefined-bool-conversion %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-bool-conversion
> -Wundefined-bool-conversion %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wbool-conversion %s
> +
> +void test1(int &x) {
> + if (x == 1) { }
> + if (&x) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced null
> pointer in well-defined C++ code; pointer may be assumed always converted
> to true}}
> +
> + if (!&x) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced null
> pointer in well-defined C++ code; pointer may be assumed always converted
> to true}}
> +}
> +
> +class test2 {
> + test2() : x(y) {}
> +
> + void foo() {
> + if (this) { }
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; pointer may be assumed always converted to true}}
> +
> + if (!this) { }
> + // expected-warning at -1{{'this' pointer cannot be null in
> well-defined C++ code; pointer may be assumed always converted to true}}
> + }
> +
> + void bar() {
> + if (x == 1) { }
> + if (&x) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; pointer may be assumed always
> converted to true}}
> +
> + if (!&x) { }
> + // expected-warning at -1{{reference cannot be bound to dereferenced
> null pointer in well-defined C++ code; pointer may be assumed always
> converted to true}}
> + }
> +
> + int &x;
> + int y;
> +};
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140805/84ffed33/attachment.html>
More information about the cfe-commits
mailing list