<div dir="ltr"><br><br><div class="gmail_quote">On Wed, Apr 29, 2015 at 9:58 AM Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote">On Wed, Apr 29, 2015 at 8:20 AM Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On Apr 28, 2015, at 6:26 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Tue, Apr 28, 2015 at 4:01 PM, Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br>
> Author: adrian<br>
> Date: Tue Apr 28 18:01:24 2015<br>
> New Revision: 236059<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=236059&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=236059&view=rev</a><br>
> Log:<br>
> Debug Info: Represent local anonymous unions as anonymous unions<br>
> in the debug info. This patch deletes a hack that emits the members<br>
> of local anonymous unions as local variables.<br>
><br>
> Besides being morally wrong, the existing representation using local<br>
> variables breaks internal assumptions about the local variables' storage<br>
> size.<br>
><br>
> Compiling<br>
><br>
> ```<br>
> void fn1() {<br>
> union {<br>
> int i;<br>
> char c;<br>
> };<br>
> i = c;<br>
> }<br>
><br>
> ```<br>
><br>
> with -g -O3 -verify will cause the verifier to fail after SROA splits<br>
> the 32-bit storage for the "local variable" c into two pieces because the<br>
> second piece is clearly outside the 8-bit range that is expected for a<br>
> variable of type char. Given the choice I'd rather fix the debug<br>
> representation than weaken the verifier.<br>
><br>
> Debuggers generally already know how to deal with anonymous unions when<br>
> they are members of C++ record types, but they may have problems finding<br>
> the local anonymous struct members in the expression evaluator.<br>
><br>
> Seems we're not so lucky: <a href="http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/21650/steps/gdb-75-check/logs/gdb.cp__anon-union.exp" target="_blank">http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/21650/steps/gdb-75-check/logs/gdb.cp__anon-union.exp</a><br>
><br>
> Could you revert & I'll look into getting you a reduced test case/demonstration of the issue? (can you run GDB 7.5? Perhaps a simple test case would demonstrate the issue if you're lucky, otherwise I can reduce one from the failing test case)<br>
<br>
I reverted the commit in r236110. I probably won’t need a reduction — my guess from the log is that gdb expects a local variable to be present.<br>
<br>
My suggestion is to emit local artificial shadow variables and then weaken the Verifier to not verify artificial variables. In a next step, we could use the new debugger tuning target feature to make the artificial local variables and the weakened verifier a gdb-specific behavior, file a bug against gdb, and eventually remove it altogether.<br>
<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>FWIW I don't want to use the "tuning" parameters to also affect correctness.</div></div></div></blockquote><div><br></div><div>OK. Dave and I debated this a little in person, here's a proposal:</div><div><br></div><div>by default it will have the gdb specific behavior, but if you're tuning for lldb (or any other debugger I guess?) it won't be there.</div><div><br></div><div>Thoughts?</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br></div><div>-eric</div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-- adrian<br>
<br>
><br>
><br>
> rdar://problem/20730771<br>
><br>
> Modified:<br>
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
> cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp<br>
><br>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=236059&r1=236058&r2=236059&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=236059&r1=236058&r2=236059&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Apr 28 18:01:24 2015<br>
> @@ -2835,31 +2835,6 @@ void CGDebugInfo::EmitDeclare(const VarD<br>
> return;<br>
> } else if (isa<VariableArrayType>(VD->getType()))<br>
> Expr.push_back(llvm::dwarf::DW_OP_deref);<br>
> - } else if (const RecordType *RT = dyn_cast<RecordType>(VD->getType())) {<br>
> - // If VD is an anonymous union then Storage represents value for<br>
> - // all union fields.<br>
> - const RecordDecl *RD = cast<RecordDecl>(RT->getDecl());<br>
> - if (RD->isUnion() && RD->isAnonymousStructOrUnion()) {<br>
> - for (const auto *Field : RD->fields()) {<br>
> - llvm::MDType *FieldTy = getOrCreateType(Field->getType(), Unit);<br>
> - StringRef FieldName = Field->getName();<br>
> -<br>
> - // Ignore unnamed fields. Do not ignore unnamed records.<br>
> - if (FieldName.empty() && !isa<RecordType>(Field->getType()))<br>
> - continue;<br>
> -<br>
> - // Use VarDecl's Tag, Scope and Line number.<br>
> - auto *D = DBuilder.createLocalVariable(<br>
> - Tag, Scope, FieldName, Unit, Line, FieldTy,<br>
> - CGM.getLangOpts().Optimize, Flags, ArgNo);<br>
> -<br>
> - // Insert an llvm.dbg.declare into the current block.<br>
> - DBuilder.insertDeclare(Storage, D, DBuilder.createExpression(Expr),<br>
> - llvm::DebugLoc::get(Line, Column, Scope),<br>
> - Builder.GetInsertBlock());<br>
> - }<br>
> - return;<br>
> - }<br>
> }<br>
><br>
> // Create the descriptor for the variable.<br>
><br>
> Modified: cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp?rev=236059&r1=236058&r2=236059&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp?rev=236059&r1=236058&r2=236059&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp (original)<br>
> +++ cfe/trunk/test/CodeGenCXX/debug-info-anon-union-vars.cpp Tue Apr 28 18:01:24 2015<br>
> @@ -21,8 +21,26 @@ int test_it() {<br>
> return (c == 1);<br>
> }<br>
><br>
> +// This is not necessary (and actually harmful because it breaks IR assumptions)<br>
> +// for local variables.<br>
> +void foo() {<br>
> + union {<br>
> + int i;<br>
> + char c;<br>
> + };<br>
> + i = 8;<br>
> +}<br>
> +<br>
> // CHECK: [[FILE:.*]] = !MDFile(filename: "{{.*}}debug-info-anon-union-vars.cpp",<br>
> // CHECK: !MDGlobalVariable(name: "c",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br>
> // CHECK: !MDGlobalVariable(name: "d",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br>
> // CHECK: !MDGlobalVariable(name: "a",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br>
> // CHECK: !MDGlobalVariable(name: "b",{{.*}} file: [[FILE]], line: 6,{{.*}} isLocal: true, isDefinition: true<br>
> +// CHECK: !MDLocalVariable(<br>
> +// CHECK-NOT: name:<br>
> +// CHECK: type: ![[UNION:[0-9]+]]<br>
> +// CHECK: ![[UNION]] = !MDCompositeType(tag: DW_TAG_union_type,<br>
> +// CHECK-NOT: name:<br>
> +// CHECK: elements<br>
> +// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "i", scope: ![[UNION]],<br>
> +// CHECK: !MDDerivedType(tag: DW_TAG_member, name: "c", scope: ![[UNION]],<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>
><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></blockquote></div></div>