[PATCH] A new warning, -Wtautological-array-compare, which warns when an array is compared against a null pointer
David Blaikie
dblaikie at gmail.com
Tue Jan 28 21:30:38 PST 2014
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/d18d0867/attachment.html>
More information about the cfe-commits
mailing list