<div dir="ltr">Meanwhile, all our bots are red due to this commit.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jun 9, 2014 at 3:59 AM, Gabriel Dos Reis <span dir="ltr"><<a href="mailto:gdr@integrable-solutions.net" target="_blank">gdr@integrable-solutions.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="">On Sun, Jun 8, 2014 at 4:20 PM, NAKAMURA Takumi <span dir="ltr"><<a href="mailto:geek4civic@gmail.com" target="_blank">geek4civic@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">We see warnings in the llvm/clang tree. Shall we honor them?<br>
<br>
/home/tnakamura/fio/ninja/llvm-project/llvm/lib/IR/AsmWriter.cpp:2159:8:<br>
error: 'this' pointer cannot be null in well-defined C++ code; pointer<br>
may be assumed always converted to true<br>
[-Werror,-Wundefined-bool-conversion]<br>
if (!this) {<br>
~^~~~<br>
llvm/lib/IR/AsmWriter.cpp:2175:8: error: 'this' pointer cannot be null<br>
in well-defined C++ code; pointer may be assumed always converted to<br>
true [-Werror,-Wundefined-bool-conversion]<br>
if (!this) {<br>
~^~~~<br>
<br>
clang/lib/AST/StmtPrinter.cpp:2028:7: error: 'this' pointer cannot be<br>
null in well-defined C++ code; comparison may be assumed to always<br>
evaluate to false [-Werror,-Wtautological-undefined-compare]<br>
if (this == nullptr) {<br>
^~~~ ~~~~~~~<br></blockquote><div><br></div></div><div>Interesting. Looks like potentially a source of bug in the existing code :-)</div><div><br></div><div>-- Gaby</div><div><div class="h5"><div><br></div><div>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
2014-06-07 6:39 GMT+09:00 Richard Trieu <<a href="mailto:rtrieu@google.com" target="_blank">rtrieu@google.com</a>>:<br>
> Author: rtrieu<br>
> Date: Fri Jun 6 16:39:26 2014<br>
> New Revision: 210372<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=210372&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=210372&view=rev</a><br>
> Log:<br>
> Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings<br>
> to detect underfined behavior involving pointers.<br>
><br>
> Added:<br>
> cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp<br>
> cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp<br>
> Modified:<br>
> cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> cfe/trunk/test/Analysis/inlining/path-notes.cpp<br>
> cfe/trunk/test/Analysis/misc-ps-region-store.cpp<br>
> cfe/trunk/test/Analysis/reference.cpp<br>
> cfe/trunk/test/Analysis/stack-addr-ps.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Jun 6 16:39:26 2014<br>
> @@ -38,7 +38,9 @@ def LiteralConversion : DiagGroup<"liter<br>
> def StringConversion : DiagGroup<"string-conversion">;<br>
> def SignConversion : DiagGroup<"sign-conversion">;<br>
> def PointerBoolConversion : DiagGroup<"pointer-bool-conversion">;<br>
> -def BoolConversion : DiagGroup<"bool-conversion", [ PointerBoolConversion ] >;<br>
> +def UndefinedBoolConversion : DiagGroup<"undefined-bool-conversion">;<br>
> +def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,<br>
> + UndefinedBoolConversion]>;<br>
> def IntConversion : DiagGroup<"int-conversion">;<br>
> def EnumConversion : DiagGroup<"enum-conversion">;<br>
> def FloatConversion : DiagGroup<"float-conversion">;<br>
> @@ -298,10 +300,12 @@ def StrncatSize : DiagGroup<"strncat-siz<br>
> def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;<br>
> def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;<br>
> def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;<br>
> +def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;<br>
> def TautologicalCompare : DiagGroup<"tautological-compare",<br>
> [TautologicalOutOfRangeCompare,<br>
> TautologicalPointerCompare,<br>
> - TautologicalOverlapCompare]>;<br>
> + TautologicalOverlapCompare,<br>
> + TautologicalUndefinedCompare]>;<br>
> def HeaderHygiene : DiagGroup<"header-hygiene">;<br>
> def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;<br>
> def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">;<br>
><br>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 6 16:39:26 2014<br>
> @@ -2400,10 +2400,27 @@ def warn_impcast_pointer_to_bool : Warni<br>
> "address of%select{| function| array}0 '%1' will always evaluate to "<br>
> "'true'">,<br>
> InGroup<PointerBoolConversion>;<br>
> +def warn_this_bool_conversion : Warning<<br>
> + "'this' pointer cannot be null in well-defined C++ code; pointer may be "<br>
> + "assumed always converted to true">, InGroup<UndefinedBoolConversion>;<br>
> +def warn_address_of_reference_bool_conversion : Warning<<br>
> + "reference cannot be bound to dereferenced null pointer in well-defined C++ "<br>
> + "code; pointer may be assumed always converted to true">,<br>
> + InGroup<UndefinedBoolConversion>;<br>
> +<br>
> def warn_null_pointer_compare : Warning<<br>
> "comparison of %select{address of|function|array}0 '%1' %select{not |}2"<br>
> "equal to a null pointer is always %select{true|false}2">,<br>
> InGroup<TautologicalPointerCompare>;<br>
> +def warn_this_null_compare : Warning<<br>
> + "'this' pointer cannot be null in well-defined C++ code; comparison may be "<br>
> + "assumed to always evaluate to %select{true|false}0">,<br>
> + InGroup<TautologicalUndefinedCompare>;<br>
> +def warn_address_of_reference_null_compare : Warning<<br>
> + "reference cannot be bound to dereferenced null pointer in well-defined C++ "<br>
> + "code; comparison may be assumed to always evaluate to "<br>
> + "%select{true|false}0">,<br>
> + InGroup<TautologicalUndefinedCompare>;<br>
><br>
> def note_function_warning_silence : Note<<br>
> "prefix with the address-of operator to silence this warning">;<br>
><br>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -6189,6 +6189,13 @@ void Sema::DiagnoseAlwaysNonNullPointer(<br>
><br>
> const bool IsCompare = NullKind != Expr::NPCK_NotNull;<br>
><br>
> + if (isa<CXXThisExpr>(E)) {<br>
> + unsigned DiagID = IsCompare ? diag::warn_this_null_compare<br>
> + : diag::warn_this_bool_conversion;<br>
> + Diag(E->getExprLoc(), DiagID) << E->getSourceRange() << Range << IsEqual;<br>
> + return;<br>
> + }<br>
> +<br>
> bool IsAddressOf = false;<br>
><br>
> if (UnaryOperator *UO = dyn_cast<UnaryOperator>(E)) {<br>
> @@ -6218,9 +6225,14 @@ void Sema::DiagnoseAlwaysNonNullPointer(<br>
> // Address of function is used to silence the function warning.<br>
> if (IsFunction)<br>
> return;<br>
> - // Address of reference can be null.<br>
> - if (T->isReferenceType())<br>
> +<br>
> + if (T->isReferenceType()) {<br>
> + unsigned DiagID = IsCompare<br>
> + ? diag::warn_address_of_reference_null_compare<br>
> + : diag::warn_address_of_reference_bool_conversion;<br>
> + Diag(E->getExprLoc(), DiagID) << E->getSourceRange() << Range << IsEqual;<br>
> return;<br>
> + }<br>
> }<br>
><br>
> // Found nothing.<br>
><br>
> Modified: cfe/trunk/test/Analysis/inlining/path-notes.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.cpp?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.cpp?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Analysis/inlining/path-notes.cpp (original)<br>
> +++ cfe/trunk/test/Analysis/inlining/path-notes.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -1,5 +1,5 @@<br>
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config c++-inlining=destructors -std=c++11 -verify %s<br>
> -// 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<br>
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-output=text -analyzer-config c++-inlining=destructors -std=c++11 -verify -Wno-tautological-undefined-compare %s<br>
> +// 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<br>
> // RUN: FileCheck --input-file=%t.plist %s<br>
><br>
> class Foo {<br>
><br>
> Modified: cfe/trunk/test/Analysis/misc-ps-region-store.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.cpp?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/misc-ps-region-store.cpp?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Analysis/misc-ps-region-store.cpp (original)<br>
> +++ cfe/trunk/test/Analysis/misc-ps-region-store.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -1,5 +1,5 @@<br>
> -// 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<br>
> -// 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<br>
> +// 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<br>
> +// 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<br>
><br>
> void clang_analyzer_warnIfReached();<br>
><br>
><br>
> Modified: cfe/trunk/test/Analysis/reference.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Analysis/reference.cpp (original)<br>
> +++ cfe/trunk/test/Analysis/reference.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -1,4 +1,4 @@<br>
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -analyzer-store=region -analyzer-constraints=range -verify -Wno-null-dereference %s<br>
> +// 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<br>
><br>
> void clang_analyzer_eval(bool);<br>
><br>
><br>
> Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=210372&r1=210371&r2=210372&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=210372&r1=210371&r2=210372&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)<br>
> +++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -1,4 +1,4 @@<br>
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region -verify %s<br>
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store=region -verify %s -Wno-undefined-bool-conversion<br>
><br>
> typedef __INTPTR_TYPE__ intptr_t;<br>
><br>
><br>
> Added: cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=210372&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp?rev=210372&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/warn-tautological-undefined-compare.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -0,0 +1,34 @@<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-undefined-compare %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-tautological-compare -Wtautological-undefined-compare %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare %s<br>
> +<br>
> +void test1(int &x) {<br>
> + if (x == 1) { }<br>
> + if (&x == 0) { }<br>
> + // 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}}<br>
> + if (&x != 0) { }<br>
> + // 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}}<br>
> +}<br>
> +<br>
> +class test2 {<br>
> + test2() : x(y) {}<br>
> +<br>
> + void foo() {<br>
> + if (this == 0) { }<br>
> + // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to false}}<br>
> + if (this != 0) { }<br>
> + // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; comparison may be assumed to always evaluate to true}}<br>
> + }<br>
> +<br>
> + void bar() {<br>
> + if (x == 1) { }<br>
> + if (&x == 0) { }<br>
> + // 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}}<br>
> + if (&x != 0) { }<br>
> + // 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}}<br>
> + }<br>
> +<br>
> + int &x;<br>
> + int y;<br>
> +};<br>
><br>
> Added: cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=210372&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp?rev=210372&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp (added)<br>
> +++ cfe/trunk/test/SemaCXX/warn-undefined-bool-conversion.cpp Fri Jun 6 16:39:26 2014<br>
> @@ -0,0 +1,37 @@<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wundefined-bool-conversion %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-bool-conversion -Wundefined-bool-conversion %s<br>
> +// RUN: %clang_cc1 -fsyntax-only -verify -Wbool-conversion %s<br>
> +<br>
> +void test1(int &x) {<br>
> + if (x == 1) { }<br>
> + if (&x) { }<br>
> + // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> +<br>
> + if (!&x) { }<br>
> + // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> +}<br>
> +<br>
> +class test2 {<br>
> + test2() : x(y) {}<br>
> +<br>
> + void foo() {<br>
> + if (this) { }<br>
> + // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> +<br>
> + if (!this) { }<br>
> + // expected-warning@-1{{'this' pointer cannot be null in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> + }<br>
> +<br>
> + void bar() {<br>
> + if (x == 1) { }<br>
> + if (&x) { }<br>
> + // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> +<br>
> + if (!&x) { }<br>
> + // expected-warning@-1{{reference cannot be bound to dereferenced null pointer in well-defined C++ code; pointer may be assumed always converted to true}}<br>
> + }<br>
> +<br>
> + int &x;<br>
> + int y;<br>
> +};<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>