[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list and support DWARF5 line tables

2019-12-05 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57f8a998ceaf: [lldb] Don't put compile unit name into 
the support file list and support… (authored by labath).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70954/new/

https://reviews.llvm.org/D70954

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
@@ -0,0 +1,129 @@
+# Test handling of DWARF5 line tables. In particular, test that we handle files
+# which are present in the line table more than once.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "source info -f file0.c" -o "source info -f file1.c" \
+# RUN:   -o "breakpoint set -f file0.c -l 42" \
+# RUN:   -o "breakpoint set -f file0.c -l 47" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK-LABEL: source info -f file0.c
+# CHECK: [0x-0x0001): /file0.c:42
+# CHECK-LABEL: source info -f file1.c
+# CHECK: [0x0001-0x0002): /file1.c:47
+# CHECK-LABEL: breakpoint set -f file0.c -l 42
+# CHECK: Breakpoint 1: {{.*}}`foo,
+# CHECK-LABEL: breakpoint set -f file0.c -l 47
+# CHECK: Breakpoint 2: {{.*}}`foo + 2,
+
+.text
+.globl  foo
+foo:
+nop
+nop
+nop
+.Lfoo_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   0   # DW_CHILDREN_no
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   19  # DW_AT_language
+.byte   5   # DW_FORM_data2
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   16  # DW_AT_stmt_list
+.byte   23  # DW_FORM_sec_offset
+.byte   27  # DW_AT_comp_dir
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  5   # DWARF version number
+.byte   1   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev   # Offset Into Abbrev. Section
+.byte   1   # Abbrev [1] 0xc:0x23 DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.short  12  # DW_AT_language
+.asciz  "file0.c"   # DW_AT_name
+.long   .Lline_table_begin  # DW_AT_stmt_list
+.asciz  "/" # DW_AT_comp_dir
+.quad   foo # DW_AT_low_pc
+.long   .Lfoo_end-foo   # DW_AT_high_pc
+.Ldebug_info_end0:
+
+.section.debug_line,"",@progbits
+.Lline_table_begin:
+.long .Lline_end-.Lline_start
+.Lline_start:
+.short  5   # DWARF version number
+.byte   8   # Address Size (in bytes)
+.byte   0   # Segment Selector Size
+.long   .Lheader_end-.Lheader_start
+.Lheader_start:
+.byte   1   # Minimum Instruction Length
+.byte   1   # Maximum Operations per Instruction
+.byte   1   # Default is_stmt
+.byte   0   # Line Base
+.byte   0   # Line Range
+.byte   5   # Opcode Base
+.byte   0, 1, 1, 1  # Standard Opcode Lengths
+
+# Directory table format
+.byte   1   # One element per directory entry
+.byte   1   # DW_LNCT_path
+.byte   0x08# DW_FORM_string
+
+# Directory table entries
+.byte   1   # 1 directory
+   

[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list

2019-12-03 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 231904.
labath added a comment.

Add the dwarf5 part + a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70954/new/

https://reviews.llvm.org/D70954

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s

Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-debug_line.s
@@ -0,0 +1,129 @@
+# Test handling of DWARF5 line tables. In particular, test that we handle files
+# which are present in the line table more than once.
+
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "source info -f file0.c" -o "source info -f file1.c" \
+# RUN:   -o "breakpoint set -f file0.c -l 42" \
+# RUN:   -o "breakpoint set -f file0.c -l 47" \
+# RUN:   -o exit | FileCheck %s
+
+# CHECK-LABEL: source info -f file0.c
+# CHECK: [0x-0x0001): /file0.c:42
+# CHECK-LABEL: source info -f file1.c
+# CHECK: [0x0001-0x0002): /file1.c:47
+# CHECK-LABEL: breakpoint set -f file0.c -l 42
+# CHECK: Breakpoint 1: {{.*}}`foo,
+# CHECK-LABEL: breakpoint set -f file0.c -l 47
+# CHECK: Breakpoint 2: {{.*}}`foo + 2,
+
+.text
+.globl  foo
+foo:
+nop
+nop
+nop
+.Lfoo_end:
+
+.section.debug_abbrev,"",@progbits
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   0   # DW_CHILDREN_no
+.byte   37  # DW_AT_producer
+.byte   8   # DW_FORM_string
+.byte   19  # DW_AT_language
+.byte   5   # DW_FORM_data2
+.byte   3   # DW_AT_name
+.byte   8   # DW_FORM_string
+.byte   16  # DW_AT_stmt_list
+.byte   23  # DW_FORM_sec_offset
+.byte   27  # DW_AT_comp_dir
+.byte   8   # DW_FORM_string
+.byte   17  # DW_AT_low_pc
+.byte   1   # DW_FORM_addr
+.byte   18  # DW_AT_high_pc
+.byte   6   # DW_FORM_data4
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+
+.section.debug_info,"",@progbits
+.Lcu_begin0:
+.long   .Ldebug_info_end0-.Ldebug_info_start0 # Length of Unit
+.Ldebug_info_start0:
+.short  5   # DWARF version number
+.byte   1   # DWARF Unit Type
+.byte   8   # Address Size (in bytes)
+.long   .debug_abbrev   # Offset Into Abbrev. Section
+.byte   1   # Abbrev [1] 0xc:0x23 DW_TAG_compile_unit
+.asciz  "Hand-written DWARF"# DW_AT_producer
+.short  12  # DW_AT_language
+.asciz  "file0.c"   # DW_AT_name
+.long   .Lline_table_begin  # DW_AT_stmt_list
+.asciz  "/" # DW_AT_comp_dir
+.quad   foo # DW_AT_low_pc
+.long   .Lfoo_end-foo   # DW_AT_high_pc
+.Ldebug_info_end0:
+
+.section.debug_line,"",@progbits
+.Lline_table_begin:
+.long .Lline_end-.Lline_start
+.Lline_start:
+.short  5   # DWARF version number
+.byte   8   # Address Size (in bytes)
+.byte   0   # Segment Selector Size
+.long   .Lheader_end-.Lheader_start
+.Lheader_start:
+.byte   1   # Minimum Instruction Length
+.byte   1   # Maximum Operations per Instruction
+.byte   1   # Default is_stmt
+.byte   0   # Line Base
+.byte   0   # Line Range
+.byte   5   # Opcode Base
+.byte   0, 1, 1, 1  # Standard Opcode Lengths
+
+# Directory table format
+.byte   1   # One element per directory entry
+.byte   1   # DW_LNCT_path
+.byte   0x08# DW_FORM_string
+
+# Directory table entries
+.byte   1   # 1 directory
+.asciz  "/"
+
+# File table format
+.byte   2   # 2 elements pe

[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list

2019-12-03 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

This seems to break `Shell/SymbolFile/DWARF/debug-types-address-ranges.s` on my 
machine: https://teemperor.de/pub/D70954.log


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70954/new/

https://reviews.llvm.org/D70954



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list

2019-12-03 Thread Pavel Labath via Phabricator via lldb-commits
labath planned changes to this revision.
labath added a comment.

It seems one dwarf5 test is failing with this change (or rather, it was 
accidentally passing before as it picked up the fake line 0 entry). Since this 
patch is not that big, I'll just fold the dwarf5 followup into it. Planning 
changes == I need to come up with a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70954/new/

https://reviews.llvm.org/D70954



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70954: [lldb] Don't put compile unit name into the support file list

2019-12-03 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: clayborg, JDevlieghere.
Herald added a subscriber: aprantl.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.

Lldb's "format-independent" debug info made use of the fact that DWARF
(<=4) did not use the file index zero, and reused the support file index
zero for storing the compile unit name.

While this provided some convenience for DWARF<=4, it meant that the PDB
plugin needed to artificially remap file indices in order to free up
index 0. Furthermore, DWARF v5 make file index 0 legal, which meant that
similar remapping would be needed in the dwarf plugin too.

What this patch does instead is remove the requirement of having the
compile unit name in the index 0. It is not that useful since the name
can always be fetched from the CompileUnit object. Remapping code in the
pdb plugin(s) has been removed or simplified.

DWARF plugin has started inserting an empty FileSpec at index 0 to
ensure the indices keep matching up. In a follow-up patch, I'll start
adding the file zero in case of DWARF v5.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70954

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/CompileUnit.cpp

Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -210,23 +210,12 @@
 uint32_t CompileUnit::FindLineEntry(uint32_t start_idx, uint32_t line,
 const FileSpec *file_spec_ptr, bool exact,
 LineEntry *line_entry_ptr) {
-  uint32_t file_idx = 0;
+  if (!file_spec_ptr)
+file_spec_ptr = &GetPrimaryFile();
+  uint32_t file_idx = GetSupportFiles().FindFileIndex(0, *file_spec_ptr, true);
+  if (file_idx == UINT32_MAX)
+return UINT32_MAX;
 
-  if (file_spec_ptr) {
-file_idx = GetSupportFiles().FindFileIndex(1, *file_spec_ptr, true);
-if (file_idx == UINT32_MAX)
-  return UINT32_MAX;
-  } else {
-// All the line table entries actually point to the version of the Compile
-// Unit that is in the support files (the one at 0 was artificially added.)
-// So prefer the one further on in the support files if it exists...
-const FileSpecList &support_files = GetSupportFiles();
-const bool full = true;
-file_idx = support_files.FindFileIndex(
-1, support_files.GetFileSpecAtIndex(0), full);
-if (file_idx == UINT32_MAX)
-  file_idx = 0;
-  }
   LineTable *line_table = GetLineTable();
   if (line_table)
 return line_table->FindLineEntryIndexByFileIndex(start_idx, file_idx, line,
@@ -253,7 +242,7 @@
 return;
 
   uint32_t file_idx =
-  GetSupportFiles().FindFileIndex(1, file_spec, true);
+  GetSupportFiles().FindFileIndex(0, file_spec, true);
   while (file_idx != UINT32_MAX) {
 file_indexes.push_back(file_idx);
 file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true);
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -371,10 +371,6 @@
 support_files.AppendIfUnique(spec);
   }
 
-  // LLDB uses the DWARF-like file numeration (one based),
-  // the zeroth file is the compile unit itself
-  support_files.Insert(0, comp_unit.GetPrimaryFile());
-
   return true;
 }
 
@@ -1881,9 +1877,7 @@
   if (!source_files)
 return;
 
-  // LLDB uses the DWARF-like file numeration (one based)
-  int index = 1;
-
+  int index = 0;
   while (auto file = source_files->getNext()) {
 uint32_t source_id = file->getUniqueId();
 index_map[source_id] = index++;
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1110,9 +1110,7 @@
   // LLDB wants the index of the file in the list of support files.
   auto fn_iter = llvm::find(cci->m_file_list, *efn);
   lldbassert(fn_iter != cci->m_file_list.end());
-  // LLDB support file indices are 1-based.
-  uint32_t file_index =
-  1 + std::distance(cci->m_file_list.begin(), fn_iter);
+  uint32_t file_index = std::distance(cci->m_file_list.begin(), fn_iter);
 
   std::unique_ptr sequence(
   line_table->CreateLineSequenceContainer());
@@ -1155,14 +1153,6 @@
 FileSpec spec(f, style);
 support_files.Append(spec);
   }
-
-  llvm::SmallString<64> main_source_file =
-  m_index->compilands().GetMainSourceFile(*cci);
-  FileSpec::Style style = main_source_file.startswith("/")