r220153 - CodeGen: ConstStructBuilder must verify packed constraints after padding

[David Majnemer]

On Sun, Oct 19, 2014 at 12:54 PM, Chandler Carruth <chandlerc at google.com>
wrote:

> On Sat, Oct 18, 2014 at 5:03 PM, David Majnemer <david.majnemer at gmail.com>
> 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!
>

Should be fixed with r220175.


>
>
>> 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141019/ee6d0777/attachment.html>