commit 6f615d68e372599eaf18787de4ac7312e1b248ca Author: Michael Spencer Date: Tue Oct 19 01:07:06 2010 -0400 CodeGen: Add padding to struct function arguments that are expanded to their scalar values. diff --git a/lib/CodeGen/ABIInfo.h b/lib/CodeGen/ABIInfo.h index ce10398..7bab0ff 100644 --- a/lib/CodeGen/ABIInfo.h +++ b/lib/CodeGen/ABIInfo.h @@ -93,8 +93,8 @@ namespace clang { , bool Realign = false) { return ABIArgInfo(Indirect, 0, Alignment, ByVal, Realign); } - static ABIArgInfo getExpand() { - return ABIArgInfo(Expand); + static ABIArgInfo getExpand(bool IncludePadding) { + return ABIArgInfo(Expand, 0, 0, IncludePadding); } Kind getKind() const { return TheKind; } @@ -139,6 +139,11 @@ namespace clang { return BoolData1; } + bool getExpandIncludePadding() const { + assert(TheKind == Expand && "Invalid kind!"); + return BoolData0; + } + void dump() const; }; diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index aeff447..4a627bc 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -21,6 +21,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" +#include "clang/AST/RecordLayout.h" #include "clang/Frontend/CodeGenOptions.h" #include "llvm/Attributes.h" #include "llvm/Support/CallSite.h" @@ -300,22 +301,39 @@ CGFunctionInfo::CGFunctionInfo(unsigned _CallingConvention, void CodeGenTypes::GetExpandedTypes(QualType Ty, std::vector &ArgTys, + bool IncludePadding, bool IsRecursive) { const RecordType *RT = Ty->getAsStructureType(); assert(RT && "Can only expand structure types."); const RecordDecl *RD = RT->getDecl(); assert(!RD->hasFlexibleArrayMember() && "Cannot expand structure with flexible array."); - - for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); - i != e; ++i) { - const FieldDecl *FD = *i; + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + uint64_t Offset = 0; + unsigned int i = 0; + for (RecordDecl::field_iterator fi = RD->field_begin(), e = RD->field_end(); + fi != e; ++fi, ++i) { + const FieldDecl *FD = *fi; + QualType FT = FD->getType(); assert(!FD->isBitField() && "Cannot expand structure with bit-field members."); + assert(i < Layout.getFieldCount() + && "ASTRecordLayout does not match RecordDecl!"); + + // Check for padding. + if (IncludePadding) { + uint64_t FieldOffset = Layout.getFieldOffset(i); + if (FieldOffset > Offset) { + uint64_t PaddingSize = FieldOffset - Offset; + const llvm::IntegerType *ITy = + llvm::Type::getIntNTy(getLLVMContext(), PaddingSize); + ArgTys.push_back(ITy); + } + Offset = FieldOffset + getContext().getTypeSize(FT); + } - QualType FT = FD->getType(); if (CodeGenFunction::hasAggregateLLVMType(FT)) - GetExpandedTypes(FT, ArgTys, IsRecursive); + GetExpandedTypes(FT, ArgTys, IncludePadding, IsRecursive); else ArgTys.push_back(ConvertType(FT, IsRecursive)); } @@ -323,7 +341,8 @@ void CodeGenTypes::GetExpandedTypes(QualType Ty, llvm::Function::arg_iterator CodeGenFunction::ExpandTypeFromArgs(QualType Ty, LValue LV, - llvm::Function::arg_iterator AI) { + llvm::Function::arg_iterator AI, + bool IncludePadding) { const RecordType *RT = Ty->getAsStructureType(); assert(RT && "Can only expand structure types."); @@ -331,15 +350,28 @@ CodeGenFunction::ExpandTypeFromArgs(QualType Ty, LValue LV, assert(LV.isSimple() && "Unexpected non-simple lvalue during struct expansion."); llvm::Value *Addr = LV.getAddress(); - for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); - i != e; ++i) { - FieldDecl *FD = *i; + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + uint64_t Offset = 0; + unsigned int i = 0; + for (RecordDecl::field_iterator fi = RD->field_begin(), e = RD->field_end(); + fi != e; ++fi, ++i) { + FieldDecl *FD = *fi; QualType FT = FD->getType(); + // Check for padding. + if (IncludePadding) { + uint64_t FieldOffset = Layout.getFieldOffset(i); + if (FieldOffset > Offset) { + // Skip arg. Only one arg of padding can be between each real argument. + ++AI; + } + Offset = FieldOffset + getContext().getTypeSize(FT); + } + // FIXME: What are the right qualifiers here? LValue LV = EmitLValueForField(Addr, FD, 0); if (CodeGenFunction::hasAggregateLLVMType(FT)) { - AI = ExpandTypeFromArgs(FT, LV, AI); + AI = ExpandTypeFromArgs(FT, LV, AI, IncludePadding); } else { EmitStoreThroughLValue(RValue::get(AI), LV, FT); ++AI; @@ -351,22 +383,43 @@ CodeGenFunction::ExpandTypeFromArgs(QualType Ty, LValue LV, void CodeGenFunction::ExpandTypeToArgs(QualType Ty, RValue RV, - llvm::SmallVector &Args) { + llvm::SmallVector &Args, + bool IncludePadding) { const RecordType *RT = Ty->getAsStructureType(); assert(RT && "Can only expand structure types."); RecordDecl *RD = RT->getDecl(); assert(RV.isAggregate() && "Unexpected rvalue during struct expansion"); + const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD); + uint64_t Offset = 0; llvm::Value *Addr = RV.getAggregateAddr(); - for (RecordDecl::field_iterator i = RD->field_begin(), e = RD->field_end(); - i != e; ++i) { - FieldDecl *FD = *i; + unsigned int i = 0; + for (RecordDecl::field_iterator fi = RD->field_begin(), e = RD->field_end(); + fi != e; ++fi, ++i) { + assert(i < Layout.getFieldCount() + && "ASTRecordLayout does not match RecordDecl!"); + FieldDecl *FD = *fi; QualType FT = FD->getType(); + // Check for padding. + if (IncludePadding) { + uint64_t FieldOffset = Layout.getFieldOffset(i); + if (FieldOffset > Offset) { + uint64_t PaddingSize = FieldOffset - Offset; + const llvm::IntegerType *ITy = + llvm::Type::getIntNTy(getLLVMContext(), PaddingSize); + // This is a padding value that should never be read. + llvm::Value *PaddingValue = llvm::UndefValue::get(ITy); + Args.push_back(PaddingValue); + } + Offset = FieldOffset + getContext().getTypeSize(FT); + } + // FIXME: What are the right qualifiers here? LValue LV = EmitLValueForField(Addr, FD, 0); if (CodeGenFunction::hasAggregateLLVMType(FT)) { - ExpandTypeToArgs(FT, RValue::getAggregate(LV.getAddress()), Args); + ExpandTypeToArgs(FT, RValue::getAggregate(LV.getAddress()), Args, + IncludePadding); } else { RValue RV = EmitLoadOfLValue(LV, FT); assert(RV.isScalar() && @@ -662,7 +715,8 @@ CodeGenTypes::GetFunctionType(const CGFunctionInfo &FI, bool IsVariadic, } case ABIArgInfo::Expand: - GetExpandedTypes(it->type, ArgTys, IsRecursive); + GetExpandedTypes(it->type, ArgTys, AI.getExpandIncludePadding(), + IsRecursive); break; } } @@ -812,7 +866,8 @@ void CodeGenModule::ConstructAttributeList(const CGFunctionInfo &FI, // FIXME: This is rather inefficient. Do we ever actually need to do // anything here? The result should be just reconstructed on the other // side, so extension should be a non-issue. - getTypes().GetExpandedTypes(ParamType, Tys, false); + getTypes().GetExpandedTypes(ParamType, Tys, AI.getExpandIncludePadding(), + false); Index += Tys.size(); continue; } @@ -985,7 +1040,8 @@ void CodeGenFunction::EmitFunctionProlog(const CGFunctionInfo &FI, // arguments. llvm::Value *Temp = CreateMemTemp(Ty, Arg->getName() + ".addr"); llvm::Function::arg_iterator End = - ExpandTypeFromArgs(Ty, MakeAddrLValue(Temp, Ty), AI); + ExpandTypeFromArgs(Ty, MakeAddrLValue(Temp, Ty), AI, + ArgI.getExpandIncludePadding()); EmitParmDecl(*Arg, Temp); // Name the arguments used in expansion and increment AI. @@ -1258,7 +1314,7 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, } case ABIArgInfo::Expand: - ExpandTypeToArgs(I->second, RV, Args); + ExpandTypeToArgs(I->second, RV, Args, ArgInfo.getExpandIncludePadding()); break; } } diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index e02cedd..f6ff96a 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -1685,13 +1685,14 @@ private: /// argument. llvm::Function::arg_iterator ExpandTypeFromArgs(QualType Ty, LValue Dst, - llvm::Function::arg_iterator AI); + llvm::Function::arg_iterator AI, bool IncludePadding); /// ExpandTypeToArgs - Expand an RValue \arg Src, with the LLVM type for \arg /// Ty, into individual arguments on the provided vector \arg Args. See /// ABIArgInfo::Expand. void ExpandTypeToArgs(QualType Ty, RValue Src, - llvm::SmallVector &Args); + llvm::SmallVector &Args, + bool IncludePadding); llvm::Value* EmitAsmInput(const AsmStmt &S, const TargetInfo::ConstraintInfo &Info, diff --git a/lib/CodeGen/CodeGenTypes.h b/lib/CodeGen/CodeGenTypes.h index 36acd72..5499326 100644 --- a/lib/CodeGen/CodeGenTypes.h +++ b/lib/CodeGen/CodeGenTypes.h @@ -207,6 +207,7 @@ public: // These are internal details of CGT that shouldn't be used externally. /// argument types it would be passed as on the provided vector \arg /// ArgTys. See ABIArgInfo::Expand. void GetExpandedTypes(QualType Ty, std::vector &ArgTys, + bool IncludeTypes, bool IsRecursive); /// IsZeroInitializable - Return whether a type can be diff --git a/lib/CodeGen/TargetInfo.cpp b/lib/CodeGen/TargetInfo.cpp index 79b5efe..373f467 100644 --- a/lib/CodeGen/TargetInfo.cpp +++ b/lib/CodeGen/TargetInfo.cpp @@ -351,6 +351,7 @@ class X86_32ABIInfo : public ABIInfo { bool IsDarwinVectorABI; bool IsSmallStructInRegABI; + bool IsExpandStructIncludePaddingABI; static bool isRegisterSize(unsigned Size) { return (Size == 8 || Size == 16 || Size == 32 || Size == 64); @@ -380,14 +381,23 @@ public: virtual llvm::Value *EmitVAArg(llvm::Value *VAListAddr, QualType Ty, CodeGenFunction &CGF) const; - X86_32ABIInfo(CodeGen::CodeGenTypes &CGT, bool d, bool p) - : ABIInfo(CGT), IsDarwinVectorABI(d), IsSmallStructInRegABI(p) {} + X86_32ABIInfo(CodeGen::CodeGenTypes &CGT, + bool isDarwinVectorABI, + bool isSmallStructInRegABI, + bool isExpandStructIncludePaddingABI) + : ABIInfo(CGT) + , IsDarwinVectorABI(isDarwinVectorABI) + , IsSmallStructInRegABI(isSmallStructInRegABI) + , IsExpandStructIncludePaddingABI(isExpandStructIncludePaddingABI) {} }; class X86_32TargetCodeGenInfo : public TargetCodeGenInfo { public: - X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool d, bool p) - :TargetCodeGenInfo(new X86_32ABIInfo(CGT, d, p)) {} + X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool dar_vec, + bool struct_in_reg, + bool struct_padding) + :TargetCodeGenInfo(new X86_32ABIInfo(CGT, dar_vec, struct_in_reg, + struct_padding)) {} void SetTargetAttributes(const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule &CGM) const; @@ -653,7 +663,7 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty) const { // optimizations. if (getContext().getTypeSize(Ty) <= 4*32 && canExpandIndirectArgument(Ty, getContext())) - return ABIArgInfo::getExpand(); + return ABIArgInfo::getExpand(IsExpandStructIncludePaddingABI); return getIndirectResult(Ty); } @@ -2663,7 +2673,10 @@ const TargetCodeGenInfo &CodeGenModule::getTargetCodeGenInfo() { switch (Triple.getOS()) { case llvm::Triple::Darwin: return *(TheTargetCodeGenInfo = - new X86_32TargetCodeGenInfo(Types, true, true)); + new X86_32TargetCodeGenInfo(Types, true, true, false)); + case llvm::Triple::Win32: + return *(TheTargetCodeGenInfo = + new X86_32TargetCodeGenInfo(Types, false, false, true)); case llvm::Triple::Cygwin: case llvm::Triple::MinGW32: case llvm::Triple::AuroraUX: @@ -2671,11 +2684,11 @@ const TargetCodeGenInfo &CodeGenModule::getTargetCodeGenInfo() { case llvm::Triple::FreeBSD: case llvm::Triple::OpenBSD: return *(TheTargetCodeGenInfo = - new X86_32TargetCodeGenInfo(Types, false, true)); + new X86_32TargetCodeGenInfo(Types, false, true, false)); default: return *(TheTargetCodeGenInfo = - new X86_32TargetCodeGenInfo(Types, false, false)); + new X86_32TargetCodeGenInfo(Types, false, false, false)); } case llvm::Triple::x86_64: diff --git a/test/CodeGen/struct-arg-padding.c b/test/CodeGen/struct-arg-padding.c new file mode 100644 index 0000000..033f90f --- /dev/null +++ b/test/CodeGen/struct-arg-padding.c @@ -0,0 +1,23 @@ +// This verifies that structs passed to functions contain the correct padding +// with ABIs that require it. +// SEE: PR8398 + +// RUN: %clang_cc1 -triple i386-pc-win32 -emit-llvm %s -o - | FileCheck %s + +typedef struct { int a; /* 32 bits of padding */ double b; } IntDoubleStruct; + +void testValue(IntDoubleStruct a); +void testVA(int a, ...); + +int test() { + IntDoubleStruct id = { 32, 64 }; + + testValue(id); + testVA(1, id); + + return sizeof(id); +} + +// CHECK: i32 {{.*}}, i32 undef, double {{.*}} +// CHECK: i32 {{.*}}, i32 undef, double {{.*}} +// CHECK: ret i32 16