From d1e07e9c9ab00bfe6c6056fec1c88300bd897a2b Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Fri, 10 Oct 2014 18:57:10 +0000 Subject: [PATCH] CodeGen: FieldMemcpyizer didn't handle copies starting inside bitfields It's possible to construct cases where the first field we are trying to copy is in the middle of an IR field. In some complicated cases, we would fail to use an appropriate offset inside the object. Earlier builds of clang seemed to miscompile the code by copying an insufficient number of bytes. Up until now, we would assert: the copying offset was insufficiently aligned. This fixes PR21232. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@219524 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGClass.cpp | 20 ++++++++++++++++---- test/CodeGenCXX/pod-member-memcpys.cpp | 13 +++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp index 941e998bfd..49760853dc 100644 --- a/lib/CodeGen/CGClass.cpp +++ b/lib/CodeGen/CGClass.cpp @@ -796,13 +796,13 @@ namespace { addNextField(F); } - CharUnits getMemcpySize() const { + CharUnits getMemcpySize(uint64_t FirstByteOffset) const { unsigned LastFieldSize = LastField->isBitField() ? LastField->getBitWidthValue(CGF.getContext()) : CGF.getContext().getTypeSize(LastField->getType()); uint64_t MemcpySizeBits = - LastFieldOffset + LastFieldSize - FirstFieldOffset + + LastFieldOffset + LastFieldSize - FirstByteOffset + CGF.getContext().getCharWidth() - 1; CharUnits MemcpySize = CGF.getContext().toCharUnitsFromBits(MemcpySizeBits); @@ -818,19 +818,31 @@ namespace { CharUnits Alignment; + uint64_t FirstByteOffset; if (FirstField->isBitField()) { const CGRecordLayout &RL = CGF.getTypes().getCGRecordLayout(FirstField->getParent()); const CGBitFieldInfo &BFInfo = RL.getBitFieldInfo(FirstField); Alignment = CharUnits::fromQuantity(BFInfo.StorageAlignment); + // FirstFieldOffset is not appropriate for bitfields, + // it won't tell us what the storage offset should be and thus might not + // be properly aligned. + // + // Instead calculate the storage offset using the offset of the field in + // the struct type. + const llvm::DataLayout &DL = CGF.CGM.getDataLayout(); + FirstByteOffset = + DL.getStructLayout(RL.getLLVMType()) + ->getElementOffsetInBits(RL.getLLVMFieldNo(FirstField)); } else { Alignment = CGF.getContext().getDeclAlign(FirstField); + FirstByteOffset = FirstFieldOffset; } - assert((CGF.getContext().toCharUnitsFromBits(FirstFieldOffset) % + assert((CGF.getContext().toCharUnitsFromBits(FirstByteOffset) % Alignment) == 0 && "Bad field alignment."); - CharUnits MemcpySize = getMemcpySize(); + CharUnits MemcpySize = getMemcpySize(FirstByteOffset); QualType RecordTy = CGF.getContext().getTypeDeclType(ClassDecl); llvm::Value *ThisPtr = CGF.LoadCXXThis(); LValue DestLV = CGF.MakeNaturalAlignAddrLValue(ThisPtr, RecordTy); diff --git a/test/CodeGenCXX/pod-member-memcpys.cpp b/test/CodeGenCXX/pod-member-memcpys.cpp index 31d774c534..5bd8adfc1d 100644 --- a/test/CodeGenCXX/pod-member-memcpys.cpp +++ b/test/CodeGenCXX/pod-member-memcpys.cpp @@ -67,6 +67,13 @@ struct BitfieldMember2 { NonPOD np; }; +struct BitfieldMember3 { + virtual void f(); + int : 8; + int x : 1; + int y; +}; + struct InnerClassMember { struct { int a, b, c, d; @@ -174,6 +181,7 @@ CALL_AO(PackedMembers) CALL_CC(PackedMembers) CALL_CC(BitfieldMember2) +CALL_CC(BitfieldMember3) CALL_CC(ReferenceMember) CALL_CC(InnerClassMember) CALL_CC(BitfieldMember) @@ -243,6 +251,11 @@ CALL_CC(Basic) // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 8{{.*}}) // CHECK: ret void +// BitfieldMember3 copy-constructor: +// CHECK-LABEL: define linkonce_odr void @_ZN15BitfieldMember3C2ERKS_(%struct.BitfieldMember3* %this, %struct.BitfieldMember3* dereferenceable({{[0-9]+}})) +// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* %4, i64 8, i32 8, i1 false) +// CHECK: ret void + // BitfieldMember2 copy-constructor: // CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* dereferenceable({{[0-9]+}})) // CHECK-2: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 4, i1 false) -- 2.50.1