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

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added a reviewer: rjmccall.
Herald added a project: clang.
rsmith added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:176
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
+  if (T->getBaseElementTypeUnsafe()->isIncompleteType()) {
     Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is 
best.
----------------
I don't know if it matters in practice, but this is still wrong. An incomplete 
type can have a known alignment, for a case like `struct alignas(32) S;`. 
Perhaps we should remove this test entirely and call `getTypeAlignIfKnown` 
instead of `getTypeAlign[InChars]` below.


The code was assuming all incomplete types don't have meaningful alignment, but 
that clearly isn't true.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79052

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGenCXX/alignment.cpp


Index: clang/test/CodeGenCXX/alignment.cpp
===================================================================
--- clang/test/CodeGenCXX/alignment.cpp
+++ clang/test/CodeGenCXX/alignment.cpp
@@ -309,3 +309,7 @@
     AlignedArray result = d.bArray;
   }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -173,7 +173,7 @@
     *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
+  if (T->getBaseElementTypeUnsafe()->isIncompleteType()) {
     Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is 
best.
   } else {
     // For C++ class pointees, we don't know whether we're pointing at a



Index: clang/test/CodeGenCXX/alignment.cpp
===================================================================
--- clang/test/CodeGenCXX/alignment.cpp
+++ clang/test/CodeGenCXX/alignment.cpp
@@ -309,3 +309,7 @@
     AlignedArray result = d.bArray;
   }
 }
+
+// CHECK-LABEL: @_Z22incomplete_array_derefPA_i
+// CHECK: load i32, i32* {{%.*}}, align 4
+int incomplete_array_deref(int (*p)[]) { return (*p)[2]; }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -173,7 +173,7 @@
     *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
-  if (T->isIncompleteType()) {
+  if (T->getBaseElementTypeUnsafe()->isIncompleteType()) {
     Alignment = CharUnits::One(); // Shouldn't be used, but pessimistic is best.
   } else {
     // For C++ class pointees, we don't know whether we're pointing at a

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits