r210372 - Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings
Richard Trieu
rtrieu at google.com
Mon Jun 9 16:04:46 PDT 2014
r210497 and r210498 should make LLVM and Clang clean
for -Wundefined-bool-conversion and -Wtautological-undefined-compare. Let
me know if there are any problems remaining after these revisions.
On Mon, Jun 9, 2014 at 1:04 PM, Richard Trieu <rtrieu at google.com> wrote:
> Sorry for the breakages. I am getting a patch together to fix these
> issues.
>
>
> On Mon, Jun 9, 2014 at 1:07 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>> Meanwhile, all our bots are red due to this commit.
>>
>>
>> On Mon, Jun 9, 2014 at 3:59 AM, Gabriel Dos Reis <
>> gdr at integrable-solutions.net> wrote:
>>
>>>
>>>
>>>
>>> On Sun, Jun 8, 2014 at 4:20 PM, NAKAMURA Takumi <geek4civic at gmail.com>
>>> wrote:
>>>
>>>> We see warnings in the llvm/clang tree. Shall we honor them?
>>>>
>>>> /home/tnakamura/fio/ninja/llvm-project/llvm/lib/IR/AsmWriter.cpp:2159:8:
>>>> error: 'this' pointer cannot be null in well-defined C++ code; pointer
>>>> may be assumed always converted to true
>>>> [-Werror,-Wundefined-bool-conversion]
>>>> if (!this) {
>>>> ~^~~~
>>>> llvm/lib/IR/AsmWriter.cpp:2175:8: error: 'this' pointer cannot be null
>>>> in well-defined C++ code; pointer may be assumed always converted to
>>>> true [-Werror,-Wundefined-bool-conversion]
>>>> if (!this) {
>>>> ~^~~~
>>>>
>>>> clang/lib/AST/StmtPrinter.cpp:2028:7: error: 'this' pointer cannot be
>>>> null in well-defined C++ code; comparison may be assumed to always
>>>> evaluate to false [-Werror,-Wtautological-undefined-compare]
>>>> if (this == nullptr) {
>>>> ^~~~ ~~~~~~~
>>>>
>>>
>>> Interesting. Looks like potentially a source of bug in the existing
>>> code :-)
>>>
>>> -- Gaby
>>>
>>>
>>>
>>>>
>>>> 2014-06-07 6:39 GMT+09:00 Richard Trieu <rtrieu at google.com>:
>>>> > 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
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>> _______________________________________________
>> 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/20140609/f73d7afb/attachment.html>
More information about the cfe-commits
mailing list