[Sema] Diagnose array access preceding the array bounds even when the base type is incomplete.

[riccibruno]

Summary

When the type of the base expression after IgnoreParenCasts is incomplete,
it is still possible to diagnose an array access which precedes the array bounds.

This is a follow-up on D55862 which added an early return when the type of
the base expression after IgnoreParenCasts was incomplete.

[riccibruno]

Friendly ping ?

[efriedma]

LGTM

cfe/trunk/lib/Sema/SemaChecking.cpp

void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
   BaseExpr = BaseExpr->IgnoreParenCasts();
   const ConstantArrayType *ArrayTy =
       Context.getAsConstantArrayType(BaseExpr->getType());
 
   if (!ArrayTy)
     return;
 
   const Type *BaseType = ArrayTy->getElementType().getTypePtr();
-  // It is possible that the type of the base expression after IgnoreParenCasts
-  // is incomplete, even though the type of the base expression before
-  // IgnoreParenCasts is complete (see PR39746 for an example). In this case we
-  // have no information about whether the array access is out-of-bounds.
-  if (BaseType->isIncompleteType())
-    return;
 
   Expr::EvalResult Result;
   if (!IndexExpr->EvaluateAsInt(Result, Context, Expr::SE_AllowSideEffects))
     return;
 
   llvm::APSInt index = Result.Val.getInt();
   if (IndexNegated)
     index = -index;
 
   const NamedDecl *ND = nullptr;
   if (const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(BaseExpr))
     ND = DRE->getDecl();
   if (const MemberExpr *ME = dyn_cast<MemberExpr>(BaseExpr))
     ND = ME->getMemberDecl();
 
   if (index.isUnsigned() || !index.isNegative()) {
+    // It is possible that the type of the base expression after
+    // IgnoreParenCasts is incomplete, even though the type of the base
+    // expression before IgnoreParenCasts is complete (see PR39746 for an
+    // example). In this case we have no information about whether the array
+    // access exceeds the array bounds. However we can still diagnose an array
+    // access which precedes the array bounds.
+    if (BaseType->isIncompleteType())
+      return;
+
     llvm::APInt size = ArrayTy->getSize();
     if (!size.isStrictlyPositive())
       return;
 
     if (BaseType != EffectiveType) {
       // Make sure we're comparing apples to apples when comparing index to size
       uint64_t ptrarith_typesize = Context.getTypeSize(EffectiveType);
       uint64_t array_typesize = Context.getTypeSize(BaseType);

cfe/trunk/test/SemaCXX/array-bounds.cpp

 };
 struct multi_s multi2[4]; // expected-note {{array 'multi2' declared here}}
 int test_struct_multiarray() {
   return multi2[4].arr[0]; // expected-warning {{array index 4 is past the end of the array (which contains 4 elements)}}
 }
 
 namespace PR39746 {
   struct S;
-  extern S xxx[2];
+  extern S xxx[2]; // expected-note {{array 'xxx' declared here}}
   class C {};
 
   C &f() { return reinterpret_cast<C *>(xxx)[1]; } // no-warning
+  // We have no info on whether this is out-of-bounds.
   C &g() { return reinterpret_cast<C *>(xxx)[2]; } // no-warning
+  // We can still diagnose this.
+  C &h() { return reinterpret_cast<C *>(xxx)[-1]; } // expected-warning {{array index -1 is before the beginning of the array}}
 }