ilya updated this revision to Diff 235184.
ilya added a comment.

1. Refactor `CheckArrayAccess()` to take `AllowPastTheEnd` parameter to avoid a 
false positive array out-of-bounds warning for `&(cond ? arr1[N] : arr2[N])`.
2. Use `IgnoreParenCasts()` instead of `IgnoreParenImpCasts()` in 
`CheckArrayAccess()` to avoid false negatives when explicit cast is involved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71714/new/

https://reviews.llvm.org/D71714

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Parser/cxx-ambig-decl-expr.cpp
  clang/test/SemaCXX/array-bounds.cpp

Index: clang/test/SemaCXX/array-bounds.cpp
===================================================================
--- clang/test/SemaCXX/array-bounds.cpp
+++ clang/test/SemaCXX/array-bounds.cpp
@@ -27,7 +27,7 @@
 };
 
 void f1(int a[1]) {
-  int val = a[3]; // no warning for function argumnet
+  int val = a[3]; // no warning for function argument
 }
 
 void f2(const int (&a)[2]) { // expected-note {{declared here}}
@@ -133,7 +133,7 @@
 
 int test_sizeof_as_condition(int flag) {
   int arr[2] = { 0, 0 }; // expected-note {{array 'arr' declared here}}
-  if (flag) 
+  if (flag)
     return sizeof(char) != sizeof(char) ? arr[2] : arr[1];
   return sizeof(char) == sizeof(char) ? arr[2] : arr[1]; // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
 }
@@ -241,7 +241,7 @@
 }
 
 int test_pr11007_aux(const char * restrict, ...);
-  
+
 // Test checking with varargs.
 void test_pr11007() {
   double a[5]; // expected-note {{array 'a' declared here}}
@@ -309,3 +309,25 @@
     foo<int>(); // expected-note 1{{in instantiation of function template specialization}}
   };
 }
+
+namespace PR44343 {
+  const unsigned int array[2] = {0, 1}; // expected-note 5{{array 'array' declared here}}
+
+  const int i1 = (const int)array[2]; // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+  const int i2 = static_cast<const int>(array[2]); // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+  const int &i3 = reinterpret_cast<const int&>(array[2]); // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+  unsigned int &i4 = const_cast<unsigned int&>(array[2]); // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+  int i5 = int(array[2]); // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+  const unsigned int *i6 = &(1 > 0 ? array[2] : array[1]); // no warning for one-past-end element's address retrieval
+
+  struct Base {
+    virtual ~Base();
+  };
+
+  struct Derived : Base {
+  };
+
+  Base baseArr[2]; // expected-note {{array 'baseArr' declared here}}
+  Derived *d1 = dynamic_cast<Derived *>(&baseArr[2]); // no warning for one-past-end element's address retrieval
+  Derived &d2 = dynamic_cast<Derived &>(baseArr[2]); // expected-warning {{array index 2 is past the end of the array (which contains 2 elements)}}
+}
Index: clang/test/Parser/cxx-ambig-decl-expr.cpp
===================================================================
--- clang/test/Parser/cxx-ambig-decl-expr.cpp
+++ clang/test/Parser/cxx-ambig-decl-expr.cpp
@@ -24,7 +24,7 @@
 
   // This is array indexing not an array declarator because a comma expression
   // is not syntactically a constant-expression.
-  int(x[1,1]); // expected-warning 2{{unused}}
+  int(x[1,0]); // expected-warning 2{{unused}}
 
   // This is array indexing not an array declaration because a braced-init-list
   // is not syntactically a constant-expression.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13368,62 +13368,63 @@
                             << ND->getDeclName());
 }
 
