<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sun, Oct 19, 2014 at 12:54 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Sat, Oct 18, 2014 at 5:03 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: majnemer<br>
Date: Sat Oct 18 19:03:10 2014<br>
New Revision: 220153<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=220153&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=220153&view=rev</a><br>
Log:<br>
CodeGen: ConstStructBuilder must verify packed constraints after padding<br>
<br>
Before, ConstStructBuilder::AppendBytes would check packed constraints<br>
prior to padding being added before the field's offset. However, adding<br>
this padding might force our struct to be packed. Because we wouldn't<br>
check *after* adding padding, ConstStructBuilder would be in an<br>
inconsistent state leading to a crash.<br></blockquote><div><br></div></span><div>Much to my surprise, this commit introduces a miscompile that is causing all the LNT bots to go red.</div><div><br></div><div>You can reproduce this super easily though:</div><div><br></div><div>./bin/clang -m64 .../your/path/to/the/test/suite/SingleSource/UnitTests/2006-01-23-UnionInit.c -w -o unioninit<br></div><div>./unioninit</div><div><br></div><div>The last line should read:</div><div>rdar://6828787: 23122, -12312731, -312<br></div><div><br></div><div>But with your patch it reads:</div><div>rdar://6828787: 23122, -188, -312<br></div><div><br></div><div>Lemme know if you have trouble reproducing, I can send you preprocessed source, whatever.</div><div><br></div><div>I've reverted the patch for now to get our bots back in r220169. Sorry for the trouble!</div></div></div></div></blockquote><div><br></div><div>Should be fixed with r220175.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
This fixes PR21300.<br>
<br>
Modified:<br>
cfe/trunk/lib/CodeGen/CGExprConstant.cpp<br>
cfe/trunk/test/CodeGen/const-init.c<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=220153&r1=220152&r2=220153&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=220153&r1=220152&r2=220153&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Sat Oct 18 19:03:10 2014<br>
@@ -106,15 +106,6 @@ AppendBytes(CharUnits FieldOffsetInChars<br>
CharUnits AlignedNextFieldOffsetInChars =<br>
NextFieldOffsetInChars.RoundUpToAlignment(FieldAlignment);<br>
<br>
- if (AlignedNextFieldOffsetInChars > FieldOffsetInChars) {<br>
- assert(!Packed && "Alignment is wrong even with a packed struct!");<br>
-<br>
- // Convert the struct to a packed struct.<br>
- ConvertStructToPacked();<br>
-<br>
- AlignedNextFieldOffsetInChars = NextFieldOffsetInChars;<br>
- }<br>
-<br>
if (AlignedNextFieldOffsetInChars < FieldOffsetInChars) {<br>
// We need to append padding.<br>
AppendPadding(FieldOffsetInChars - NextFieldOffsetInChars);<br>
@@ -122,6 +113,16 @@ AppendBytes(CharUnits FieldOffsetInChars<br>
assert(NextFieldOffsetInChars == FieldOffsetInChars &&<br>
"Did not add enough padding!");<br>
<br>
+ AlignedNextFieldOffsetInChars =<br>
+ NextFieldOffsetInChars.RoundUpToAlignment(FieldAlignment);<br>
+ }<br>
+<br>
+ if (AlignedNextFieldOffsetInChars > FieldOffsetInChars) {<br>
+ assert(!Packed && "Alignment is wrong even with a packed struct!");<br>
+<br>
+ // Convert the struct to a packed struct.<br>
+ ConvertStructToPacked();<br>
+<br>
AlignedNextFieldOffsetInChars = NextFieldOffsetInChars;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/CodeGen/const-init.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/const-init.c?rev=220153&r1=220152&r2=220153&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/const-init.c?rev=220153&r1=220152&r2=220153&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGen/const-init.c (original)<br>
+++ cfe/trunk/test/CodeGen/const-init.c Sat Oct 18 19:03:10 2014<br>
@@ -159,3 +159,14 @@ void g29() {<br>
static int b[1] = { "asdf" }; // expected-warning {{incompatible pointer to integer conversion initializing 'int' with an expression of type 'char [5]'}}<br>
static int c[1] = { L"a" };<br>
}<br>
+<br>
+// PR21300<br>
+void g30() {<br>
+#pragma pack(1)<br>
+ static struct {<br>
+ int : 1;<br>
+ int x;<br>
+ } a = {};<br>
+ // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 }>, align 1<br>
+#pragma pack()<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>