[PATCH] A new warning, -Wtautological-array-compare, which warns when an array is compared against a null pointer
Richard Trieu
rtrieu at google.com
Tue Jan 28 22:01:17 PST 2014
GCC's -Waddress doesn't catch this or (&x == 0). GCC's -Waddress does
catch (x) for arrays, and (&x) for all types, but Clang doesn't. Clang's
-Waddress is currently empty and only there for command line compatibility.
Clang's warnings that overlap with GCC's -Waddress are spread out among
other warning flags, for instance in some of the conversion warnings.
On Tue, Jan 28, 2014 at 9:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
> Does this overlap at all with GCC's -Waddress? (it might be that it should
> be a subgroup of -Waddress - or generalized to catch some more cases (what
> about &x == 0 or if (&x)))
>
>
> On Tue, Jan 28, 2014 at 9:05 PM, Richard Trieu <rtrieu at google.com> wrote:
>
>> Extend -Wtautological-compare to warn when an array is compared to a null
>> value. Arrays can be implicitly converted to a valid pointer, so it will
>> never be null. Thus, the result of the comparison can be known at run time.
>>
>> This will not warn on externally linked weak arrays, which can be null,
>> and inside macros, which can sometimes expect pointers. Several tests were
>> updated to disable this warning.
>>
>> int foo() {
>> int array[5];
>> if (array != 0) // New warning here
>> return array[0];
>> else
>> return 0;
>> }
>>
>> http://llvm-reviews.chandlerc.com/D2645
>>
>> Files:
>> test/Sema/const-eval.c
>> test/SemaCXX/nullptr_in_arithmetic_ops.cpp
>> test/SemaCXX/warn-tautological-compare.cpp
>> test/SemaCXX/null_in_arithmetic_ops.cpp
>> test/SemaCXX/constant-expression-cxx11.cpp
>> include/clang/Basic/DiagnosticSemaKinds.td
>> include/clang/Basic/DiagnosticGroups.td
>> lib/Sema/SemaExpr.cpp
>>
>> Index: test/Sema/const-eval.c
>> ===================================================================
>> --- test/Sema/const-eval.c
>> +++ test/Sema/const-eval.c
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s
>> +// RUN: %clang_cc1 -fsyntax-only -verify -triple i686-linux %s
>> -Wno-tautological-array-compare
>>
>> #define EVAL_EXPR(testno, expr) int test##testno = sizeof(struct{char
>> qq[expr];});
>> int x;
>> Index: test/SemaCXX/nullptr_in_arithmetic_ops.cpp
>> ===================================================================
>> --- test/SemaCXX/nullptr_in_arithmetic_ops.cpp
>> +++ test/SemaCXX/nullptr_in_arithmetic_ops.cpp
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -fsyntax-only -fblocks -std=c++11 -verify %s
>> +// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-array-compare
>> -fblocks -std=c++11 -verify %s
>>
>> void foo() {
>> int a;
>> Index: test/SemaCXX/warn-tautological-compare.cpp
>> ===================================================================
>> --- test/SemaCXX/warn-tautological-compare.cpp
>> +++ test/SemaCXX/warn-tautological-compare.cpp
>> @@ -25,3 +25,40 @@
>> if (x < kintmax) {}
>> }
>> }
>> +
>> +namespace ArrayCompare {
>> +#define GetValue(ptr) ((ptr != 0) ? ptr[0] : 0)
>> + extern int a[] __attribute__((weak));
>> + int b[] = {8,13,21};
>> + struct {
>> + int x[10];
>> + } c;
>> + const char str[] = "text";
>> + void ignore() {
>> + if (a == 0) {}
>> + if (a != 0) {}
>> + (void)GetValue(b);
>> + }
>> + void test() {
>> + if (b == 0) {}
>> + // expected-warning at -1{{comparison of array of type 'int [3]' equal
>> to a null pointer is always false}}
>> + if (b != 0) {}
>> + // expected-warning at -1{{comparison of array of type 'int [3]' not
>> equal to a null pointer is always true}}
>> + if (0 == b) {}
>> + // expected-warning at -1{{comparison of array of type 'int [3]' equal
>> to a null pointer is always false}}
>> + if (0 != b) {}
>> + // expected-warning at -1{{comparison of array of type 'int [3]' not
>> equal to a null pointer is always true}}
>> + if ("true" == 0) {}
>> + // expected-warning at -1{{comparison of array of type 'const char
>> [5]' equal to a null pointer is always false}}
>> + if ("true" != 0) {}
>> + // expected-warning at -1{{comparison of array of type 'const char
>> [5]' not equal to a null pointer is always true}}
>> + if (c.x != 0) {}
>> + // expected-warning at -1{{comparison of array of type 'int [10]' not
>> equal to a null pointer is always true}}
>> + if (c.x != 0) {}
>> + // expected-warning at -1{{comparison of array of type 'int [10]' not
>> equal to a null pointer is always true}}
>> + if (str == 0) {}
>> + // expected-warning at -1{{comparison of array of type 'const char
>> [5]' equal to a null pointer is always false}}
>> + if (str != 0) {}
>> + // expected-warning at -1{{comparison of array of type 'const char
>> [5]' not equal to a null pointer is always true}}
>> + }
>> +}
>> Index: test/SemaCXX/null_in_arithmetic_ops.cpp
>> ===================================================================
>> --- test/SemaCXX/null_in_arithmetic_ops.cpp
>> +++ test/SemaCXX/null_in_arithmetic_ops.cpp
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks
>> -Wnull-arithmetic -verify -Wno-string-plus-int %s
>> +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -fblocks
>> -Wnull-arithmetic -verify -Wno-string-plus-int
>> -Wno-tautological-array-compare %s
>> #include <stddef.h>
>>
>> void f() {
>> Index: test/SemaCXX/constant-expression-cxx11.cpp
>> ===================================================================
>> --- test/SemaCXX/constant-expression-cxx11.cpp
>> +++ test/SemaCXX/constant-expression-cxx11.cpp
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -triple i686-linux -Wno-string-plus-int
>> -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions
>> -verify -std=c++11 -pedantic %s -Wno-comment
>> +// RUN: %clang_cc1 -triple i686-linux -Wno-string-plus-int
>> -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions
>> -verify -std=c++11 -pedantic %s -Wno-comment -Wno-tautological-array-compare
>>
>> namespace StaticAssertFoldTest {
>>
>> Index: include/clang/Basic/DiagnosticSemaKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticSemaKinds.td
>> +++ include/clang/Basic/DiagnosticSemaKinds.td
>> @@ -4643,6 +4643,10 @@
>> def warn_runsigned_always_true_comparison : Warning<
>> "comparison of %0 unsigned%select{| enum}2 expression is always %1">,
>> InGroup<TautologicalCompare>;
>> +def warn_tautological_array_compare : Warning<
>> + "comparison of array of type %0 %select{not equal to|equal to}1 a null
>> "
>> + "pointer is always %select{true|false}1">,
>> + InGroup<TautologicalArrayCompare>;
>> def warn_comparison_of_mixed_enum_types : Warning<
>> "comparison of two values with different enumeration types"
>> "%diff{ ($ and $)|}0,1">,
>> Index: include/clang/Basic/DiagnosticGroups.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticGroups.td
>> +++ include/clang/Basic/DiagnosticGroups.td
>> @@ -283,8 +283,10 @@
>> def StringPlusChar : DiagGroup<"string-plus-char">;
>> def StrncatSize : DiagGroup<"strncat-size">;
>> def TautologicalOutOfRangeCompare :
>> DiagGroup<"tautological-constant-out-of-range-compare">;
>> +def TautologicalArrayCompare : DiagGroup<"tautological-array-compare">;
>> def TautologicalCompare : DiagGroup<"tautological-compare",
>> - [TautologicalOutOfRangeCompare]>;
>> + [TautologicalOutOfRangeCompare,
>> + TautologicalArrayCompare]>;
>> def HeaderHygiene : DiagGroup<"header-hygiene">;
>> def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">;
>> def CompareDistinctPointerType :
>> DiagGroup<"compare-distinct-pointer-types">;
>> Index: lib/Sema/SemaExpr.cpp
>> ===================================================================
>> --- lib/Sema/SemaExpr.cpp
>> +++ lib/Sema/SemaExpr.cpp
>> @@ -7646,6 +7646,30 @@
>> }
>> }
>>
>> +static void checkArrayNullCompare(Sema &S, SourceLocation Loc,
>> + ExprResult &ArrayExpr, ExprResult
>> &NullExpr,
>> + bool IsEquality) {
>> + if (Loc.isMacroID())
>> + return;
>> +
>> + Expr *Array = ArrayExpr.get()->IgnoreParens();
>> + ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(Array);
>> + if (!ICE || ICE->getCastKind() != CK_ArrayToPointerDecay)
>> + return;
>> +
>> + Array = ICE->getSubExpr();
>> +
>> + if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Array)) {
>> + ValueDecl *VD = DRE->getDecl();
>> + if (VD->hasExternalFormalLinkage() && VD->isWeak())
>> + return;
>> + }
>> +
>> + S.Diag(Loc, diag::warn_tautological_array_compare)
>> + << Array->getSourceRange() << NullExpr.get()->getSourceRange()
>> + << Array->getType() << IsEquality;
>> +}
>> +
>> static void diagnoseLogicalNotOnLHSofComparison(Sema &S, ExprResult &LHS,
>> ExprResult &RHS,
>> SourceLocation Loc,
>> @@ -7830,6 +7854,14 @@
>> bool RHSIsNull = RHS.get()->isNullPointerConstant(Context,
>>
>> Expr::NPC_ValueDependentIsNull);
>>
>> + if (!IsRelational && LHSIsNull != RHSIsNull) {
>> + bool IsEquality = Opc == BO_EQ;
>> + if (RHSIsNull)
>> + checkArrayNullCompare(*this, Loc, LHS, RHS, IsEquality);
>> + else
>> + checkArrayNullCompare(*this, Loc, RHS, LHS, IsEquality);
>> + }
>> +
>> // All of the following pointer-related warnings are GCC extensions,
>> except
>> // when handling null pointer constants.
>> if (LHSType->isPointerType() && RHSType->isPointerType()) { // C99
>> 6.5.8p2
>>
>> _______________________________________________
>> 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/20140128/e24f7b05/attachment.html>
More information about the cfe-commits
mailing list