r210372 - Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings

Gabriel Dos Reis gdr at integrable-solutions.net
Sun Jun 8 16:59:16 PDT 2014


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140608/10aed36a/attachment.html>


More information about the cfe-commits mailing list