r251783 - [MS ABI] Don't zero-initialize vbptrs in bases
David Majnemer via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 2 01:01:44 PST 2015
Author: majnemer
Date: Mon Nov 2 03:01:44 2015
New Revision: 251783
URL: http://llvm.org/viewvc/llvm-project?rev=251783&view=rev
Log:
[MS ABI] Don't zero-initialize vbptrs in bases
Certain CXXConstructExpr nodes require zero-initialization before a
constructor is called. We had a bug in the case where the constructor
is called on a virtual base: we zero-initialized the base's vbptr field.
A complementary bug is present in MSVC where no zero-initialization
occurs for the subobject at all.
This fixes PR25370.
Modified:
cfe/trunk/lib/CodeGen/CGCXXABI.cpp
cfe/trunk/lib/CodeGen/CGCXXABI.h
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
Modified: cfe/trunk/lib/CodeGen/CGCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.cpp?rev=251783&r1=251782&r2=251783&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.cpp Mon Nov 2 03:01:44 2015
@@ -326,3 +326,7 @@ CGCXXABI::emitTerminateForUnexpectedExce
CatchTypeInfo CGCXXABI::getCatchAllTypeInfo() {
return CatchTypeInfo{nullptr, 0};
}
+
+std::vector<CharUnits> CGCXXABI::getVBPtrOffsets(const CXXRecordDecl *RD) {
+ return std::vector<CharUnits>();
+}
Modified: cfe/trunk/lib/CodeGen/CGCXXABI.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCXXABI.h?rev=251783&r1=251782&r2=251783&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGCXXABI.h (original)
+++ cfe/trunk/lib/CodeGen/CGCXXABI.h Mon Nov 2 03:01:44 2015
@@ -425,6 +425,9 @@ public:
virtual size_t getSrcArgforCopyCtor(const CXXConstructorDecl *,
FunctionArgList &Args) const = 0;
+ /// Gets the offsets of all the virtual base pointers in a given class.
+ virtual std::vector<CharUnits> getVBPtrOffsets(const CXXRecordDecl *RD);
+
/// Gets the pure virtual member call function.
virtual StringRef GetPureVirtualCallName() = 0;
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=251783&r1=251782&r2=251783&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Mon Nov 2 03:01:44 2015
@@ -15,6 +15,7 @@
#include "CGCUDARuntime.h"
#include "CGCXXABI.h"
#include "CGDebugInfo.h"
+#include "CGRecordLayout.h"
#include "CGObjCRuntime.h"
#include "clang/CodeGen/CGFunctionInfo.h"
#include "clang/Frontend/CodeGenOptions.h"
@@ -356,7 +357,35 @@ static void EmitNullBaseClassInitializat
DestPtr = CGF.Builder.CreateElementBitCast(DestPtr, CGF.Int8Ty);
const ASTRecordLayout &Layout = CGF.getContext().getASTRecordLayout(Base);
- llvm::Value *SizeVal = CGF.CGM.getSize(Layout.getNonVirtualSize());
+ CharUnits NVSize = Layout.getNonVirtualSize();
+
+ // We cannot simply zero-initialize the entire base sub-object if vbptrs are
+ // present, they are initialized by the most derived class before calling the
+ // constructor.
+ SmallVector<std::pair<CharUnits, CharUnits>, 1> Stores;
+ Stores.emplace_back(CharUnits::Zero(), NVSize);
+
+ // Each store is split by the existence of a vbptr.
+ CharUnits VBPtrWidth = CGF.getPointerSize();
+ std::vector<CharUnits> VBPtrOffsets =
+ CGF.CGM.getCXXABI().getVBPtrOffsets(Base);
+ for (CharUnits VBPtrOffset : VBPtrOffsets) {
+ std::pair<CharUnits, CharUnits> LastStore = Stores.pop_back_val();
+ CharUnits LastStoreOffset = LastStore.first;
+ CharUnits LastStoreSize = LastStore.second;
+
+ CharUnits SplitBeforeOffset = LastStoreOffset;
+ CharUnits SplitBeforeSize = VBPtrOffset - SplitBeforeOffset;
+ assert(!SplitBeforeSize.isNegative() && "negative store size!");
+ if (!SplitBeforeSize.isZero())
+ Stores.emplace_back(SplitBeforeOffset, SplitBeforeSize);
+
+ CharUnits SplitAfterOffset = VBPtrOffset + VBPtrWidth;
+ CharUnits SplitAfterSize = LastStoreSize - SplitAfterOffset;
+ assert(!SplitAfterSize.isNegative() && "negative store size!");
+ if (!SplitAfterSize.isZero())
+ Stores.emplace_back(SplitAfterOffset, SplitAfterSize);
+ }
// If the type contains a pointer to data member we can't memset it to zero.
// Instead, create a null constant and copy it to the destination.
@@ -364,14 +393,12 @@ static void EmitNullBaseClassInitializat
// like -1, which happens to be the pattern used by member-pointers.
// TODO: isZeroInitializable can be over-conservative in the case where a
// virtual base contains a member pointer.
- if (!CGF.CGM.getTypes().isZeroInitializable(Base)) {
- llvm::Constant *NullConstant = CGF.CGM.EmitNullConstantForBase(Base);
-
- llvm::GlobalVariable *NullVariable =
- new llvm::GlobalVariable(CGF.CGM.getModule(), NullConstant->getType(),
- /*isConstant=*/true,
- llvm::GlobalVariable::PrivateLinkage,
- NullConstant, Twine());
+ llvm::Constant *NullConstantForBase = CGF.CGM.EmitNullConstantForBase(Base);
+ if (!NullConstantForBase->isNullValue()) {
+ llvm::GlobalVariable *NullVariable = new llvm::GlobalVariable(
+ CGF.CGM.getModule(), NullConstantForBase->getType(),
+ /*isConstant=*/true, llvm::GlobalVariable::PrivateLinkage,
+ NullConstantForBase, Twine());
CharUnits Align = std::max(Layout.getNonVirtualAlignment(),
DestPtr.getAlignment());
@@ -380,14 +407,29 @@ static void EmitNullBaseClassInitializat
Address SrcPtr = Address(CGF.EmitCastToVoidPtr(NullVariable), Align);
// Get and call the appropriate llvm.memcpy overload.
- CGF.Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal);
- return;
- }
-
+ for (std::pair<CharUnits, CharUnits> Store : Stores) {
+ CharUnits StoreOffset = Store.first;
+ CharUnits StoreSize = Store.second;
+ llvm::Value *StoreSizeVal = CGF.CGM.getSize(StoreSize);
+ CGF.Builder.CreateMemCpy(
+ CGF.Builder.CreateConstInBoundsByteGEP(DestPtr, StoreOffset),
+ CGF.Builder.CreateConstInBoundsByteGEP(SrcPtr, StoreOffset),
+ StoreSizeVal);
+ }
+
// Otherwise, just memset the whole thing to zero. This is legal
// because in LLVM, all default initializers (other than the ones we just
// handled above) are guaranteed to have a bit pattern of all zeros.
- CGF.Builder.CreateMemSet(DestPtr, CGF.Builder.getInt8(0), SizeVal);
+ } else {
+ for (std::pair<CharUnits, CharUnits> Store : Stores) {
+ CharUnits StoreOffset = Store.first;
+ CharUnits StoreSize = Store.second;
+ llvm::Value *StoreSizeVal = CGF.CGM.getSize(StoreSize);
+ CGF.Builder.CreateMemSet(
+ CGF.Builder.CreateConstInBoundsByteGEP(DestPtr, StoreOffset),
+ CGF.Builder.getInt8(0), StoreSizeVal);
+ }
+ }
}
void
Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=251783&r1=251782&r2=251783&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Mon Nov 2 03:01:44 2015
@@ -90,6 +90,25 @@ public:
return 1;
}
+ std::vector<CharUnits> getVBPtrOffsets(const CXXRecordDecl *RD) override {
+ std::vector<CharUnits> VBPtrOffsets;
+ const ASTContext &Context = getContext();
+ const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
+
+ const VBTableGlobals &VBGlobals = enumerateVBTables(RD);
+ for (const VPtrInfo *VBT : *VBGlobals.VBTables) {
+ const ASTRecordLayout &SubobjectLayout =
+ Context.getASTRecordLayout(VBT->BaseWithVPtr);
+ CharUnits Offs = VBT->NonVirtualOffset;
+ Offs += SubobjectLayout.getVBPtrOffset();
+ if (VBT->getVBaseWithVPtr())
+ Offs += Layout.getVBaseClassOffset(VBT->getVBaseWithVPtr());
+ VBPtrOffsets.push_back(Offs);
+ }
+ llvm::array_pod_sort(VBPtrOffsets.begin(), VBPtrOffsets.end());
+ return VBPtrOffsets;
+ }
+
StringRef GetPureVirtualCallName() override { return "_purecall"; }
StringRef GetDeletedVirtualCallName() override { return "_purecall"; }
Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp?rev=251783&r1=251782&r2=251783&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-virtual-inheritance.cpp Mon Nov 2 03:01:44 2015
@@ -455,3 +455,29 @@ void destroy(E *obj) {
}
}
+
+namespace test5 {
+// PR25370: Don't zero-initialize vbptrs in virtual bases.
+struct A {
+ virtual void f();
+};
+
+struct B : virtual A {
+ int Field;
+};
+
+struct C : B {
+ C();
+};
+
+C::C() : B() {}
+// CHECK-LABEL: define x86_thiscallcc %"struct.test5::C"* @"\01??0C at test5@@QAE at XZ"(
+// CHECK: %[[THIS:.*]] = load %"struct.test5::C"*, %"struct.test5::C"**
+// CHECK: br i1 %{{.*}}, label %[[INIT_VBASES:.*]], label %[[SKIP_VBASES:.*]]
+
+// CHECK: %[[SKIP_VBASES]]
+// CHECK: %[[B:.*]] = bitcast %"struct.test5::C"* %[[THIS]] to %"struct.test5::B"*
+// CHECK: %[[B_i8:.*]] = bitcast %"struct.test5::B"* %[[B]] to i8*
+// CHECK: %[[FIELD:.*]] = getelementptr inbounds i8, i8* %[[B_i8]], i32 4
+// CHECK: call void @llvm.memset.p0i8.i32(i8* %[[FIELD]], i8 0, i32 4, i32 4, i1 false)
+}
More information about the cfe-commits
mailing list