Remove DWARFExpression::LocationListSize

Summary:
The only reason for this function's existance is so that we could pass
the correct size into the DWARFExpression constructor. However, there is
no harm in passing the entire data extractor into the DWARFExpression,
since the same code is performing the size determination as well as the
subsequent parse. So, if we get malformed input or there's a bug in the
parser, we'd compute the wrong size anyway.

Additionally, reducing the number of entry points into the location list
parsing machinery makes it easier to switch the llvm debug_loc(lists)
parsers.

While inside, I added a couple of tests for invalid location list
handling.

Reviewers: JDevlieghere, clayborg

Subscribers: aprantl, javed.absar, kristof.beyls, lldb-commits

Differential Revision: https://reviews.llvm.org/D66789

llvm-svn: 370373

diff --git a/lldb/include/lldb/Expression/DWARFExpression.h b/lldb/include/lldb/Expression/DWARFExpression.h
index 4157cf3b083a4da6597d9e2749a8d422ecbb159b..44015b4e418f54b5fdb4954eed2019db17ea4039 100644 (file)
--- a/lldb/include/lldb/Expression/DWARFExpression.h
+++ b/lldb/include/lldb/Expression/DWARFExpression.h
@@ -238,10 +238,6 @@ public:
                               lldb::addr_t loclist_base_load_addr,
                               lldb::addr_t address, ABI *abi);
 
-  static size_t LocationListSize(const DWARFUnit *dwarf_cu,
-                                 const DataExtractor &debug_loc_data,
-                                 lldb::offset_t offset);
-
   static bool PrintDWARFExpression(Stream &s, const DataExtractor &data,
                                    int address_size, int dwarf_ref_size,
                                    bool location_expression);

diff --git a/lldb/lit/SymbolFile/DWARF/debug_loc.s b/lldb/lit/SymbolFile/DWARF/debug_loc.s
index ce7177d7d56b31ea9f93fa4920c763e8579ad182..1fc862f8a1fc5bfd65df4f1963d93174c8192697 100644 (file)
--- a/lldb/lit/SymbolFile/DWARF/debug_loc.s
+++ b/lldb/lit/SymbolFile/DWARF/debug_loc.s
@@ -1,14 +1,22 @@
+# Test debug_loc parsing, including the cases of invalid input. The exact
+# behavior in the invalid cases is not particularly important, but it should be
+# "reasonable".
+
 # REQUIRES: x86
 
 # RUN: llvm-mc -triple=x86_64-pc-linux -filetype=obj %s > %t
-# RUN: lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
+# RUN: %lldb %t -o "image lookup -v -a 0" -o "image lookup -v -a 2" -o exit \
 # RUN:   | FileCheck %s
 
 # CHECK-LABEL: image lookup -v -a 0
-# CHECK: Variable: {{.*}}, name = "x", type = "int", location = rdi,
+# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = rdi,
+# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
+# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
 # CHECK-LABEL: image lookup -v -a 2
-# CHECK: Variable: {{.*}}, name = "x", type = "int", location = rax,
+# CHECK: Variable: {{.*}}, name = "x0", type = "int", location = rax,
+# CHECK: Variable: {{.*}}, name = "x1", type = "int", location = ,
+# CHECK: Variable: {{.*}}, name = "x2", type = "int", location = ,
 
         .type   f,@function
 f:                                      # @f
@@ -27,8 +35,12 @@ f:                                      # @f
         .asciz  "f"
 .Linfo_string4:
         .asciz  "int"
-.Linfo_string5:
-        .asciz  "x"
+.Lx0:
+        .asciz  "x0"
+.Lx1:
+        .asciz  "x1"
+.Lx2:
+        .asciz  "x2"
 
         .section        .debug_loc,"",@progbits
 .Ldebug_loc0:
@@ -42,6 +54,10 @@ f:                                      # @f
         .byte   80                      # super-register DW_OP_reg0
         .quad   0
         .quad   0
+.Ldebug_loc2:
+        .quad   .Lfunc_begin0-.Lfunc_begin0
+        .quad   .Lfunc_end0-.Lfunc_begin0
+        .short  0xdead                  # Loc expr size
 
         .section        .debug_abbrev,"",@progbits
         .byte   1                       # Abbreviation Code
@@ -104,10 +120,18 @@ f:                                      # @f
         .quad   .Lfunc_begin0           # DW_AT_low_pc
         .long   .Lfunc_end0-.Lfunc_begin0 # DW_AT_high_pc
         .long   .Linfo_string3          # DW_AT_name
-        .long   83                      # DW_AT_type
-        .byte   3                       # Abbrev [3] 0x43:0xf DW_TAG_formal_parameter
+        .long   .Lint                   # DW_AT_type
+        .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
         .long   .Ldebug_loc0            # DW_AT_location
