From 7e488640bee012a27313eca993bf1d43d9fac08f Mon Sep 17 00:00:00 2001
From: Chen Steenvoorden <dutchen18@gmail.com>
Date: Mon, 21 Oct 2024 13:57:49 +0200
Subject: [PATCH] Fix inconsistent base padding

When inlining fields of a direct base class, extra padding would
sometimes be added to ensure fields in the derived class are at the same
offset as they are in the base class, even if one is packed and the
other isn't.

While this extra padding makes sure that fields end up at the same byte
offset, the extra padding fields do shift the index of some fields. The
`ComputeNonVirtualBaseClassGepPath` function in `CGClass.cpp` did not
properly take into account the possibility these fields having a
different index when inlined into a derived class.

This is fixed by also adding base classes of the direct base of a
derived class into the derived class' `NonVirtualBases` map.
---
 clang/lib/CodeGen/CGClass.cpp               |  4 ++-
 clang/lib/CodeGen/CGRecordLayout.h          |  7 ++++++
 clang/lib/CodeGen/CGRecordLayoutBuilder.cpp | 28 ++++++++++++++-------
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index b172ed4a4a94..e2bcc491d1b1 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -39,6 +39,7 @@ ComputeNonVirtualBaseClassGepPath(CodeGenModule& CGM,
                                   const CXXRecordDecl *DerivedClass,
                                   llvm::ArrayRef<const CXXRecordDecl*> BasePath) {
   const CXXRecordDecl *RD = DerivedClass;
+  const CXXRecordDecl *TopRD = DerivedClass;
   // Compute the expected type of the GEP expression
   llvm::Type* ret = nullptr;
 
@@ -47,11 +48,12 @@ ComputeNonVirtualBaseClassGepPath(CodeGenModule& CGM,
     if(!BaseDecl->isEmpty() && !ASTLayout.getBaseClassOffset(BaseDecl).isZero())
     {
       // Get the layout.
-      const CGRecordLayout &Layout = CGM.getTypes().getCGRecordLayout(RD);
+      const CGRecordLayout &Layout = CGM.getTypes().getCGRecordLayout(TopRD);
       uint32_t index=Layout.getNonVirtualBaseLLVMFieldNo(BaseDecl);
 
       GEPIndexes.push_back(llvm::ConstantInt::get(CGM.Int32Ty, index));
       ret = Layout.getLLVMType()->getElementType(index);
+      TopRD = BaseDecl;
     }
     RD = BaseDecl;
   }
diff --git a/clang/lib/CodeGen/CGRecordLayout.h b/clang/lib/CodeGen/CGRecordLayout.h
index 856fb8dff198..5642753a8a80 100644
--- a/clang/lib/CodeGen/CGRecordLayout.h
+++ b/clang/lib/CodeGen/CGRecordLayout.h
@@ -236,6 +236,13 @@ class CGRecordLayout {
 
   typedef  llvm::DenseMap<const CXXRecordDecl *, unsigned>::const_iterator const_base_iterator;
 
+  const_base_iterator nvbases_begin() const {
+        return NonVirtualBases.begin();
+  }
+  const_base_iterator nvbases_end() const {
+        return NonVirtualBases.end();
+  }
+
   const_base_iterator vbases_begin() const {
         return CompleteObjectVirtualBases.begin();
   }
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index 308be561fc13..4b8ae5db2f6f 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -858,8 +858,22 @@ void CGRecordLowering::fillOutputFields() {
                 (Member->Kind == MemberInfo::Field && Member->FD && Member->FD->getType()->isStructureOrClassType() && !D->isUnion())))
     {
       assert(isa<llvm::StructType>(Member->Data));
+      llvm::DenseMap<unsigned, const CXXRecordDecl *> BaseNonVirtualBases;
+      unsigned BaseFieldIndex = 0;
       llvm::StructType* ST = cast<llvm::StructType>(Member->Data);
       CharUnits Offset = CharUnits::Zero();
+      DirectBase = ST;
+      if (Member->Kind == MemberInfo::Base)
+      {
+        DirectBaseLayout = &Types.getCGRecordLayout(Member->RD);
+        for (auto it = DirectBaseLayout->nvbases_begin(); it != DirectBaseLayout->nvbases_end(); ++it)
+          BaseNonVirtualBases.insert({it->second, it->first});
+      }
+      else
+      {
+        DirectBaseLayout = &Types.getCGRecordLayout(cast<RecordDecl>(Member->FD->getType()->getAsTagDecl()));
+        Fields[Member->FD->getCanonicalDecl()] = 0xffffffff;
+      }
       for (auto* el: ST->elements())
       {
         // Insert explicit padding where needed.
@@ -874,21 +888,17 @@ void CGRecordLowering::fillOutputFields() {
         }
         FieldTypes.push_back(el);
         Offset += getSize(el);
+        if (Member->Kind == MemberInfo::Base && BaseNonVirtualBases.count(BaseFieldIndex))
+          NonVirtualBases[BaseNonVirtualBases.lookup(BaseFieldIndex)] = FieldTypes.size() - 1;
+        BaseFieldIndex += 1;
       }
-      // For C-style inheritance, we cannot avoid padding after the "base"
-      if (Member->Kind != MemberInfo::Base && !ST->isPacked())
+      // We cannot avoid padding after the direct base
+      if (!ST->isPacked())
       {
           CharUnits Size = Offset.alignTo(getAlignment(ST));
           appendPaddingBytes(Size - Offset);
           Offset = Size;
       }
-      DirectBase = ST;
-      if (Member->Kind == MemberInfo::Base)
-        DirectBaseLayout = &Types.getCGRecordLayout(Member->RD);
-      else {
-        DirectBaseLayout = &Types.getCGRecordLayout(cast<RecordDecl>(Member->FD->getType()->getAsTagDecl()));
-        Fields[Member->FD->getCanonicalDecl()] = 0xffffffff;
-      }
       totalNumberOfNonVirtualBases = DirectBaseLayout->totalNumberOfNonVirtualBases;
       continue;
     }