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 <[email protected]> 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 <[email protected]> wrote: > >> Meanwhile, all our bots are red due to this commit. >> >> >> On Mon, Jun 9, 2014 at 3:59 AM, Gabriel Dos Reis < >> [email protected]> wrote: >> >>> >>> >>> >>> On Sun, Jun 8, 2014 at 4:20 PM, NAKAMURA Takumi <[email protected]> >>> 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 <[email protected]>: >>>> > 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@-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@-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@-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@-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@-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@-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@-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@-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@-1{{'this' pointer cannot be null in >>>> well-defined C++ code; pointer may be assumed always converted to true}} >>>> > + >>>> > + if (!this) { } >>>> > + // expected-warning@-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@-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@-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 >>>> > [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 >>>> >>> >>> >>> _______________________________________________ >>> 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 >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
