Fix issue 726: Wrong alignment for struct fields on x86_64/amd64

ir/iraggr.cpp

@@ -28,7 +28,6 @@
 IrAggr::IrAggr(AggregateDeclaration* aggr)
 :   aggrdecl(aggr),
     type(aggr->type),
-    packed((type->ty == Tstruct) ? type->alignsize() == 1 : false),
     // above still need to be looked at
     init(0),
     constInit(0),
@@ -231,9 +230,9 @@ llvm::Constant* IrAggr::createInitializerConstant(
         for (itr = constants.begin(); itr != end; ++itr)
             types.push_back((*itr)->getType());
         if (!initializerType)
-            initializerType = LLStructType::get(gIR->context(), types, packed);
+            initializerType = LLStructType::get(gIR->context(), types, isPacked());
         else
-            initializerType->setBody(types, packed);
+            initializerType->setBody(types, isPacked());
     }
 
     // build constant
@@ -258,10 +257,6 @@ void IrAggr::addFieldInitializers(
         }
     }
 
-    const bool packed = (type->ty == Tstruct)
-        ? type->alignsize() == 1
-        : false;
-
     // Build up vector with one-to-one mapping to field indices.
     const size_t n = decl->fields.dim;
     llvm::SmallVector<VarInitConst, 16> data(n);
@@ -342,7 +337,7 @@ void IrAggr::addFieldInitializers(
 
         // get next aligned offset for this field
         size_t alignedoffset = offset;
-        if (!packed)
+        if (!isPacked())
         {
             alignedoffset = realignOffset(alignedoffset, vd->type);
         }

ir/iraggr.h

@@ -41,9 +41,6 @@ struct IrAggr
     /// Aggregate D type.
     Type* type;
 
-    /// true only for: align(1) struct S { ... }
-    bool packed;
-
     //////////////////////////////////////////////////////////////////////////
 
     /// Create the __initZ symbol lazily.
@@ -138,6 +135,9 @@ protected:
     /// Create the Interface[] interfaces ClassInfo field initializer.
     LLConstant* getClassInfoInterfaces();
 
+    /// Returns true, if the LLVM struct type for the aggregate is declared as packed
+    bool isPacked() const;
+
 private:
     /// Recursively adds all the initializers for the given aggregate and, in
     /// case of a class type, all its base classes.

ir/irclass.cpp

@@ -401,6 +401,11 @@ llvm::GlobalVariable * IrAggr::getInterfaceVtbl(BaseClass * b, bool new_instance
     return GV;
 }
 
+bool IrAggr::isPacked() const
+{
+    return static_cast<IrTypeAggr*>(type->ctype)->packed;
+}
+
 //////////////////////////////////////////////////////////////////////////////
 
 LLConstant * IrAggr::getClassInfoInterfaces()

ir/irtypeaggr.cpp

@@ -245,6 +245,18 @@ IrTypeAggr::IrTypeAggr(AggregateDeclaration * ad)
 {
 }
 
+bool IrTypeAggr::isPacked(AggregateDeclaration* ad)
+{
+    for (unsigned i = 0; i < ad->fields.dim; i++)
+    {
+        VarDeclaration* vd = static_cast<VarDeclaration*>(ad->fields.data[i]);
+        unsigned a = vd->type->alignsize() - 1;
+        if (((vd->offset + a) & ~a) != vd->offset)
+            return true;
+    }
+    return false;
+}
+
 void IrTypeAggr::getMemberLocation(VarDeclaration* var, unsigned& fieldIndex,
     unsigned& byteOffset) const
 {

ir/irtypeaggr.h

@@ -68,10 +68,17 @@ public:
     /// used for resolving forward references.
     llvm::DIType diCompositeType;
 
+    /// true, if the LLVM struct type for the aggregate is declared as packed
+    bool packed;
+
 protected:
     ///
     IrTypeAggr(AggregateDeclaration* ad);
 
+    /// Returns true, if the LLVM struct type for the aggregate must be declared
+    /// as packed.
+    static bool isPacked(AggregateDeclaration* ad);
+
     /// AggregateDeclaration this type represents.
     AggregateDeclaration* aggr;
 

ir/irtypeclass.cpp

@@ -96,7 +96,14 @@ IrTypeClass* IrTypeClass::get(ClassDeclaration* cd)
     LOG_SCOPE;
     IF_LOG Logger::println("Instance size: %u", cd->structsize);
 
-    AggrTypeBuilder builder(false);
+    // This class may contain an align declaration. See issue 726.
+    t->packed = false;
+    for (ClassDeclaration *base = cd; base != 0 && !t->packed; base = base->baseClass)
+    {
+        t->packed = isPacked(base);
+    }
+
+    AggrTypeBuilder builder(t->packed);
 
     // add vtbl
     builder.addType(llvm::PointerType::get(t->vtbl_type, 0), Target::ptrsize);
@@ -127,7 +134,7 @@ IrTypeClass* IrTypeClass::get(ClassDeclaration* cd)
         fatal();
 
     // set struct body and copy GEP indices
-    isaStruct(t->type)->setBody(builder.defaultTypes(), false);
+    isaStruct(t->type)->setBody(builder.defaultTypes(), t->packed);
     t->varGEPIndices = builder.varGEPIndices();
 
     // VTBL

ir/irtypestruct.cpp

@@ -50,11 +50,18 @@ IrTypeStruct* IrTypeStruct::get(StructDeclaration* sd)
     if (sd->sizeok != SIZEOKdone)
         return t;
 
+    t->packed = sd->alignment == 1;
+    if (!t->packed)
+    {
+        // Unfortunately, the previous check is not enough in case the struct
+        // contains an align declaration. See issue 726.
+        t->packed = isPacked(sd);
+    }
 
-    AggrTypeBuilder builder(sd->alignment == 1);
+    AggrTypeBuilder builder(t->packed);
     builder.addAggregate(sd);
     builder.addTailPadding(sd->structsize);
-    isaStruct(t->type)->setBody(builder.defaultTypes(), sd->alignment == 1);
+    isaStruct(t->type)->setBody(builder.defaultTypes(), t->packed);
     t->varGEPIndices = builder.varGEPIndices();
 
     IF_LOG Logger::cout() << "final struct type: " << *t->type << std::endl;

tests/d2/dmd-testsuite

@@ -1 +1 @@
-Subproject commit 9609c53f2ab74890200b5134b1f45597a2019189
+Subproject commit 358b9d536f3c412524e8038ed610e3804dd140c9