From: Ted Kremenek <kremenek@apple.com>
Date: Tue, 1 Mar 2011 18:41:00 +0000 (+0000)
Subject: For C++, enhance -Warray-bounds to recursively analyze array subscript accesses in... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3aea4dac830cb03d883a13ab30c28a3a53beca58;p=clang

For C++, enhance -Warray-bounds to recursively analyze array subscript accesses in ?: expressions.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@126766 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 633657a193..161ea296e8 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5199,7 +5199,7 @@ public:
                                                 unsigned ByteNo) const;
 
 private:  
-  void CheckArrayAccess(const ArraySubscriptExpr *E);
+  void CheckArrayAccess(const Expr *E);
   bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall);
   bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall);
 
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index d83809d708..cdcab31eef 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -3134,10 +3134,11 @@ void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
     << TRange << Op->getSourceRange();
 }
 
-void Sema::CheckArrayAccess(const clang::ArraySubscriptExpr *E) {
+static void CheckArrayAccess_Check(Sema &S,
+                                   const clang::ArraySubscriptExpr *E) {
   const Expr *BaseExpr = E->getBase()->IgnoreParenImpCasts();
   const ConstantArrayType *ArrayTy =
-    Context.getAsConstantArrayType(BaseExpr->getType());
+    S.Context.getAsConstantArrayType(BaseExpr->getType());
   if (!ArrayTy)
     return;
 
@@ -3145,7 +3146,7 @@ void Sema::CheckArrayAccess(const clang::ArraySubscriptExpr *E) {
   if (IndexExpr->isValueDependent())
     return;
   llvm::APSInt index;
-  if (!IndexExpr->isIntegerConstantExpr(index, Context))
+  if (!IndexExpr->isIntegerConstantExpr(index, S.Context))
     return;
 
   if (index.isUnsigned() || !index.isNegative()) {
@@ -3160,15 +3161,16 @@ void Sema::CheckArrayAccess(const clang::ArraySubscriptExpr *E) {
     if (index.slt(size))
       return;
 
-    DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
-                        PDiag(diag::warn_array_index_exceeds_bounds)
-                        << index.toString(10, true) << size.toString(10, true)
-                        << IndexExpr->getSourceRange());
+    S.DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
+                          S.PDiag(diag::warn_array_index_exceeds_bounds)
+                            << index.toString(10, true)
+                            << size.toString(10, true)
+                            << IndexExpr->getSourceRange());
   } else {
-    DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
-                        PDiag(diag::warn_array_index_precedes_bounds)
-                          << index.toString(10, true)
-                          << IndexExpr->getSourceRange());
+    S.DiagRuntimeBehavior(E->getBase()->getLocStart(), BaseExpr,
+                          S.PDiag(diag::warn_array_index_precedes_bounds)
+                            << index.toString(10, true)
+                            << IndexExpr->getSourceRange());
   }
 
   const NamedDecl *ND = NULL;
@@ -3177,8 +3179,29 @@ void Sema::CheckArrayAccess(const clang::ArraySubscriptExpr *E) {
   if (const MemberExpr *ME = dyn_cast<MemberExpr>(BaseExpr))
     ND = dyn_cast<NamedDecl>(ME->getMemberDecl());
   if (ND)
-    DiagRuntimeBehavior(ND->getLocStart(), BaseExpr,
-                        PDiag(diag::note_array_index_out_of_bounds)
-                          << ND->getDeclName());
+    S.DiagRuntimeBehavior(ND->getLocStart(), BaseExpr,
+                          S.PDiag(diag::note_array_index_out_of_bounds)
+                            << ND->getDeclName());
+}
+
+void Sema::CheckArrayAccess(const Expr *expr) {
+  while (true)
+    switch (expr->getStmtClass()) {
+      case Stmt::ParenExprClass:
+        expr = cast<ParenExpr>(expr)->getSubExpr();
+        continue;
+      case Stmt::ArraySubscriptExprClass:
+        CheckArrayAccess_Check(*this, cast<ArraySubscriptExpr>(expr));
+        return;
+      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;
+      }
+      default:
+        return;
+    }
 }
-
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index bc2251a4e0..82ec6ee6e1 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -301,8 +301,7 @@ void Sema::DefaultLvalueConversion(Expr *&E) {
   if (T.hasQualifiers())
     T = T.getUnqualifiedType();
 
-  if (const ArraySubscriptExpr *ae = dyn_cast<ArraySubscriptExpr>(E))
-    CheckArrayAccess(ae);
+  CheckArrayAccess(E);
   
   E = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue,
                                E, 0, VK_RValue);
@@ -7432,9 +7431,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, Expr *&RHS,
   }
   
   // Check for trivial buffer overflows.
-  if (const ArraySubscriptExpr *ae
-      = dyn_cast<ArraySubscriptExpr>(LHS->IgnoreParenCasts()))
-    CheckArrayAccess(ae);
+  CheckArrayAccess(LHS->IgnoreParenCasts());
   
   // C99 6.5.16p3: The type of an assignment expression is the type of the
   // left operand unless the left operand has qualified type, in which case
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 79e6d3a646..61f18795ba 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -2036,8 +2036,7 @@ Sema::PerformImplicitConversion(Expr *&From, QualType ToType,
     }
 
     // Check for trivial buffer overflows.
-    if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(From))
-      CheckArrayAccess(AE);
+    CheckArrayAccess(From);
 
     FromType = FromType.getUnqualifiedType();
     From = ImplicitCastExpr::Create(Context, FromType, CK_LValueToRValue,
diff --git a/test/SemaCXX/array-bounds.cpp b/test/SemaCXX/array-bounds.cpp
index 80646c7190..20451bf2f2 100644
--- a/test/SemaCXX/array-bounds.cpp
+++ b/test/SemaCXX/array-bounds.cpp
@@ -120,3 +120,11 @@ int test_pr9296() {
     return array[true]; // no-warning
 }
 
+int test_sizeof_as_condition(int flag) {
+  int arr[2] = { 0, 0 }; // expected-note {{array 'arr' declared here}}
+  if (flag) 
+    return sizeof(char) != sizeof(char) ? arr[2] : arr[1];
+  return sizeof(char) == sizeof(char) ? arr[2] : arr[1]; // expected-warning {{array index of '2' indexes past the end of an array (that contains 2 elements)}}
+}
+
+