<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 28, 2015, at 3:50 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Apr 28, 2015 at 3:45 PM, Adrian Prantl <span dir="ltr" class=""><<a href="mailto:aprantl@apple.com" target="_blank" class="">aprantl@apple.com</a>></span> wrote:<br class=""><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">Hi echristo, dblaikie,<br class="">
<br class="">
Debug Info: Represent local anonymous unions as anonymous unions in the debug info.<br class="">
This patch deletes a hack that emits the members of local anonymous unions as local variables.<br class="">
<br class="">
Why? Besides being morally wrong, the existing representation using local variables breaks internal assumptions about the local variables' storage size.<br class="">
<br class="">
Compiling<br class="">
<br class="">
```<br class="">
void fn1() {<br class="">
union {<br class="">
int i;<br class="">
char c;<br class="">
};<br class="">
i = c;<br class="">
}<br class="">
<br class="">
```<br class="">
<br class="">
with -g -O3 -verify will cause the verifier to fail after SROA splits the 32-bit storage for the "local variable" c into two pieces because the second piece is clearly outside the 8-bit range that is expected for a variable of type char. Given the choice I'd rather fix the debug representation than weaken the verifier.<br class="">
<br class="">
Debuggers generally already know how to deal with anonymous unions when they are members of C++ record types, but they may have problems finding the local anonymous struct members in the expression evaluator.<br class="">
<br class="">
Eric, do you know whether the local variable trick is necessary for GDB compatibility?<br class=""></blockquote><div class=""><br class="">Throwing it at the GDB buildbot's probably the best way to find out. Don't too much mind committing it first & seeing if it fails (unless you guys can run it these days, in which case a preliminary run would be nice).<br class=""><br class="">GCC produces something vaguely interesting:<br class=""><br class=""><div class=""><font face="monospace, monospace" class="">0x0000002d: DW_TAG_subprogram [2] *</font></div><div class=""><font face="monospace" class=""> ...</font></div><div class=""><font face="monospace, monospace" class="">0x0000004e: DW_TAG_lexical_block [3] *<br class=""> ...</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000063: DW_TAG_variable [4] </font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_name [DW_FORM_string] ("i")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_type [DW_FORM_ref4] (cu + 0x0098 => {0x00000098})</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_artificial [DW_FORM_flag_present] (true)</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x0000006d: DW_TAG_variable [4] </font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_name [DW_FORM_string] ("c")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_type [DW_FORM_ref4] (cu + 0x009f => {0x0000009f})</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_artificial [DW_FORM_flag_present] (true)</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000077: DW_TAG_variable [5] </font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_type [DW_FORM_ref4] (cu + 0x0080 => {0x00000080})</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_location [DW_FORM_exprloc] (<0x2> 91 60 )</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x0000007f: NULL</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000080: DW_TAG_union_type [6] *</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_byte_size [DW_FORM_data1] (0x04)</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_file [DW_FORM_data1] ("/tmp/dbginfo/union.c")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_line [DW_FORM_data1] (2)</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000084: DW_TAG_member [7] </font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_name [DW_FORM_string] ("i")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_file [DW_FORM_data1] ("/tmp/dbginfo/union.c")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_line [DW_FORM_data1] (3)</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_type [DW_FORM_ref4] (cu + 0x0098 => {0x00000098})</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x0000008d: DW_TAG_member [7] </font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_name [DW_FORM_string] ("c")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_file [DW_FORM_data1] ("/tmp/dbginfo/union.c")</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_decl_line [DW_FORM_data1] (4)</font></div><div class=""><font face="monospace, monospace" class=""> DW_AT_type [DW_FORM_ref4] (cu + 0x009f => {0x0000009f})</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000096: NULL</font></div><div class=""><font face="monospace, monospace" class=""><br class=""></font></div><div class=""><font face="monospace, monospace" class="">0x00000097: NULL</font></div></div></div></div></div></div></blockquote><div><br class=""></div>That’s pretty much the output that this patch produces, too (anonymous local variable + anonymous union). If we can use GCC as a precedent, I’m happy to commit this and see whether it breaks.</div><div><br class=""></div><div>-- adrian<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""><br class=""> </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 class="">
REPOSITORY<br class="">
rL LLVM<br class="">
<br class="">
<a href="http://reviews.llvm.org/D9329" target="_blank" class="">http://reviews.llvm.org/D9329</a><br class="">
<br class="">
Files:<br class="">
lib/CodeGen/CGDebugInfo.cpp<br class="">
test/CodeGenCXX/debug-info-anon-union-vars.cpp<br class="">
<br class="">
Index: lib/CodeGen/CGDebugInfo.cpp<br class="">
===================================================================<br class="">
--- lib/CodeGen/CGDebugInfo.cpp<br class="">
+++ lib/CodeGen/CGDebugInfo.cpp<br class="">
@@ -2835,31 +2835,6 @@<br class="">
return;<br class="">
} else if (isa<VariableArrayType>(VD->getType()))<br class="">
Expr.push_back(llvm::dwarf::DW_OP_deref);<br class="">
- } else if (const RecordType *RT = dyn_cast<RecordType>(VD->getType())) {<br class="">
- // If VD is an anonymous union then Storage represents value for<br class="">
- // all union fields.<br class="">
- const RecordDecl *RD = cast<RecordDecl>(RT->getDecl());<br class="">
- if (RD->isUnion() && RD->isAnonymousStructOrUnion()) {<br class="">
- for (const auto *Field : RD->fields()) {<br class="">
- llvm::MDType *FieldTy = getOrCreateType(Field->getType(), Unit);<br class="">
- StringRef FieldName = Field->getName();<br class="">
-<br class="">
- // Ignore unnamed fields. Do not ignore unnamed records.<br class="">
- if (FieldName.empty() && !isa<RecordType>(Field->getType()))<br class="">
- continue;<br class="">
-<br class="">
- // Use VarDecl's Tag, Scope and Line number.<br class="">
- auto *D = DBuilder.createLocalVariable(<br class="">
- Tag, Scope, FieldName, Unit, Line, FieldTy,<br class="">
- CGM.getLangOpts().Optimize, Flags, ArgNo);<br class="">
-<br class="">
- // Insert an llvm.dbg.declare into the current block.<br class="">
- DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(Expr),<br class="">
- llvm::DebugLoc::get(Line, Column, Scope),<br class="">
- Builder.GetInsertBlock());<br class="">
- }<br class="">
- return;<br class="">
- }<br class="">
}<br class="">
<br class="">
// Create the descriptor for the variable.<br class="">
Index: test/CodeGenCXX/debug-info-anon-union-vars.cpp<br class="">
===================================================================<br class="">
--- test/CodeGenCXX/debug-info-anon-union-vars.cpp<br class="">
+++ test/CodeGenCXX/debug-info-anon-union-vars.cpp<br class="">
@@ -21,8 +21,26 @@<br class="">
return (c == 1);<br class="">
}<br class="">
<br class="">
+// This is not necessary (and actually harmful because it breaks IR assumptions)<br class="">
+// for local variables.<br class="">
+void foo() {<br class="">
+ union {<br class="">
+ int i;<br class="">
+ char c;<br class="">
+ };<br class="">
+ i = 8;<br class="">
+}<br class="">
+<br class="">
// CHECK: [[FILE:.*]] = !MDFile(filename: "{{.*}}debug-info-anon-union-vars.cpp",<br class="">
// CHECK: !MDGlobalVariable(name: "c",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br class="">
// CHECK: !MDGlobalVariable(name: "d",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br class="">
// CHECK: !MDGlobalVariable(name: "a",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br class="">
// CHECK: !MDGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br class="">
+// CHECK: !MDLocalVariable(<br class="">
+// CHECK-NOT: name:<br class="">
+// CHECK: type: ![[UNION:[0-9]+]]<br class="">
+// CHECK: ![[UNION]] = !MDCompositeType(tag: DW_TAG_union_type,<br class="">
+// CHECK-NOT: name:<br class="">
+// CHECK: elements<br class="">
+// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]],<br class="">
+// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]],<br class="">
<br class="">
EMAIL PREFERENCES<br class="">
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank" class="">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br class="">
</blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>