<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 17, 2014 at 5:29 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2014-Jun-03, at 18:31, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
> Author: dblaikie<br>
> Date: Tue Jun 3 20:30:59 2014<br>
> New Revision: 210143<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=210143&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=210143&view=rev</a><br>
> Log:<br>
> DebugInfo: Partial revert r209984 due to more cases where abstract DbgVariables do not have associated DIEs.<br>
><br>
> Along with a test case to demonstrate that due to inlining order there<br>
> are cases where abstract variable DIEs are not constructed since the<br>
> abstract subprogram was built due to a previous inlining that optimized<br>
> away those variables. This produces incorrect debug info (the 'missing'<br>
> abstract variable causes the inlined instance of that variable to be<br>
> emitted with a full description (name, line, file) rather than<br>
> referencing the abstract origin), but this commit at least ensures that<br>
> it doesn't crash...<br>
><br>
> Added:<br>
> llvm/trunk/test/DebugInfo/missing-abstract-variable.ll<br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=210143&r1=210142&r2=210143&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=210143&r1=210142&r2=210143&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Tue Jun 3 20:30:59 2014<br>
> @@ -1781,7 +1781,11 @@ std::unique_ptr<DIE> DwarfUnit::construc<br>
><br>
> // Define variable debug information entry.<br>
> auto VariableDie = make_unique<DIE>(DV.getTag());<br>
> - if (DbgVariable *AbsVar = DV.getAbstractVariable())<br>
> + DbgVariable *AbsVar = DV.getAbstractVariable();<br>
> + // FIXME: any missing abstract variable missing a DIE will result in incorrect<br>
> + // DWARF. More details in test/DebugInfo/missing-abstract-variable.ll for an<br>
> + // example of why this is happening.<br>
> + if (AbsVar && AbsVar->getDIE())<br>
> addDIEEntry(*VariableDie, dwarf::DW_AT_abstract_origin, *AbsVar->getDIE());<br>
> else {<br>
> if (!Name.empty())<br>
><br>
> Added: llvm/trunk/test/DebugInfo/missing-abstract-variable.ll<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/missing-abstract-variable.ll?rev=210143&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/missing-abstract-variable.ll?rev=210143&view=auto</a><br>
> ==============================================================================<br>
> --- llvm/trunk/test/DebugInfo/missing-abstract-variable.ll (added)<br>
> +++ llvm/trunk/test/DebugInfo/missing-abstract-variable.ll Tue Jun 3 20:30:59 2014<br>
> @@ -0,0 +1,181 @@<br>
> +; REQUIRES: object-emission<br>
> +<br>
> +; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump -debug-dump=info - | FileCheck %s<br>
> +<br>
> +; Build from the following source with clang -O2.<br>
> +<br>
> +; The important details are that 'x's abstract definition is first built during<br>
> +; the definition of 'b', where the parameter to 'x' is constant and so 'x's 's'<br>
> +; variable is optimized away. No abstract definition DIE for 's' is constructed.<br>
> +; Then, during 'a' emission, the abstract DbgVariable for 's' is created, but<br>
> +; the abstract DIE isn't (since the abstract definition for 'b' is already<br>
> +; built). This results in 's' inlined in 'a' being emitted with its name, line,<br>
> +; file there, rather than referencing an abstract definition.<br>
> +<br>
> +; extern int t;<br>
> +;<br>
> +; void f(int);<br>
> +;<br>
> +; inline void x(bool b) {<br>
> +; if (b) {<br>
> +; int s = t;<br>
> +; f(s);<br>
> +; }<br>
> +; f(0);<br>
> +; }<br>
> +;<br>
> +; void b() {<br>
> +; x(false);<br>
> +; }<br>
> +;<br>
> +; void a(bool u) {<br>
> +; x(u);<br>
> +; }<br>
> +<br>
> +; CHECK: [[ABS_X:.*]]: DW_TAG_subprogram<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_name {{.*}} "x"<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: [[ABS_B:.*]]: DW_TAG_formal_parameter<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_name {{.*}} "b"<br>
> +; FIXME: Missing 'x's local 's' variable.<br>
> +<br>
> +; CHECK: DW_TAG_subprogram<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_name {{.*}} "b"<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_inlined_subroutine<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_abstract_origin {{.*}} {[[ABS_X]]}<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_formal_parameter<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_abstract_origin {{.*}} {[[ABS_B]]}<br>
> +; Notice 'x's local variable 's' is missing. Not necessarily a bug here,<br>
> +; since it's been optimized entirely away and it should be described in<br>
> +; abstract subprogram.<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: NULL<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: NULL<br>
> +<br>
> +; CHECK: DW_TAG_subprogram<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_name {{.*}} "a"<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_formal_parameter<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_inlined_subroutine<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_abstract_origin {{.*}} {[[ABS_X]]}<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; FIXME: This formal parameter goes missing at least at -O2, maybe before that.<br>
> +; CHECK: DW_TAG_formal_parameter<br>
> +; CHECK-NOT: DW_TAG<br>
> +; CHECK: DW_AT_abstract_origin {{.*}} {[[ABS_B]]}<br>
> +<br>
> +; The two lexical blocks here are caused by the scope of the if that includes<br>
> +; the condition variable, and the scope within the if's composite statement. I'm<br>
> +; not sure we really need both of them.<br>
> +<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_lexical_block<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_lexical_block<br>
> +; CHECK-NOT: {{DW_TAG|NULL}}<br>
> +; CHECK: DW_TAG_variable<br>
> +; CHECK-NOT: DW_TAG<br>
> +<br>
> +; FIXME: This shouldn't have a name here, it should use DW_AT_abstract_origin<br>
> +; to reference an abstract variable definition instead<br>
> +<br>
> +; CHECK: DW_AT_name {{.*}} "s"<br>
> +<br>
> +<br>
> +@t = external global i32<br>
> +<br>
> +; Function Attrs: uwtable<br>
> +define void @_Z1bv() #0 {<br>
> +entry:<br>
> + tail call void @llvm.dbg.value(metadata !24, i64 0, metadata !25), !dbg !27<br>
> + tail call void @_Z1fi(i32 0), !dbg !28<br>
> + ret void, !dbg !29<br>
> +}<br>
> +<br>
> +; Function Attrs: uwtable<br>
> +define void @_Z1ab(i1 zeroext %u) #0 {<br>
> +entry:<br>
> + tail call void @llvm.dbg.value(metadata !{i1 %u}, i64 0, metadata !13), !dbg !30<br>
> + tail call void @llvm.dbg.value(metadata !{i1 %u}, i64 0, metadata !31), !dbg !33<br>
> + br i1 %u, label %if.then.i, label %_Z1xb.exit, !dbg !34<br>
> +<br>
> +if.then.i: ; preds = %entry<br>
> + %0 = load i32* @t, align 4, !dbg !35, !tbaa !36<br>
> + tail call void @llvm.dbg.value(metadata !{i32 %0}, i64 0, metadata !40), !dbg !35<br>
> + tail call void @_Z1fi(i32 %0), !dbg !41<br>
> + br label %_Z1xb.exit, !dbg !42<br>
> +<br>
> +_Z1xb.exit: ; preds = %entry, %if.then.i<br>
> + tail call void @_Z1fi(i32 0), !dbg !43<br>
> + ret void, !dbg !44<br>
> +}<br>
> +<br>
> +declare void @_Z1fi(i32) #1<br>
> +<br>
> +; Function Attrs: nounwind readnone<br>
> +declare void @llvm.dbg.value(metadata, i64, metadata) #2<br>
> +<br>
> +attributes #0 = { uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }<br>
> +attributes #1 = { "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }<br>
> +attributes #2 = { nounwind readnone }<br>
> +<br>
> +!<a href="http://llvm.dbg.cu" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
> +!llvm.module.flags = !{!21, !22}<br>
> +!llvm.ident = !{!23}<br>
> +<br>
> +!0 = metadata !{i32 786449, metadata !1, i32 4, metadata !"clang version 3.5.0 ", i1 true, metadata !"", i32 0, metadata !2, metadata !2, metadata !3, metadata !2, metadata !2, metadata !"", i32 1} ; [ DW_TAG_compile_unit ] [/tmp/dbginfo/missing-abstract-variables.cc] [DW_LANG_C_plus_plus]<br>
> +!1 = metadata !{metadata !"missing-abstract-variables.cc", metadata !"/tmp/dbginfo"}<br>
> +!2 = metadata !{}<br>
> +!3 = metadata !{metadata !4, metadata !8, metadata !14}<br>
> +!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"b", metadata !"b", metadata !"_Z1bv", i32 13, metadata !6, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, void ()* @_Z1bv, null, null, metadata !2, i32 13} ; [ DW_TAG_subprogram ] [line 13] [def] [b]<br>
> +!5 = metadata !{i32 786473, metadata !1} ; [ DW_TAG_file_type ] [/tmp/dbginfo/missing-abstract-variables.cc]<br>
> +!6 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !7, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br>
> +!7 = metadata !{null}<br>
> +!8 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"a", metadata !"a", metadata !"_Z1ab", i32 17, metadata !9, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, void (i1)* @_Z1ab, null, null, metadata !12, i32 17} ; [ DW_TAG_subprogram ] [line 17] [def] [a]<br>
> +!9 = metadata !{i32 786453, i32 0, null, metadata !"", i32 0, i64 0, i64 0, i64 0, i32 0, null, metadata !10, i32 0, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]<br>
> +!10 = metadata !{null, metadata !11}<br>
> +!11 = metadata !{i32 786468, null, null, metadata !"bool", i32 0, i64 8, i64 8, i64 0, i32 0, i32 2} ; [ DW_TAG_base_type ] [bool] [line 0, size 8, align 8, offset 0, enc DW_ATE_boolean]<br>
> +!12 = metadata !{metadata !13}<br>
> +!13 = metadata !{i32 786689, metadata !8, metadata !"u", metadata !5, i32 16777233, metadata !11, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [u] [line 17]<br>
> +!14 = metadata !{i32 786478, metadata !1, metadata !5, metadata !"x", metadata !"x", metadata !"_Z1xb", i32 5, metadata !9, i1 false, i1 true, i32 0, i32 0, null, i32 256, i1 true, null, null, null, metadata !15, i32 5} ; [ DW_TAG_subprogram ] [line 5] [def] [x]<br>
> +!15 = metadata !{metadata !16, metadata !17}<br>
> +!16 = metadata !{i32 786689, metadata !14, metadata !"b", metadata !5, i32 16777221, metadata !11, i32 0, i32 0} ; [ DW_TAG_arg_variable ] [b] [line 5]<br>
> +!17 = metadata !{i32 786688, metadata !18, metadata !"s", metadata !5, i32 7, metadata !20, i32 0, i32 0} ; [ DW_TAG_auto_variable ] [s] [line 7]<br>
> +!18 = metadata !{i32 786443, metadata !1, metadata !19, i32 6, i32 0, i32 0, i32 1} ; [ DW_TAG_lexical_block ] [/tmp/dbginfo/missing-abstract-variables.cc]<br>
> +!19 = metadata !{i32 786443, metadata !1, metadata !14, i32 6, i32 0, i32 0, i32 0} ; [ DW_TAG_lexical_block ] [/tmp/dbginfo/missing-abstract-variables.cc]<br>
<br>
</div></div>These lexical blocks are invalid IR with too many operands, and wouldn't<br>
pass DILexicalBlock::Verify(). Is that intentional, or should I fix it?<br>
<br>
(I'm looking at r217978, which hasn't changed since this commit. I'm not<br>
sure if it was valid at the time.)<br></blockquote><div><br></div><div>Without the debug info verifier enabled by default (at least by default when reading textual IR in llc - preferably when reading textual IR in clang too) many test cases bit rot /really/ quickly. We (or at leanst I) tend to only fix the tests that cause failures when I make a schema change.<br><br>I assume these ones were affected by the changes to move disciminators to DILexicalBlockFile from DILexicalBlock.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
> +!20 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64 32, i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]<br>
> +!21 = metadata !{i32 2, metadata !"Dwarf Version", i32 4}<br>
> +!22 = metadata !{i32 2, metadata !"Debug Info Version", i32 1}<br>
> +!23 = metadata !{metadata !"clang version 3.5.0 "}<br>
> +!24 = metadata !{i1 false}<br>
> +!25 = metadata !{i32 786689, metadata !14, metadata !"b", metadata !5, i32 16777221, metadata !11, i32 0, metadata !26} ; [ DW_TAG_arg_variable ] [b] [line 5]<br>
> +!26 = metadata !{i32 14, i32 0, metadata !4, null}<br>
> +!27 = metadata !{i32 5, i32 0, metadata !14, metadata !26}<br>
> +!28 = metadata !{i32 10, i32 0, metadata !14, metadata !26}<br>
> +!29 = metadata !{i32 15, i32 0, metadata !4, null}<br>
> +!30 = metadata !{i32 17, i32 0, metadata !8, null}<br>
> +!31 = metadata !{i32 786689, metadata !14, metadata !"b", metadata !5, i32 16777221, metadata !11, i32 0, metadata !32} ; [ DW_TAG_arg_variable ] [b] [line 5]<br>
> +!32 = metadata !{i32 18, i32 0, metadata !8, null}<br>
> +!33 = metadata !{i32 5, i32 0, metadata !14, metadata !32}<br>
> +!34 = metadata !{i32 6, i32 0, metadata !19, metadata !32}<br>
> +!35 = metadata !{i32 7, i32 0, metadata !18, metadata !32}<br>
> +!36 = metadata !{metadata !37, metadata !37, i64 0}<br>
> +!37 = metadata !{metadata !"int", metadata !38, i64 0}<br>
> +!38 = metadata !{metadata !"omnipotent char", metadata !39, i64 0}<br>
> +!39 = metadata !{metadata !"Simple C/C++ TBAA"}<br>
> +!40 = metadata !{i32 786688, metadata !18, metadata !"s", metadata !5, i32 7, metadata !20, i32 0, metadata !32} ; [ DW_TAG_auto_variable ] [s] [line 7]<br>
> +!41 = metadata !{i32 8, i32 0, metadata !18, metadata !32} ; [ DW_TAG_imported_declaration ]<br>
> +!42 = metadata !{i32 9, i32 0, metadata !18, metadata !32}<br>
> +!43 = metadata !{i32 10, i32 0, metadata !14, metadata !32}<br>
> +!44 = metadata !{i32 19, i32 0, metadata !8, null}<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div></div></blockquote></div><br></div></div>