r220153 - CodeGen: ConstStructBuilder must verify packed constraints after padding

[Chandler Carruth]

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!


> 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/f14f09ad/attachment.html>