From: David Majnemer Date: Sun, 19 Oct 2014 23:40:06 +0000 (+0000) Subject: CodeGen: ConstStructBuilder must verify packed constraints after padding X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=05db1e2826e6125353689fc8729d73b608e62eea;p=clang CodeGen: ConstStructBuilder must verify packed constraints after padding This reverts commit r220169 which reverted r220153. However, it also contains additional changes: - We may need to add padding *after* we've packed the struct. This occurs when the aligned next field offset is greater than the new field's offset. When this occurs, we make the struct packed. *However*, once packed the next field offset might be less than the new feild's offset. It is in this case that we might further pad the struct. - We would pad structs which were perfectly sized! This behavior is immensely old. This behavior came from blindly subtracting NextFieldOffsetInChars from RecordSize. This doesn't take into account the fact that the struct might have a greater overall alignment than the last field. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@220175 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprConstant.cpp b/lib/CodeGen/CGExprConstant.cpp index 421e7deccc..088a5d08b9 100644 --- a/lib/CodeGen/CGExprConstant.cpp +++ b/lib/CodeGen/CGExprConstant.cpp @@ -104,16 +104,7 @@ AppendBytes(CharUnits FieldOffsetInChars, llvm::Constant *InitCst) { // Round up the field offset to the alignment of the field type. CharUnits AlignedNextFieldOffsetInChars = - NextFieldOffsetInChars.RoundUpToAlignment(FieldAlignment); - - if (AlignedNextFieldOffsetInChars > FieldOffsetInChars) { - assert(!Packed && "Alignment is wrong even with a packed struct!"); - - // Convert the struct to a packed struct. - ConvertStructToPacked(); - - AlignedNextFieldOffsetInChars = NextFieldOffsetInChars; - } + NextFieldOffsetInChars.RoundUpToAlignment(FieldAlignment); if (AlignedNextFieldOffsetInChars < FieldOffsetInChars) { // We need to append padding. @@ -122,6 +113,24 @@ AppendBytes(CharUnits FieldOffsetInChars, llvm::Constant *InitCst) { assert(NextFieldOffsetInChars == FieldOffsetInChars && "Did not add enough padding!"); + AlignedNextFieldOffsetInChars = + NextFieldOffsetInChars.RoundUpToAlignment(FieldAlignment); + } + + if (AlignedNextFieldOffsetInChars > FieldOffsetInChars) { + assert(!Packed && "Alignment is wrong even with a packed struct!"); + + // Convert the struct to a packed struct. + ConvertStructToPacked(); + + // After we pack the struct, we may need to insert padding. + if (NextFieldOffsetInChars < FieldOffsetInChars) { + // We need to append padding. + AppendPadding(FieldOffsetInChars - NextFieldOffsetInChars); + + assert(NextFieldOffsetInChars == FieldOffsetInChars && + "Did not add enough padding!"); + } AlignedNextFieldOffsetInChars = NextFieldOffsetInChars; } @@ -486,10 +495,14 @@ llvm::Constant *ConstStructBuilder::Finalize(QualType Ty) { // No tail padding is necessary. } else { // Append tail padding if necessary. - AppendTailPadding(LayoutSizeInChars); - CharUnits LLVMSizeInChars = - NextFieldOffsetInChars.RoundUpToAlignment(LLVMStructAlignment); + NextFieldOffsetInChars.RoundUpToAlignment(LLVMStructAlignment); + + if (LLVMSizeInChars != LayoutSizeInChars) + AppendTailPadding(LayoutSizeInChars); + + LLVMSizeInChars = + NextFieldOffsetInChars.RoundUpToAlignment(LLVMStructAlignment); // Check if we need to convert the struct to a packed struct. if (NextFieldOffsetInChars <= LayoutSizeInChars && @@ -501,7 +514,10 @@ llvm::Constant *ConstStructBuilder::Finalize(QualType Ty) { "Converting to packed did not help!"); } - assert(LayoutSizeInChars == NextFieldOffsetInChars && + LLVMSizeInChars = + NextFieldOffsetInChars.RoundUpToAlignment(LLVMStructAlignment); + + assert(LayoutSizeInChars == LLVMSizeInChars && "Tail padding mismatch!"); } diff --git a/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c b/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c index 33bd800456..7401e114dd 100644 --- a/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c +++ b/test/CodeGen/2008-07-22-bitfield-init-after-zero-len-array.c @@ -8,5 +8,4 @@ struct et7 { 52, }; -// CHECK: @yv7 = global -// CHECK: i8 52, +// CHECK: @yv7 = global %struct.et7 { [0 x float] zeroinitializer, i8 52 } diff --git a/test/CodeGen/const-init.c b/test/CodeGen/const-init.c index 7d7ccae370..b86274fc82 100644 --- a/test/CodeGen/const-init.c +++ b/test/CodeGen/const-init.c @@ -159,3 +159,25 @@ void g29() { static int b[1] = { "asdf" }; // expected-warning {{incompatible pointer to integer conversion initializing 'int' with an expression of type 'char [5]'}} static int c[1] = { L"a" }; } + +// PR21300 +void g30() { +#pragma pack(1) + static struct { + int : 1; + int x; + } a = {}; + // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 }>, align 1 +#pragma pack() +} + +void g31() { +#pragma pack(4) + static struct { + short a; + long x; + short z; + } a = {23122, -12312731, -312}; +#pragma pack() + // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4 +} diff --git a/test/CodeGenCXX/atomicinit.cpp b/test/CodeGenCXX/atomicinit.cpp index f453194468..91b990b70d 100644 --- a/test/CodeGenCXX/atomicinit.cpp +++ b/test/CodeGenCXX/atomicinit.cpp @@ -1,9 +1,9 @@ // RUN: %clang_cc1 %s -emit-llvm -O1 -o - -triple=i686-apple-darwin9 -std=c++11 | FileCheck %s -// CHECK-DAG: @_ZN7PR180978constant1aE = global {{.*}} { i16 1, i8 6, i8 undef }, align 4 -// CHECK-DAG: @_ZN7PR180978constant1bE = global {{.*}} { i16 2, i8 6, i8 undef }, align 4 -// CHECK-DAG: @_ZN7PR180978constant1cE = global {{.*}} { i16 3, i8 6, i8 undef }, align 4 -// CHECK-DAG: @_ZN7PR180978constant1yE = global {{.*}} { {{.*}} { i16 4, i8 6, i8 undef }, i32 5 }, align 4 +// CHECK-DAG: @_ZN7PR180978constant1aE = global { i16, i8 } { i16 1, i8 6 }, align 4 +// CHECK-DAG: @_ZN7PR180978constant1bE = global { i16, i8 } { i16 2, i8 6 }, align 4 +// CHECK-DAG: @_ZN7PR180978constant1cE = global { i16, i8 } { i16 3, i8 6 }, align 4 +// CHECK-DAG: @_ZN7PR180978constant1yE = global { { i16, i8 }, i32 } { { i16, i8 } { i16 4, i8 6 }, i32 5 }, align 4 struct A { _Atomic(int) i; diff --git a/test/CodeGenCXX/debug-info-template.cpp b/test/CodeGenCXX/debug-info-template.cpp index 126a09f83a..b9fa47a250 100644 --- a/test/CodeGenCXX/debug-info-template.cpp +++ b/test/CodeGenCXX/debug-info-template.cpp @@ -83,7 +83,7 @@ // CHECK: metadata [[PTOARGS:![0-9]*]], metadata !"{{.*}}"} ; [ DW_TAG_structure_type ] [PaddingAtEndTemplate<&PaddedObj>] // CHECK: [[PTOARGS]] = metadata !{metadata [[PTOARG1:![0-9]*]]} -// CHECK: [[PTOARG1]] = metadata !{metadata !"0x30\00\000\000", null, metadata [[CONST_PADDINGATEND_PTR:![0-9]*]], { i32, i8, [3 x i8] }* @PaddedObj, null} ; [ DW_TAG_template_value_parameter ] +// CHECK: [[PTOARG1]] = metadata !{metadata !"0x30\00\000\000", null, metadata [[CONST_PADDINGATEND_PTR:![0-9]*]], %struct.PaddingAtEnd* @PaddedObj, null} ; [ DW_TAG_template_value_parameter ] // CHECK: [[CONST_PADDINGATEND_PTR]] = {{.*}} ; [ DW_TAG_pointer_type ] [line 0, size 64, align 64, offset 0] [from _ZTS12PaddingAtEnd] // CHECK: metadata !"[[TCNESTED]]", %"struct.TC::nested"* @tci, null} ; [ DW_TAG_variable ] [tci] diff --git a/test/Modules/templates.mm b/test/Modules/templates.mm index 67a1e070e2..d60b873d0f 100644 --- a/test/Modules/templates.mm +++ b/test/Modules/templates.mm @@ -12,10 +12,10 @@ void testInlineRedeclEarly() { @import templates_right; -// CHECK-DAG: @list_left = global { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 8, -// CHECK-DAG: @list_right = global { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 12, -// CHECK-DAG: @_ZZ15testMixedStructvE1l = {{.*}} constant { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 1, -// CHECK-DAG: @_ZZ15testMixedStructvE1r = {{.*}} constant { %{{.*}}*, i32, [4 x i8] } { %{{.*}}* null, i32 2, +// CHECK-DAG: @list_left = global %class.List { %"struct.List::node"* null, i32 8 }, align 8 +// CHECK-DAG: @list_right = global %class.List { %"struct.List::node"* null, i32 12 }, align 8 +// CHECK-DAG: @_ZZ15testMixedStructvE1l = {{.*}} constant %class.List { %{{.*}}* null, i32 1 }, align 8 +// CHECK-DAG: @_ZZ15testMixedStructvE1r = {{.*}} constant %class.List { %{{.*}}* null, i32 2 }, align 8 // CHECK-DAG: @_ZN29WithUndefinedStaticDataMemberIA_iE9undefinedE = external global void testTemplateClasses() {