[clang] bf8b63e - [clang codegen] Fix alignment of "Address" for incomplete array pointer.

[Eli Friedman via cfe-commits]

Author: Eli Friedman
Date: 2020-06-23T17:16:17-07:00
New Revision: bf8b63ed296c1ecad03c83b798ffbfa039cbceb4

URL: https://github.com/llvm/llvm-project/commit/bf8b63ed296c1ecad03c83b798ffbfa039cbceb4
DIFF: https://github.com/llvm/llvm-project/commit/bf8b63ed296c1ecad03c83b798ffbfa039cbceb4.diff

LOG: [clang codegen] Fix alignment of "Address" for incomplete array pointer.

The code was assuming all incomplete types don't have meaningful
alignment, but incomplete arrays do have meaningful alignment.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45710

Differential Revision: https://reviews.llvm.org/D79052

Added: 
    

Modified: 
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/test/CodeGenCXX/alignment.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 7a9df700581e..f0ab5165584c 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -5990,6 +5990,9 @@ CharUnits CodeGenModule::getNaturalTypeAlignment(QualType T,
   if (TBAAInfo)
     *TBAAInfo = getTBAAAccessInfo(T);
 
+  // FIXME: This duplicates logic in ASTContext::getTypeAlignIfKnown. But
+  // that doesn't return the information we need to compute BaseInfo.
+
   // Honor alignment typedef attributes even on incomplete types.
   // We also honor them straight for C++ class types, even as pointees;
   // there's an expressivity gap here.
@@ -6001,32 +6004,46 @@ CharUnits CodeGenModule::getNaturalTypeAlignment(QualType T,
     }
   }
 
+  bool AlignForArray = T->isArrayType();
+
+  // Analyze the base element type, so we don't get confused by incomplete
+  // array types.
+  T = getContext().getBaseElementType(T);
+
+  if (T->isIncompleteType()) {
+    // We could try to replicate the logic from
+    // ASTContext::getTypeAlignIfKnown, but nothing uses the alignment if the
+    // type is incomplete, so it's impossible to test. We could try to reuse
+    // getTypeAlignIfKnown, but that doesn't return the information we need
+    // to set BaseInfo.  So just ignore the possibility that the alignment is
+    // greater than one.
+    if (BaseInfo)
+      *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
+    return CharUnits::One();
+  }
+
   if (BaseInfo)
     *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
-    Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is best.
+  // For C++ class pointees, we don't know whether we're pointing at a
+  // base or a complete object, so we generally need to use the
+  // non-virtual alignment.
+  const CXXRecordDecl *RD;
+  if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
+    Alignment = getClassPointerAlignment(RD);
   } else {
-    // For C++ class pointees, we don't know whether we're pointing at a
-    // base or a complete object, so we generally need to use the
-    // non-virtual alignment.
-    const CXXRecordDecl *RD;
-    if (forPointeeType && (RD = T->getAsCXXRecordDecl())) {
-      Alignment = getClassPointerAlignment(RD);
-    } else {
-      Alignment = getContext().getTypeAlignInChars(T);
-      if (T.getQualifiers().hasUnaligned())
-        Alignment = CharUnits::One();
-    }
+    Alignment = getContext().getTypeAlignInChars(T);
+    if (T.getQualifiers().hasUnaligned())
+      Alignment = CharUnits::One();
+  }
 
-    // Cap to the global maximum type alignment unless the alignment
-    // was somehow explicit on the type.
-    if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
-      if (Alignment.getQuantity() > MaxAlign &&
-          !getContext().isAlignmentRequired(T))
-        Alignment = CharUnits::fromQuantity(MaxAlign);
-    }
+  // Cap to the global maximum type alignment unless the alignment
+  // was somehow explicit on the type.
+  if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
+    if (Alignment.getQuantity() > MaxAlign &&
+        !getContext().isAlignmentRequired(T))
+      Alignment = CharUnits::fromQuantity(MaxAlign);
   }
   return Alignment;
 }

diff  --git a/clang/test/CodeGenCXX/alignment.cpp b/clang/test/CodeGenCXX/alignment.cpp
index 37509fcb4dd5..c9378bf20a47 100644
--- a/clang/test/CodeGenCXX/alignment.cpp
+++ b/clang/test/CodeGenCXX/alignment.cpp
@@ -308,4 +308,20 @@ namespace test1 {
     D d;
     AlignedArray result = d.bArray;
   }
+
+  // CHECK-LABEL: @_ZN5test11hEPA_NS_1BE
+  void h(B (*b)[]) {
+    // CHECK: [[RESULT:%.*]] = alloca [[ARRAY]], align 64
+    // CHECK: [[B_P:%.*]] = load [0 x [[B]]]*, [0 x [[B]]]**
+    // CHECK: [[ELEMENT_P:%.*]] = getelementptr inbounds [0 x [[B]]], [0 x [[B]]]* [[B_P]], i64 0
+    // CHECK: [[ARRAY_P:%.*]] = getelementptr inbounds [[B]], [[B]]* [[ELEMENT_P]], i32 0, i32 2
+    // CHECK: [[T0:%.*]] = bitcast [[ARRAY]]* [[RESULT]] to i8*
+    // CHECK: [[T1:%.*]] = bitcast [[ARRAY]]* [[ARRAY_P]] to i8*
+    // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 64 [[T0]], i8* align 16 [[T1]], i64 16, i1 false)
+    AlignedArray result = (*b)->bArray;
+  }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }