[Sema] Don't try to account for the size of an incomplete type in CheckArrayAccess

[riccibruno]

Summary

When checking that the array access is not out-of-bounds in CheckArrayAccess,
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
In this case we have no information about whether the array access is out-of-bounds,
and we should just bail-out instead. This fixes PR39746 which was caused by trying
to obtain the size of an incomplete type.

[efriedma]

lib/Sema/SemaChecking.cpp

12381 ↗

(On Diff #178807)

This comment is really confusing; the meaning of "base type" and "effective type" in this context isn't clear. Also, "effective type" is a term defined in the C standard to mean something else. Better to explicitly say you're talking about the type of the base expression before/after the call to IgnoreParenCasts.

12383 ↗

(On Diff #178807)

You don't need to explicitly pass nullptr here; there's a default argument.

test/SemaCXX/array-bounds.cpp

294 ↗

(On Diff #178807)

Not sure it actually makes sense to print a diagnostic here... at least, not this diagnostic. We don't know whether the result here is actually out of bounds.

[riccibruno]

test/SemaCXX/array-bounds.cpp

294 ↗

(On Diff #178807)

Right. I think that the right thing to do then is to not print anything
since we really don't have any information about whether this
is out of bounds.

[efriedma]

lib/Sema/SemaChecking.cpp

12357 ↗

(On Diff #178848)

Using getPointeeOrArrayElementType here is kind of confusing; instead, we can just call ArrayTy->getElementType().

[riccibruno]

Used ArrayTy->getElementType()

[efriedma]

LGTM

cfe/trunk/lib/Sema/SemaChecking.cpp

void Sema::CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
   IndexExpr = IndexExpr->IgnoreParenImpCasts();
   if (IndexExpr->isValueDependent())
     return;
 
   const Type *EffectiveType =
       BaseExpr->getType()->getPointeeOrArrayElementType();
   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()) {
     llvm::APInt size = ArrayTy->getSize();
     if (!size.isStrictlyPositive())
       return;
 
-    const Type *BaseType = BaseExpr->getType()->getPointeeOrArrayElementType();
     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);
       // Handle ptrarith_typesize being zero, such as when casting to void*
       if (!ptrarith_typesize) ptrarith_typesize = 1;
       if (ptrarith_typesize != array_typesize) {
         // There's a cast to a different size type involved

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

 
 struct multi_s {
   int arr[4];
 };
 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];
+  class C {};
+
+  C &f() { return reinterpret_cast<C *>(xxx)[1]; } // no-warning
+  C &g() { return reinterpret_cast<C *>(xxx)[2]; } // no-warning
+}