-        .long   .Linfo_string5          # DW_AT_name
+        .long   .Lx0                    # DW_AT_name
+        .long   .Lint-.Lcu_begin0       # DW_AT_type
+        .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
+        .long   0xdeadbeef              # DW_AT_location
+        .long   .Lx1                    # DW_AT_name
+        .long   .Lint-.Lcu_begin0       # DW_AT_type
+        .byte   3                       # Abbrev [3] DW_TAG_formal_parameter
+        .long   .Ldebug_loc2            # DW_AT_location
+        .long   .Lx2                    # DW_AT_name
         .long   .Lint-.Lcu_begin0       # DW_AT_type
         .byte   0                       # End Of Children Mark
 .Lint:

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 507e4fa4c7949469132b185a0ae97a50ce5d14c5..44aacfde43c06bf7478612c8c78a84a74dda5aba 100644 (file)
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -2708,29 +2708,6 @@ bool DWARFExpression::Evaluate(
   return true; // Return true on success
 }
 
-size_t DWARFExpression::LocationListSize(const DWARFUnit *dwarf_cu,
-                                         const DataExtractor &debug_loc_data,
-                                         lldb::offset_t offset) {
-  const lldb::offset_t debug_loc_offset = offset;
-  while (debug_loc_data.ValidOffset(offset)) {
-    lldb::addr_t start_addr = LLDB_INVALID_ADDRESS;
-    lldb::addr_t end_addr = LLDB_INVALID_ADDRESS;
-    if (!AddressRangeForLocationListEntry(dwarf_cu, debug_loc_data, &offset,
-                                          start_addr, end_addr))
-      break;
-
-    if (start_addr == 0 && end_addr == 0)
-      break;
-
-    uint16_t loc_length = debug_loc_data.GetU16(&offset);
-    offset += loc_length;
-  }
-
-  if (offset > debug_loc_offset)
-    return offset - debug_loc_offset;
-  return 0;
-}
-
 bool DWARFExpression::AddressRangeForLocationListEntry(
     const DWARFUnit *dwarf_cu, const DataExtractor &debug_loc_data,
     lldb::offset_t *offset_ptr, lldb::addr_t &low_pc, lldb::addr_t &high_pc) {

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index a7349d197004a70d5c955392cb04b2350bc325b6..e5b301d24c0c579fc6c0f61f4554bcf38b04d4e2 100644 (file)
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -343,17 +343,11 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
               *frame_base = DWARFExpression(
                   module, DataExtractor(data, block_offset, block_length), cu);
             } else {
-              const DWARFDataExtractor &debug_loc_data = dwarf.DebugLocData();
-              const dw_offset_t debug_loc_offset = form_value.Unsigned();
-
-              size_t loc_list_length = DWARFExpression::LocationListSize(
-                  cu, debug_loc_data, debug_loc_offset);
-              if (loc_list_length > 0) {
-                *frame_base = DWARFExpression(module,
-                                              DataExtractor(debug_loc_data,
-                                                            debug_loc_offset,
-                                                            loc_list_length),
-                                              cu);
+              DataExtractor data = dwarf.DebugLocData();
+              const dw_offset_t offset = form_value.Unsigned();
+              if (data.ValidOffset(offset)) {
+                data = DataExtractor(data, offset, data.GetByteSize() - offset);
+                *frame_base = DWARFExpression(module, data, cu);
                 if (lo_pc != LLDB_INVALID_ADDRESS) {
                   assert(lo_pc >= cu->GetBaseAddress());
                   frame_base->SetLocationListSlide(lo_pc -

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index f625e2bdedc123335c311410cf787151c65d23fb..f91c1237f13a5fdadbac5604a0fa739c5e872271 100644 (file)
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3302,17 +3302,11 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
                   module, DataExtractor(data, block_offset, block_length),
                   die.GetCU());
             } else {
-              const DWARFDataExtractor &debug_loc_data = DebugLocData();
-              const dw_offset_t debug_loc_offset = form_value.Unsigned();
-
-              size_t loc_list_length = DWARFExpression::LocationListSize(
-                  die.GetCU(), debug_loc_data, debug_loc_offset);
-              if (loc_list_length > 0) {
-                location = DWARFExpression(module,
-                                           DataExtractor(debug_loc_data,
-                                                         debug_loc_offset,
-                                                         loc_list_length),
-                                           die.GetCU());
+              DataExtractor data = DebugLocData();
+              const dw_offset_t offset = form_value.Unsigned();
+              if (data.ValidOffset(offset)) {
+                data = DataExtractor(data, offset, data.GetByteSize() - offset);
+                location = DWARFExpression(module, data, die.GetCU());
                 assert(func_low_pc != LLDB_INVALID_ADDRESS);
                 location.SetLocationListSlide(
                     func_low_pc -