From: Faisal Vali <faisalv@yahoo.com>
Date: Sat, 14 Dec 2013 00:40:05 +0000 (+0000)
Subject: Quick-Fix pointer arithmetic when performing multi-D new-array initialization.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b7d369845fe03e330bafc9d7ae8c5f173e71a31b;p=clang

Quick-Fix pointer arithmetic when performing multi-D new-array initialization.

clang still doesn't emit the right llvm code when initializing multi-D arrays it seems.

For e.g. the following code would still crash for me on Windows 7, 64 bit:

auto f4 = new int[100][200][300]{{{1,2,3}, {4, 5, 6}}, {{10, 20, 30}}};

It seems that the final new loop that iterates through each outermost array and memsets it to zero gets confused with its final ptr arithmetic.

This patch ensures that it converts the pointer to the allocated type (int [200][300]) before incrementing it (instead of using the base type: 'int').

Richard somewhat squeamishly approved the patch (as a quick fix to potentially make it into 3.4) - while exhorting for a more optimized fix in the future. http://llvm-reviews.chandlerc.com/D2398

Thanks Richard!

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

diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 7d5dc4c42c..2dba75e4d4 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -864,9 +864,29 @@ CodeGenFunction::EmitNewArrayInitializer(const CXXNewExpr *E,
     cleanupDominator->eraseFromParent();
   }
 
-  // Advance to the next element.
-  llvm::Value *nextPtr = Builder.CreateConstGEP1_32(curPtr, 1, "array.next");
-
+  // FIXME: The code below intends to initialize the individual array base
+  // elements, one at a time - but when dealing with multi-dimensional arrays -
+  // the pointer arithmetic can get confused - so the fix below entails casting
+  // to the allocated type to ensure that we get the pointer arithmetic right.
+  // It seems like the right approach here, it to really initialize the
+  // individual array base elements one at a time since it'll generate less
+  // code. I think the problem is that the wrong type is being passed into
+  // StoreAnyExprIntoOneUnit, but directly fixing that doesn't really work,
+  // because the Init expression has the wrong type at this point.
+  // So... this is ok for a quick fix, but we can and should do a lot better
+  // here long-term.
+
+  // Advance to the next element by adjusting the pointer type as necessary.
+  // For new int[10][20][30], alloc type is int[20][30], base type is 'int'.
+  QualType AllocType = E->getAllocatedType();
+  llvm::Type *AllocPtrTy = ConvertTypeForMem(AllocType)->getPointerTo(
+      curPtr->getType()->getPointerAddressSpace());
+  llvm::Value *curPtrAllocTy = Builder.CreateBitCast(curPtr, AllocPtrTy);
+  llvm::Value *nextPtrAllocTy =
+      Builder.CreateConstGEP1_32(curPtrAllocTy, 1, "array.next");
+  // Cast it back to the base type so that we can compare it to the endPtr.
+  llvm::Value *nextPtr =
+      Builder.CreateBitCast(nextPtrAllocTy, endPtr->getType());
   // Check whether we've gotten to the end of the array and, if so,
   // exit the loop.
   llvm::Value *isEnd = Builder.CreateICmpEQ(nextPtr, endPtr, "array.atend");
diff --git a/test/CodeGenCXX/cxx11-initializer-array-new.cpp b/test/CodeGenCXX/cxx11-initializer-array-new.cpp
index 23577bec66..8de88dcefb 100644
--- a/test/CodeGenCXX/cxx11-initializer-array-new.cpp
+++ b/test/CodeGenCXX/cxx11-initializer-array-new.cpp
@@ -99,8 +99,9 @@ void *q = new S[n][3]{ { 1, 2, 3 }, { 4, 5, 6 } };
 //   CHECK: icmp eq %[[S]]* %[[NEXT_INNER]], %[[END_INNER]]
 //   CHECK: br i1
 //
-// CHECK: %[[NEXT_OUTER:.*]] = getelementptr %[[S]]* %{{.*}}, i32 1
-// CHECK: icmp eq %[[S]]* %[[NEXT_OUTER]], %[[END_AS_S]]
+// CHECK: %[[NEXT_OUTER:.*]] = getelementptr [3 x %[[S]]]* %{{.*}}, i32 1
+// CHECK: %[[NEXT_OUTER_AS_S:.*]] = bitcast [3 x %[[S]]]* %[[NEXT_OUTER]] to %[[S]]*
+// CHECK: icmp eq %[[S]]* %[[NEXT_OUTER_AS_S]], %[[END_AS_S]]
 // CHECK: br i1
 //
 // CHECK: }