[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 21:05:54 PST 2014


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2645.1.patch
Type: text/x-patch
Size: 7485 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140128/f2f33555/attachment.bin>


More information about the cfe-commits mailing list