On Sat, Oct 18, 2014 at 5:03 PM, David Majnemer <[email protected]> wrote:
> Author: majnemer > Date: Sat Oct 18 19:03:10 2014 > New Revision: 220153 > > URL: http://llvm.org/viewvc/llvm-project?rev=220153&view=rev > Log: > CodeGen: ConstStructBuilder must verify packed constraints after padding > > Before, ConstStructBuilder::AppendBytes would check packed constraints > prior to padding being added before the field's offset. However, adding > this padding might force our struct to be packed. Because we wouldn't > check *after* adding padding, ConstStructBuilder would be in an > inconsistent state leading to a crash. > Much to my surprise, this commit introduces a miscompile that is causing all the LNT bots to go red. You can reproduce this super easily though: ./bin/clang -m64 .../your/path/to/the/test/suite/SingleSource/UnitTests/2006-01-23-UnionInit.c -w -o unioninit ./unioninit The last line should read: rdar://6828787: 23122, -12312731, -312 But with your patch it reads: rdar://6828787: 23122, -188, -312 Lemme know if you have trouble reproducing, I can send you preprocessed source, whatever. I've reverted the patch for now to get our bots back in r220169. Sorry for the trouble! > This fixes PR21300. > > Modified: > cfe/trunk/lib/CodeGen/CGExprConstant.cpp > cfe/trunk/test/CodeGen/const-init.c > > Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=220153&r1=220152&r2=220153&view=diff > > ============================================================================== > --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Sat Oct 18 19:03:10 2014 > @@ -106,15 +106,6 @@ AppendBytes(CharUnits FieldOffsetInChars > 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; > - } > - > if (AlignedNextFieldOffsetInChars < FieldOffsetInChars) { > // We need to append padding. > AppendPadding(FieldOffsetInChars - NextFieldOffsetInChars); > @@ -122,6 +113,16 @@ AppendBytes(CharUnits FieldOffsetInChars > 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(); > + > AlignedNextFieldOffsetInChars = NextFieldOffsetInChars; > } > > > Modified: cfe/trunk/test/CodeGen/const-init.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/const-init.c?rev=220153&r1=220152&r2=220153&view=diff > > ============================================================================== > --- cfe/trunk/test/CodeGen/const-init.c (original) > +++ cfe/trunk/test/CodeGen/const-init.c Sat Oct 18 19:03:10 2014 > @@ -159,3 +159,14 @@ 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() > +} > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