-void Sema::CheckArrayAccess(const Expr *expr) {
-  int AllowOnePastEnd = 0;
-  while (expr) {
-    expr = expr->IgnoreParenImpCasts();
-    switch (expr->getStmtClass()) {
-      case Stmt::ArraySubscriptExprClass: {
-        const ArraySubscriptExpr *ASE = cast<ArraySubscriptExpr>(expr);
-        CheckArrayAccess(ASE->getBase(), ASE->getIdx(), ASE,
-                         AllowOnePastEnd > 0);
-        expr = ASE->getBase();
-        break;
-      }
-      case Stmt::MemberExprClass: {
-        expr = cast<MemberExpr>(expr)->getBase();
-        break;
-      }
-      case Stmt::OMPArraySectionExprClass: {
-        const OMPArraySectionExpr *ASE = cast<OMPArraySectionExpr>(expr);
-        if (ASE->getLowerBound())
-          CheckArrayAccess(ASE->getBase(), ASE->getLowerBound(),
-                           /*ASE=*/nullptr, AllowOnePastEnd > 0);
-        return;
-      }
-      case Stmt::UnaryOperatorClass: {
-        // Only unwrap the * and & unary operators
-        const UnaryOperator *UO = cast<UnaryOperator>(expr);
-        expr = UO->getSubExpr();
-        switch (UO->getOpcode()) {
-          case UO_AddrOf:
-            AllowOnePastEnd++;
-            break;
-          case UO_Deref:
-            AllowOnePastEnd--;
-            break;
-          default:
-            return;
-        }
-        break;
-      }
-      case Stmt::ConditionalOperatorClass: {
-        const ConditionalOperator *cond = cast<ConditionalOperator>(expr);
-        if (const Expr *lhs = cond->getLHS())
-          CheckArrayAccess(lhs);
-        if (const Expr *rhs = cond->getRHS())
-          CheckArrayAccess(rhs);
-        return;
-      }
-      case Stmt::CXXOperatorCallExprClass: {
-        const auto *OCE = cast<CXXOperatorCallExpr>(expr);
-        for (const auto *Arg : OCE->arguments())
-          CheckArrayAccess(Arg);
-        return;
-      }
-      default:
-        return;
+void Sema::CheckArrayAccess(const Expr *expr, int AllowOnePastEnd) {
+  if (!expr)
+    return;
+
+  expr = expr->IgnoreParenCasts();
+  switch (expr->getStmtClass()) {
+  case Stmt::ArraySubscriptExprClass: {
+    const ArraySubscriptExpr *ASE = cast<ArraySubscriptExpr>(expr);
+    CheckArrayAccess(ASE->getBase(), ASE->getIdx(), ASE, AllowOnePastEnd > 0);
+    CheckArrayAccess(ASE->getBase(), AllowOnePastEnd);
+    return;
+  }
+  case Stmt::MemberExprClass: {
+    expr = cast<MemberExpr>(expr)->getBase();
+    CheckArrayAccess(expr, AllowOnePastEnd);
+    return;
+  }
+  case Stmt::OMPArraySectionExprClass: {
+    const OMPArraySectionExpr *ASE = cast<OMPArraySectionExpr>(expr);
+    if (ASE->getLowerBound())
+      CheckArrayAccess(ASE->getBase(), ASE->getLowerBound(),
+                       /*ASE=*/nullptr, AllowOnePastEnd > 0);
+    return;
+  }
+  case Stmt::UnaryOperatorClass: {
+    // Only unwrap the * and & unary operators
+    const UnaryOperator *UO = cast<UnaryOperator>(expr);
+    expr = UO->getSubExpr();
+    switch (UO->getOpcode()) {
+    case UO_AddrOf:
+      AllowOnePastEnd++;
+      break;
+    case UO_Deref:
+      AllowOnePastEnd--;
+      break;
+    default:
+      return;
     }
+    CheckArrayAccess(expr, AllowOnePastEnd);
+    return;
+  }
+  case Stmt::ConditionalOperatorClass: {
+    const ConditionalOperator *cond = cast<ConditionalOperator>(expr);
+    if (const Expr *lhs = cond->getLHS())
+      CheckArrayAccess(lhs, AllowOnePastEnd);
+    if (const Expr *rhs = cond->getRHS())
+      CheckArrayAccess(rhs, AllowOnePastEnd);
+    return;
+  }
+  case Stmt::CXXOperatorCallExprClass: {
+    const auto *OCE = cast<CXXOperatorCallExpr>(expr);
+    for (const auto *Arg : OCE->arguments())
+      CheckArrayAccess(Arg);
+    return;
+  }
+  default:
+    return;
   }
 }
 
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -11390,7 +11390,7 @@
   void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
                         const ArraySubscriptExpr *ASE=nullptr,
                         bool AllowOnePastEnd=true, bool IndexNegated=false);
-  void CheckArrayAccess(const Expr *E);
+  void CheckArrayAccess(const Expr *E, int AllowOnePastEnd=0);
   // Used to grab the relevant information from a FormatAttr and a
   // FunctionDeclaration.
   struct FormatStringInfo {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to