Skip to content

Commit

Permalink
Fix inconsistent base padding
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DutChen18 authored and yuri91 committed Oct 21, 2024
1 parent ff06041 commit 7e48864
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/CGClass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/CodeGen/CGRecordLayout.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
28 changes: 19 additions & 9 deletions clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down

0 comments on commit 7e48864

Please sign in to comment.