Merge pull request #4736 from kinke/fix4734

Get rid of cycles in DtoType()

Author: kinke

.github/workflows/main.yml

@@ -94,9 +94,9 @@ jobs:
         with:
           submodules: true
           fetch-depth: 50
-      - name: 'macOS 14: Switch to Xcode 16 Beta 4'
+      - name: 'macOS 14: Switch to Xcode 16 Beta 5'
         if: matrix.os == 'macos-14'
-        run: sudo xcode-select -switch /Applications/Xcode_16_beta_4.app
+        run: sudo xcode-select -switch /Applications/Xcode_16_beta_5.app
       - name: Install prerequisites
         uses: ./.github/actions/1-setup
         with:

CHANGELOG.md

@@ -10,6 +10,7 @@
 
 #### Bug fixes
 - Fix potentially corrupt IR layouts for bit fields. (#4646, #4708)
+- Fix potentially corrupt IR layouts for explicitly under-aligned aggregates, a regression introduced in LDC v1.31. (#4734, #4736)
 
 # LDC 1.39.0 (2024-07-04)
 

ir/iraggr.cpp

@@ -11,6 +11,7 @@
 
 #include "dmd/aggregate.h"
 #include "dmd/declaration.h"
+#include "dmd/errors.h"
 #include "dmd/expression.h"
 #include "dmd/identifier.h"
 #include "dmd/init.h"
@@ -217,8 +218,12 @@ IrAggr::createInitializerConstant(const VarInitMap &explicitInitializers) {
 
   // tail padding?
   const size_t structsize = aggrdecl->size(Loc());
-  if (offset < structsize)
+  if (offset < structsize) {
     add_zeros(constants, offset, structsize);
+  } else if (offset > structsize) {
+    error(Loc(), "ICE: IR aggregate constant size exceeds the frontend size");
+    fatal();
+  }
 
   // get LL field types
   llvm::SmallVector<llvm::Type *, 16> types;

ir/irtype.cpp

@@ -117,25 +117,12 @@ IrTypePointer *IrTypePointer::get(Type *dt) {
   auto &ctype = getIrType(dt);
   assert(!ctype);
 
-  LLType *elemType;
-  unsigned addressSpace = 0;
-  if (dt->ty == TY::Tnull) {
-    elemType = llvm::Type::getInt8Ty(getGlobalContext());
-  } else {
-    elemType = DtoMemType(dt->nextOf());
-    if (dt->nextOf()->ty == TY::Tfunction) {
-      addressSpace = gDataLayout->getProgramAddressSpace();
-    }
-
-    // DtoType could have already created the same type, e.g. for
-    // dt == Node* in struct Node { Node* n; }.
-    if (ctype) {
-      return ctype->isPointer();
-    }
-  }
+  unsigned addressSpace =
+      dt->ty == TY::Tpointer && dt->nextOf()->ty == TY::Tfunction
+          ? gDataLayout->getProgramAddressSpace()
+          : 0;
 
-  auto t =
-      new IrTypePointer(dt, llvm::PointerType::get(elemType, addressSpace));
+  auto t = new IrTypePointer(dt, getOpaquePtrType(addressSpace));
   ctype = t;
   return t;
 }
@@ -173,15 +160,9 @@ IrTypeArray *IrTypeArray::get(Type *dt) {
   auto &ctype = getIrType(dt);
   assert(!ctype);
 
-  LLType *elemType = DtoMemType(dt->nextOf());
-
-  // Could have already built the type as part of a struct forward reference,
-  // just as for pointers.
-  if (!ctype) {
-    llvm::Type *types[] = {DtoSize_t(), llvm::PointerType::get(elemType, 0)};
-    LLType *at = llvm::StructType::get(getGlobalContext(), types, false);
-    ctype = new IrTypeArray(dt, at);
-  }
+  llvm::Type *types[] = {DtoSize_t(), getOpaquePtrType()};
+  LLType *at = llvm::StructType::get(getGlobalContext(), types, false);
+  ctype = new IrTypeArray(dt, at);
 
   return ctype->isArray();
 }

ir/irtypeaggr.cpp

@@ -212,17 +212,18 @@ void AggrTypeBuilder::addAggregate(
     // add default type
     m_defaultTypes.push_back(llType);
 
-    unsigned fieldAlignment, fieldSize;
     if (!llType->isSized()) {
-      // forward reference in a cycle or similar, we need to trust the D type
-      fieldAlignment = DtoAlignment(vd->type);
-      fieldSize = af.size;
-    } else {
-      fieldAlignment = getABITypeAlign(llType);
-      fieldSize = getTypeAllocSize(llType);
-      assert(fieldSize <= af.size);
+      error(vd->loc,
+            "unexpected IR type forward declaration for aggregate member of "
+            "type `%s`. This is an ICE, please file an LDC issue.",
+            vd->type->toPrettyChars());
+      fatal();
     }
 
+    const unsigned fieldAlignment = getABITypeAlign(llType);
+    const unsigned fieldSize = getTypeAllocSize(llType);
+    assert(fieldSize <= af.size);
+
     // advance offset to right past this field
     if (!m_packed) {
       assert(fieldAlignment);
@@ -278,10 +279,11 @@ IrTypeAggr::IrTypeAggr(AggregateDeclaration *ad)
              LLStructType::create(gIR->context(), ad->toPrettyChars())),
       aggr(ad) {}
 
-unsigned IrTypeAggr::getMemberLocation(VarDeclaration *var, bool& isFieldIdx) const {
+unsigned IrTypeAggr::getMemberLocation(VarDeclaration *var, bool& isFieldIdx) {
   // Note: The interface is a bit more general than what we actually return.
   // Specifically, the frontend offset information we use for overlapping
   // fields is always based at the object start.
+  const auto &varGEPIndices = getVarGEPIndices();
   auto it = varGEPIndices.find(var);
   if (it != varGEPIndices.end()) {
     isFieldIdx = true;

ir/irtypeaggr.h

@@ -72,7 +72,7 @@ class IrTypeAggr : public IrType {
   /// Returns the index of the field in the LLVM struct type that corresponds
   /// to the given member variable, plus the offset to the actual field start
   /// due to overlapping (union) fields, if any.
-  unsigned getMemberLocation(VarDeclaration *var, bool& isFieldIdx) const;
+  unsigned getMemberLocation(VarDeclaration *var, bool& isFieldIdx);
 
 protected:
   ///
@@ -90,6 +90,8 @@ class IrTypeAggr : public IrType {
   /// the field index of a variable in the frontend, it only stores the byte
   /// offset.
   VarGEPIndices varGEPIndices;
+
+  virtual const VarGEPIndices &getVarGEPIndices() { return varGEPIndices; }
 };
 
 // A helper for aggregating consecutive bit fields to a group.

ir/irtypeclass.cpp

@@ -12,6 +12,7 @@
 #include "dmd/aggregate.h"
 #include "dmd/declaration.h"
 #include "dmd/dsymbol.h"
+#include "dmd/errors.h"
 #include "dmd/mtype.h"
 #include "dmd/target.h"
 #include "dmd/template.h"
@@ -60,24 +61,34 @@ void IrTypeClass::addClassData(AggrTypeBuilder &builder,
 }
 
 IrTypeClass *IrTypeClass::get(ClassDeclaration *cd) {
+  const auto t = new IrTypeClass(cd);
+  getIrType(cd->type) = t;
+  return t;
+}
+
+llvm::Type *IrTypeClass::getLLType() { return getOpaquePtrType(); }
+
+// Lazily build the actual IR struct type when needed.
+// Note that it is this function that initializes most fields!
+llvm::Type *IrTypeClass::getMemoryLLType() {
+  if (!isaStruct(type)->isOpaque())
+    return type;
+
   IF_LOG Logger::println("Building class type %s @ %s", cd->toPrettyChars(),
                          cd->loc.toChars());
   LOG_SCOPE;
 
-  const auto t = new IrTypeClass(cd);
-  getIrType(cd->type) = t;
-
   const unsigned instanceSize = cd->structsize;
   IF_LOG Logger::println("Instance size: %u", instanceSize);
 
   AggrTypeBuilder builder;
 
   // add vtbl
-  builder.addType(llvm::PointerType::get(t->vtbl_type, 0), target.ptrsize);
+  builder.addType(llvm::PointerType::get(vtbl_type, 0), target.ptrsize);
 
   if (cd->isInterfaceDeclaration()) {
     // interfaces are just a vtable
-    t->num_interface_vtbls =
+    num_interface_vtbls =
         cd->vtblInterfaces ? cd->vtblInterfaces->length : 0;
   } else {
     // classes have monitor and fields
@@ -87,34 +98,48 @@ IrTypeClass *IrTypeClass::get(ClassDeclaration *cd) {
     }
 
     // add data members recursively
-    t->addClassData(builder, cd);
+    addClassData(builder, cd);
 
     // add tail padding
     if (instanceSize) // can be 0 for opaque classes
       builder.addTailPadding(instanceSize);
   }
 
   // set struct body and copy GEP indices
-  isaStruct(t->type)->setBody(builder.defaultTypes(), builder.isPacked());
-  t->varGEPIndices = builder.varGEPIndices();
+  isaStruct(type)->setBody(builder.defaultTypes(), builder.isPacked());
+  varGEPIndices = builder.varGEPIndices();
 
-  IF_LOG Logger::cout() << "class type: " << *t->type << std::endl;
-
-  return t;
-}
+  if (!cd->isInterfaceDeclaration() && instanceSize &&
+      getTypeAllocSize(type) != instanceSize) {
+    error(cd->loc, "ICE: class IR size does not match the frontend size");
+    fatal();
+  }
 
-llvm::Type *IrTypeClass::getLLType() { return llvm::PointerType::get(type, 0); }
+  IF_LOG Logger::cout() << "class type: " << *type << std::endl;
 
-llvm::Type *IrTypeClass::getMemoryLLType() { return type; }
+  return type;
+}
 
 size_t IrTypeClass::getInterfaceIndex(ClassDeclaration *inter) {
+  getMemoryLLType(); // lazily resolve
+
   auto it = interfaceMap.find(inter);
   if (it == interfaceMap.end()) {
     return ~0UL;
   }
   return it->second;
 }
 
+unsigned IrTypeClass::getNumInterfaceVtbls() {
+  getMemoryLLType(); // lazily resolve
+  return num_interface_vtbls;
+}
+
+const VarGEPIndices &IrTypeClass::getVarGEPIndices() {
+  getMemoryLLType(); // lazily resolve
+  return varGEPIndices;
+}
+
 void IrTypeClass::addInterfaceToMap(ClassDeclaration *inter, size_t index) {
   // don't duplicate work or overwrite indices
   if (interfaceMap.find(inter) != interfaceMap.end()) {

ir/irtypeclass.h

@@ -47,7 +47,7 @@ class IrTypeClass : public IrTypeAggr {
 
   /// Returns the number of interface implementations (vtables) in this
   /// class.
-  unsigned getNumInterfaceVtbls() { return num_interface_vtbls; }
+  unsigned getNumInterfaceVtbls();
 
 protected:
   ///
@@ -73,6 +73,8 @@ class IrTypeClass : public IrTypeAggr {
 
   //////////////////////////////////////////////////////////////////////////
 
+  const VarGEPIndices &getVarGEPIndices() override;
+
   /// Adds the data members for the given class to the type builder, including
   /// those inherited from base classes/interfaces.
   void addClassData(AggrTypeBuilder &builder, ClassDeclaration *currCd);

ir/irtypestruct.cpp

@@ -11,6 +11,7 @@
 
 #include "dmd/aggregate.h"
 #include "dmd/declaration.h"
+#include "dmd/errors.h"
 #include "dmd/init.h"
 #include "dmd/mtype.h"
 #include "dmd/template.h"
@@ -91,6 +92,11 @@ IrTypeStruct *IrTypeStruct::get(StructDeclaration *sd) {
     builder.addTailPadding(sd->structsize);
     isaStruct(t->type)->setBody(builder.defaultTypes(), builder.isPacked());
     t->varGEPIndices = builder.varGEPIndices();
+
+    if (getTypeAllocSize(t->type) != sd->structsize) {
+      error(sd->loc, "ICE: struct IR size does not match the frontend size");
+      fatal();
+    }
   }
 
   IF_LOG Logger::cout() << "final struct type: " << *t->type << std::endl;

tests/compilable/gh4734.d

@@ -0,0 +1,20 @@
+// RUN: %ldc -c %s
+
+align(1) struct Item {
+    KV v;
+    uint i;
+}
+
+struct KV {
+    align(1) S* s;
+    uint k;
+}
+
+struct S {
+    Table table;
+}
+
+struct Table {
+    char a;
+    Item v;
+}