<div dir="ltr">Hi Chaoren,<div><br></div><div>I'm convinced this is a bug in the lldb support for split dwarf based on a liberal, but not unreasonable reading of the spec. While the spec does specify that a skeleton unit needs to have a DW_AT_dwo_name, from looking at the lists of legal attributes for the things of DW_TAG_compile_unit (et. al), a DW_AT_dwo_name is a legal attribute so we should be able to stick it on there. It's not optimal (i.e. it's larger debug info than absolutely necessary), but I think the improved diagnostics here are worth the size increase.</div><div><br></div><div>Thanks!</div><div><br></div><div>-eric<br><br><div class="gmail_quote"><div dir="ltr">On Mon, Mar 28, 2016 at 1:58 PM Chaoren Lin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi David,<div><br></div><div>This change caused clang to produce inferior binaries that causes LLDB to go into an infinite loop while parsing their debug info.<div><br></div><div>I don't know enough about the debug information format to tell if it's clang now generating incorrect debug info due to this change, or if it's lldb incorrectly parsing the debug info (which just happen to have been working fine until now).</div><div>Could you please take a look?<br><div><br></div><div>For LLDB folks, it's looping through these 4 functions:</div><div><br><div>lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:GetCompileUnitDIEPtrOnly()</div>lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h:GetCompileUnitDIEOnly()</div><div>lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:AddCompileUnitDIE()<br></div><div>lldb/source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:ExtractDIEsIfNeeded()<br></div><div><br></div><div><br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 24, 2016 at 11:37 AM, David Blaikie via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dblaikie<br>
Date: Thu Mar 24 13:37:08 2016<br>
New Revision: 264316<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=264316&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=264316&view=rev</a><br>
Log:<br>
[debuginfo] Include dwo_name in the split unit to improve dwp diagnostics<br>
<br>
When multiple DWP files are merged together and duplicate DWO IDs are<br>
found it's currently difficult to give an actionable error message - the<br>
DW_AT_name of the CU could be provided, but might be identical (if the<br>
same source file is built into two different configurations), which<br>
doesn't help the user identify the problem.<br>
<br>
When no intermediate DWP files are generated, the path to the two DWO<br>
files could be provided - but is lost once the DWOs are merged into a<br>
DWP.<br>
<br>
So, include the name of the DWO (dwo_name) in the split file so that<br>
collissions involving a source CU from a DWP can be better diagnosed.<br>
<br>
(improvements to llvm-dwp using this to come shortly)<br>
<br>
Modified:<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
    llvm/trunk/test/DebugInfo/X86/fission-cu.ll<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=264316&r1=264315&r2=264316&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=264316&r1=264315&r2=264316&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Thu Mar 24 13:37:08 2016<br>
@@ -391,8 +391,11 @@ DwarfDebug::constructDwarfCompileUnit(co<br>
   DwarfCompileUnit &NewCU = *OwnedUnit;<br>
   DIE &Die = NewCU.getUnitDie();<br>
   InfoHolder.addUnit(std::move(OwnedUnit));<br>
-  if (useSplitDwarf())<br>
+  if (useSplitDwarf()) {<br>
     NewCU.setSkeleton(constructSkeletonCU(NewCU));<br>
+    NewCU.addString(Die, dwarf::DW_AT_GNU_dwo_name,<br>
+                    DIUnit->getSplitDebugFilename());<br>
+  }<br>
<br>
   // LTO with assembly output shares a single line table amongst multiple CUs.<br>
   // To avoid the compilation directory being ambiguous, let the line table<br>
<br>
Modified: llvm/trunk/test/DebugInfo/X86/fission-cu.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/fission-cu.ll?rev=264316&r1=264315&r2=264316&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/fission-cu.ll?rev=264316&r1=264315&r2=264316&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/X86/fission-cu.ll (original)<br>
+++ llvm/trunk/test/DebugInfo/X86/fission-cu.ll Thu Mar 24 13:37:08 2016<br>
@@ -33,6 +33,7 @@<br>
 ; CHECK: .debug_abbrev.dwo contents:<br>
 ; CHECK: Abbrev table for offset: 0x00000000<br>
 ; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_yes<br>
+; CHECK: DW_AT_GNU_dwo_name  DW_FORM_GNU_str_index<br>
 ; CHECK: DW_AT_producer  DW_FORM_GNU_str_index<br>
 ; CHECK: DW_AT_language  DW_FORM_data2<br>
 ; CHECK: DW_AT_name      DW_FORM_GNU_str_index<br>
@@ -64,38 +65,41 @@<br>
 ; Check that the rest of the compile units have information.<br>
 ; CHECK: .debug_info.dwo contents:<br>
 ; CHECK: DW_TAG_compile_unit<br>
-; CHECK: DW_AT_producer [DW_FORM_GNU_str_index] ( indexed (00000000) string = "clang version 3.3 (trunk 169021) (llvm/trunk 169020)")<br>
+; CHECK: DW_AT_GNU_dwo_name [DW_FORM_GNU_str_index] ( indexed (00000000) string = "baz.dwo")<br>
+; CHECK: DW_AT_producer [DW_FORM_GNU_str_index] ( indexed (00000001) string = "clang version 3.3 (trunk 169021) (llvm/trunk 169020)")<br>
 ; CHECK: DW_AT_language [DW_FORM_data2]        (DW_LANG_C99)<br>
-; CHECK: DW_AT_name [DW_FORM_GNU_str_index]    ( indexed (00000001) string = "baz.c")<br>
+; CHECK: DW_AT_name [DW_FORM_GNU_str_index]    ( indexed (00000002) string = "baz.c")<br>
 ; CHECK-NOT: DW_AT_low_pc<br>
 ; CHECK-NOT: DW_AT_stmt_list<br>
 ; CHECK-NOT: DW_AT_comp_dir<br>
 ; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8]  (0x1f1f859683d49324)<br>
 ; CHECK: DW_TAG_variable<br>
-; CHECK: DW_AT_name [DW_FORM_GNU_str_index]     ( indexed (00000002) string = "a")<br>
+; CHECK: DW_AT_name [DW_FORM_GNU_str_index]     ( indexed (00000003) string = "a")<br>
 ; CHECK: DW_AT_type [DW_FORM_ref4]       (cu + 0x{{[0-9a-f]*}} => {[[TYPE:0x[0-9a-f]*]]})<br>
 ; CHECK: DW_AT_external [DW_FORM_flag_present]   (true)<br>
 ; CHECK: DW_AT_decl_file [DW_FORM_data1] (0x01)<br>
 ; CHECK: DW_AT_decl_line [DW_FORM_data1] (1)<br>
 ; CHECK: DW_AT_location [DW_FORM_exprloc] (<0x2> fb 00 )<br>
 ; CHECK: [[TYPE]]: DW_TAG_base_type<br>
-; CHECK: DW_AT_name [DW_FORM_GNU_str_index]     ( indexed (00000003) string = "int")<br>
+; CHECK: DW_AT_name [DW_FORM_GNU_str_index]     ( indexed (00000004) string = "int")<br>
<br>
 ; CHECK: .debug_str contents:<br>
 ; CHECK: 0x00000000: "baz.dwo"<br>
 ; CHECK: 0x00000008: "/usr/local/google/home/echristo/tmp"<br>
<br>
 ; CHECK: .debug_str.dwo contents:<br>
-; CHECK: 0x00000000: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)"<br>
-; CHECK: 0x00000035: "baz.c"<br>
-; CHECK: 0x0000003b: "a"<br>
-; CHECK: 0x0000003d: "int"<br>
+; CHECK: 0x00000000: "baz.dwo"<br>
+; CHECK: 0x00000008: "clang version 3.3 (trunk 169021) (llvm/trunk 169020)"<br>
+; CHECK: 0x0000003d: "baz.c"<br>
+; CHECK: 0x00000043: "a"<br>
+; CHECK: 0x00000045: "int"<br>
<br>
 ; CHECK: .debug_str_offsets.dwo contents:<br>
 ; CHECK: 0x00000000: 00000000<br>
-; CHECK: 0x00000004: 00000035<br>
-; CHECK: 0x00000008: 0000003b<br>
-; CHECK: 0x0000000c: 0000003d<br>
+; CHECK: 0x00000004: 00000008<br>
+; CHECK: 0x00000008: 0000003d<br>
+; CHECK: 0x0000000c: 00000043<br>
+; CHECK: 0x00000010: 00000045<br>
<br>
 ; Object file checks<br>
 ; For x86-64-linux we should have this set of relocations for the debug info section<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div>