[GlobalOpt] Test array indices inside structs for out-of-bounds accesses

[dmgreen]

Summary

First go at fixing PR38309. We now, from clang, can turn arrays of

static short g_data[] = {16, 16, 16, 16, 16, 16, 16, 16, 0, 0, 0, 0, 0, 0, 0, 0};

into structs of the form

@g_data = internal global <{ [8 x i16], [8 x i16] }> ...

GlobalOpt can incorrectly SROA it, not realising that the access to the first
element may overflow into the second.

I believe this makes the globalsra-partial.ll test case invalid as the %i value
could be out of bounds. I've re-purposed it as a negative test for this case.

[efriedma]

lib/Transforms/IPO/GlobalOpt.cpp

382

Do we need to do the same GEP validation here?

414

"GEPI.isSequential() || GEPI.isStruct()" is always true.

[dmgreen]

Rewrote a little to do the same checks on following GEPs.

[efriedma]

LGTM

lib/Transforms/IPO/GlobalOpt.cpp

static bool isSafeSROAElementUse(Value *V) {
   if (StoreInst *SI = dyn_cast<StoreInst>(I))
     return SI->getOperand(0) != V;
 
   // Otherwise, it must be a GEP.
   GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(I);
   if (!GEPI) return false;
 
   if (GEPI->getNumOperands() < 3 || !isa<Constant>(GEPI->getOperand(1)) ||
       !cast<Constant>(GEPI->getOperand(1))->isNullValue())
     return false;
 
   for (User *U : GEPI->users())
     if (!isSafeSROAElementUse(U))
       return false;
   return true;
 }
 
static bool IsUserOfGlobalSafeForSRA(User *U, GlobalValue *GV) {
   if (U->getNumOperands() < 3 || !isa<Constant>(U->getOperand(1)) ||
       !cast<Constant>(U->getOperand(1))->isNullValue() ||
       !isa<ConstantInt>(U->getOperand(2)))
     return false;
 
   gep_type_iterator GEPI = gep_type_begin(U), E = gep_type_end(U);
   ++GEPI;  // Skip over the pointer index.
 
-  // If this is a use of an array allocation, do a bit more checking for sanity.
-  if (GEPI.isSequential()) {
+  // If this is a use of an array allocation (possibly inside a struct), do a
+  // bit more checking for sanity.
+  if (GEPI.isSequential() || GEPI.isStruct()) {
     ConstantInt *Idx = cast<ConstantInt>(U->getOperand(2));
 
     // Check to make sure that index falls within the array.  If not,
     // something funny is going on, so we won't do the optimization.
     //
     if (GEPI.isBoundedSequential() &&
         Idx->getZExtValue() >= GEPI.getSequentialNumElements())
       return false;

test/Transforms/GlobalOpt/globalsra-partial.ll

-; In this case, the global can only be broken up by one level.
+; In this case, the global cannot be merged as i may be out of range
 
 ; RUN: opt < %s -globalopt -S | FileCheck %s
 target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
 
 @G = internal global { i32, [4 x float] } zeroinitializer               ; <{ i32, [4 x float] }*> [#uses=3]
 
-; CHECK-NOT: 12345
+; CHECK: @G = internal unnamed_addr global { i32, [4 x float] }
+; CHECK: 12345
 define void @onlystore() {
         store i32 12345, i32* getelementptr ({ i32, [4 x float] }, { i32, [4 x float] }* @G, i32 0, i32 0)
         ret void
 }
 
 define void @storeinit(i32 %i) {
         %Ptr = getelementptr { i32, [4 x float] }, { i32, [4 x float] }* @G, i32 0, i32 1, i32 %i             ; <float*> [#uses=1]
         store float 1.000000e+00, float* %Ptr
         ret void
 }
 
 define float @readval(i32 %i) {
         %Ptr = getelementptr { i32, [4 x float] }, { i32, [4 x float] }* @G, i32 0, i32 1, i32 %i             ; <float*> [#uses=1]
         %V = load float, float* %Ptr           ; <float> [#uses=1]
         ret float %V
 }