From: Eli Friedman Date: Fri, 14 Oct 2011 02:27:24 +0000 (+0000) Subject: PR11124: Don't overwrite memory outside of a base class when performing zero-initiali... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2ed7cb649aa709b875c519f4a980a1e2b5712370;p=clang PR11124: Don't overwrite memory outside of a base class when performing zero-initialization before running its constructor. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141933 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 8c05b466dc..78db5903de 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -353,6 +353,49 @@ RValue CodeGenFunction::EmitCUDAKernelCallExpr(const CUDAKernelCallExpr *E, return CGM.getCUDARuntime().EmitCUDAKernelCallExpr(*this, E, ReturnValue); } +static void EmitNullBaseClassInitialization(CodeGenFunction &CGF, + llvm::Value *DestPtr, + const CXXRecordDecl *Base) { + if (Base->isEmpty()) + return; + + DestPtr = CGF.EmitCastToVoidPtr(DestPtr); + + const ASTRecordLayout &Layout = CGF.getContext().getASTRecordLayout(Base); + CharUnits Size = Layout.getNonVirtualSize(); + CharUnits Align = Layout.getNonVirtualAlign(); + + llvm::Value *SizeVal = CGF.CGM.getSize(Size); + + // 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. + // TODO: there are other patterns besides zero that we can usefully memset, + // 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()); + NullVariable->setAlignment(Align.getQuantity()); + llvm::Value *SrcPtr = CGF.EmitCastToVoidPtr(NullVariable); + + // Get and call the appropriate llvm.memcpy overload. + CGF.Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal, Align.getQuantity()); + return; + } + + // 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, + Align.getQuantity()); +} + void CodeGenFunction::EmitCXXConstructExpr(const CXXConstructExpr *E, AggValueSlot Dest) { @@ -363,8 +406,19 @@ CodeGenFunction::EmitCXXConstructExpr(const CXXConstructExpr *E, // constructor, as can be the case with a non-user-provided default // constructor, emit the zero initialization now, unless destination is // already zeroed. - if (E->requiresZeroInitialization() && !Dest.isZeroed()) - EmitNullInitialization(Dest.getAddr(), E->getType()); + if (E->requiresZeroInitialization() && !Dest.isZeroed()) { + switch (E->getConstructionKind()) { + case CXXConstructExpr::CK_Delegating: + assert(0 && "Delegating constructor should not need zeroing"); + case CXXConstructExpr::CK_Complete: + EmitNullInitialization(Dest.getAddr(), E->getType()); + break; + case CXXConstructExpr::CK_VirtualBase: + case CXXConstructExpr::CK_NonVirtualBase: + EmitNullBaseClassInitialization(*this, Dest.getAddr(), CD->getParent()); + break; + } + } // If this is a call to a trivial default constructor, do nothing. if (CD->isTrivial() && CD->isDefaultConstructor()) diff --git a/lib/CodeGen/CGExprConstant.cpp b/lib/CodeGen/CGExprConstant.cpp index 30c978e6a6..8711c1fdd5 100644 --- a/lib/CodeGen/CGExprConstant.cpp +++ b/lib/CodeGen/CGExprConstant.cpp @@ -1343,3 +1343,8 @@ llvm::Constant *CodeGenModule::EmitNullConstant(QualType T) { // A NULL pointer is represented as -1. return getCXXABI().EmitNullMemberPointer(T->castAs()); } + +llvm::Constant * +CodeGenModule::EmitNullConstantForBase(const CXXRecordDecl *Record) { + return ::EmitNullConstant(*this, Record, false); +} diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index 48237b9b27..8e38a89990 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/lib/CodeGen/CodeGenModule.h @@ -670,6 +670,11 @@ public: /// but not always, an LLVM null constant. llvm::Constant *EmitNullConstant(QualType T); + /// EmitNullConstantForBase - Return a null constant appropriate for + /// zero-initializing a base class with the given type. This is usually, + /// but not always, an LLVM null constant. + llvm::Constant *EmitNullConstantForBase(const CXXRecordDecl *Record); + /// Error - Emit a general error that something can't be done. void Error(SourceLocation loc, StringRef error); diff --git a/test/CodeGenCXX/value-init.cpp b/test/CodeGenCXX/value-init.cpp index 04a18b3fa8..fb981d1ff7 100644 --- a/test/CodeGenCXX/value-init.cpp +++ b/test/CodeGenCXX/value-init.cpp @@ -238,6 +238,25 @@ namespace test6 { // CHECK: ret void } +namespace PR11124 { + // Make sure C::C doesn't overwrite parts of A while it is zero-initializing B + struct A { int a; A(); A(int); }; + struct B : virtual A { int b; }; + struct C : B { C(); }; + C::C() : A(3), B() {} + // CHECK: define void @_ZN7PR111241CC1Ev + // CHECK: call void @llvm.memset.p0i8.i64(i8* {{.*}}, i8 0, i64 12, i32 8, i1 false) + // CHECK-NEXT: call void @_ZN7PR111241BC2Ev + // Make sure C::C doesn't overwrite parts of A while it is zero-initializing B + + struct B2 : virtual A { int B::*b; }; + struct C2 : B2 { C2(); }; + C2::C2() : A(3), B2() {} + // CHECK: define void @_ZN7PR111242C2C1Ev + // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.*}}, i8* {{.*}}, i64 16, i32 8, i1 false) + // CHECK-NEXT: call void @_ZN7PR111242B2C2Ev +} + // CHECK: define linkonce_odr void @_ZN8zeroinit2X3IiEC2Ev(%"struct.zeroinit::X3"* %this) unnamed_addr // CHECK: call void @llvm.memset.p0i8.i64 // CHECK-NEXT: call void @_ZN8zeroinit2X2IiEC2Ev