From f7432755d0c09b8c01ced9c3b0f337a85a1ccf96 Mon Sep 17 00:00:00 2001 From: Richard Trieu Date: Fri, 6 Jun 2014 21:39:26 +0000 Subject: [PATCH] Add -Wtautological-undefined-compare and -Wundefined-bool-conversion warnings to detect underfined behavior involving pointers. llvm-svn: 210372 --- clang/include/clang/Basic/DiagnosticGroups.td | 8 +++-- clang/include/clang/Basic/DiagnosticSemaKinds.td | 17 ++++++++++ clang/lib/Sema/SemaChecking.cpp | 16 ++++++++-- clang/test/Analysis/inlining/path-notes.cpp | 4 +-- clang/test/Analysis/misc-ps-region-store.cpp | 4 +-- clang/test/Analysis/reference.cpp | 2 +- clang/test/Analysis/stack-addr-ps.cpp | 2 +- .../warn-tautological-undefined-compare.cpp | 34 ++++++++++++++++++++ .../SemaCXX/warn-undefined-bool-conversion.cpp | 37 ++++++++++++++++++++++ 9 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 clang/test/SemaCXX/warn-tautological-undefined-compare.cpp create mode 100644 clang/test/SemaCXX/warn-undefined-bool-conversion.cpp diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 8569591..e605756 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -38,7 +38,9 @@ def LiteralConversion : DiagGroup<"literal-conversion">; 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-size">; 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">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 34e7746..20495e7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2400,10 +2400,27 @@ def warn_impcast_pointer_to_bool : Warning< "address of%select{| function| array}0 '%1' will always evaluate to " "'true'">, InGroup; +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; +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; + 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; +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; +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; def note_function_warning_silence : Note< "prefix with the address-of operator to silence this warning">; diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 916ce7d..570f749 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6189,6 +6189,13 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E, const bool IsCompare = NullKind != Expr::NPCK_NotNull; + if (isa(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(E)) { @@ -6218,9 +6225,14 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E, // 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. diff --git a/clang/test/Analysis/inlining/path-notes.cpp b/clang/test/Analysis/inlining/path-notes.cpp index afbdf21..a1ac53c 100644 --- a/clang/test/Analysis/inlining/path-notes.cpp +++ b/clang/test/Analysis/inlining/path-notes.cpp @@ -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 { diff --git a/clang/test/Analysis/misc-ps-region-store.cpp b/clang/test/Analysis/misc-ps-region-store.cpp index cb51042..af9f7cf 100644 --- a/clang/test/Analysis/misc-ps-region-store.cpp +++ b/clang/test/Analysis/misc-ps-region-store.cpp @@ -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(); diff --git a/clang/test/Analysis/reference.cpp b/clang/test/Analysis/reference.cpp index bd5eaaa..cd0202e 100644 --- a/clang/test/Analysis/reference.cpp +++ b/clang/test/Analysis/reference.cpp @@ -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); diff --git a/clang/test/Analysis/stack-addr-ps.cpp b/clang/test/Analysis/stack-addr-ps.cpp index a39f9c7..a4ab2f3 100644 --- a/clang/test/Analysis/stack-addr-ps.cpp +++ b/clang/test/Analysis/stack-addr-ps.cpp @@ -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; diff --git a/clang/test/SemaCXX/warn-tautological-undefined-compare.cpp b/clang/test/SemaCXX/warn-tautological-undefined-compare.cpp new file mode 100644 index 0000000..b30b576 --- /dev/null +++ b/clang/test/SemaCXX/warn-tautological-undefined-compare.cpp @@ -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; +}; diff --git a/clang/test/SemaCXX/warn-undefined-bool-conversion.cpp b/clang/test/SemaCXX/warn-undefined-bool-conversion.cpp new file mode 100644 index 0000000..c56b6bc --- /dev/null +++ b/clang/test/SemaCXX/warn-undefined-bool-conversion.cpp @@ -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; +}; -- 2.7.4