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

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 28 16:45:48 PDT 2020


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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79052.260792.patch
Type: text/x-patch
Size: 969 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200428/19f18758/attachment-0001.bin>


More information about the cfe-commits mailing list