public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
@ 2020-01-20 10:36 Achra, Nitika
  2020-01-23 22:34 ` Tom Tromey
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2020-01-20 10:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: George, Jini Susan, Ali Tamur

[-- Attachment #1: Type: text/plain, Size: 1868 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Hello,

GDB is throwing the error  'Dwarf Error: Cannot handle DW_FORM_loclistx in DWARF reader'  while loading the executable with -gdwarf-5 flag. This patch fixes that. Please find the attached patch file. Requesting the review.


*Support for DW_AT_loclists_base and DW_FORM_loclistx.

This patch handles DW_AT_loclists_base and DW_FORM_loclistx.
DW_AT_loclists_base is a new attribute added in DWARFv5 which 
points to the beginning of the offset table of .debug_loclist
section. Reference to the location list (DW_FORM_loclistx) is 
interpreted relative to this base. DW_FORM_loclistx is a new
form added in DWARFv5 which is used to access location list.

Tested by running the testsuite before and after the patch and there 
is no increase in the number of test cases that fails. Tested with both 
-gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with 
-gdwarf-4 as well as -gdwarf5 flags.

gdb/ChangeLog:

   *dwarf2read.c (cu_debug_loc_section): Added the declaration for the function.
   (read_loclist_index): New function declaration.
   (lookup_loclist_base): New function declaration.
   (read_loclist_header): New function declaration
   (dwarf2_cu): Added loclist_base and loclist_header field.
   (dwarf2_locate_dwo_sections): Handle .debug_loclist.dwo section.
   (read_full_die_1): Read the value of DW_AT_loclists_base.
   (read_attribute_reprocess): Handle DW_FORM_loclistx.
   (read_attribute_value): Handle DW_FORM_loclistx.
   (skip_one_die): Handle DW_FORM_loclistx.
   (attr_form_is_section_offset): Handle DW_FORM_loclistx.
   (read_loclist_index): Function definition.
   (lookup_loclist_base): Function definition.
   (read_loclist_header): Function definition.
   (loclist_header): New structure declaration.

Regards,
Nitika Achra

[-- Attachment #2: 0001-Support-for-DW_AT_loclists_base-and-DW_FORM_loclistx.patch --]
[-- Type: application/octet-stream, Size: 10698 bytes --]

From 485067b2f9b5f3e061fef89aad62a264a3bf3280 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Mon, 20 Jan 2020 14:07:29 +0530
Subject: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

This patch handles DW_AT_loclists_base and DW_FORM_loclistx.
DW_AT_loclists_base is a new attribute added in DWARFv5 which
points to the beginning of the offset table of .debug_loclist
section. Reference to the location list (DW_FORM_loclistx) is
interpreted relative to this base. DW_FORM_loclistx is a new
form added in DWARFv5 which is used to access location list.

Tested by running the testsuite before and after the patch and there
is no increase in the number of test cases that fails. Tested with both
-gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with
-gdwarf-4 as well as -gdwarf5 flags.

gdb/ChangeLog:

   *dwarf2read.c (cu_debug_loc_section): Added the declaration for the function.
   (read_loclist_index): New function declaration.
   (lookup_loclist_base): New function declaration.
   (read_loclist_header): New function declaration
   (dwarf2_cu): Added loclist_base and loclist_header field.
   (dwarf2_locate_dwo_sections): Handle .debug_loclist.dwo section.
   (read_full_die_1): Read the value of DW_AT_loclists_base.
   (read_attribute_reprocess): Handle DW_FORM_loclistx.
   (read_attribute_value): Handle DW_FORM_loclistx.
   (skip_one_die): Handle DW_FORM_loclistx.
   (attr_form_is_section_offset): Handle DW_FORM_loclistx.
   (read_loclist_index): Function definition.
   (lookup_loclist_base): Function definition.
   (read_loclist_header): Function definition.
   (loclist_header): New structure declaration.
---
 gdb/dwarf2read.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 140 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index dfa2f91d45..9bd249ba72 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -104,6 +104,12 @@ static int dwarf2_loclist_index;
 static int dwarf2_locexpr_block_index;
 static int dwarf2_loclist_block_index;
 
+/* Size of .debug_loclist section header for 32-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE32 12;
+
+/* Size of .debug_loclist section header for 64-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE64 20;
+
 /* An index into a (C++) symbol name component in a symbol name as
    recorded in the mapped_index's symbol table.  For each C++ symbol
    in the symbol table, we record one entry for the start of each
@@ -373,6 +379,30 @@ struct comp_unit_head
   cu_offset type_cu_offset_in_tu;
 };
 
+/* The location list section (.debug_loclist) begins with a header,
+   which contains the following information. */
+struct loclist_header
+{
+  /* A 4-byte or 12-byte length containing the length of the
+  set of entries for this compilation unit, not including the
+  length field itself. */
+  unsigned int length;
+
+  /* A 2-byte version identifier. */
+  short version;
+
+  /* A 1-byte unsigned integer containing the size in bytes of an address on
+     the target system. */
+  unsigned char addr_size;
+
+  /* A 1-byte unsigned integer containing the size in bytes of a segment selector
+     on the target system. */
+  unsigned char segment_collector_size;
+
+  /* A 4-byte count of the number of offsets that follow the header. */
+  unsigned int offset_entry_count;
+};
+
 /* Type used for delaying computation of method physnames.
    See comments for compute_delayed_physnames.  */
 struct delayed_method_info
@@ -473,6 +503,9 @@ public:
      die_info->offset.sect_off as hash.  */
   htab_t die_hash = nullptr;
 
+  /* Header data from the location list section. */
+  struct loclist_header* loclist_header;
+
   /* Full DIEs if read in.  */
   struct die_info *dies = nullptr;
 
@@ -523,6 +556,9 @@ public:
      whether the DW_AT_ranges attribute came from the skeleton or DWO.  */
   ULONGEST ranges_base = 0;
 
+  /* The DW_AT_loclists_base attribute if present. */
+  gdb::optional<ULONGEST> loclist_base;
+
   /* When reading debug info generated by older versions of rustc, we
      have to rewrite some union types to be struct types with a
      variant part.  This rewriting must be done after the CU is fully
@@ -1705,6 +1741,14 @@ static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
 static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
 			       struct dwarf2_cu *, struct partial_symtab *);
 
+static struct dwarf2_section_info* cu_debug_loc_section (struct dwarf2_cu* cu);
+
+static CORE_ADDR read_loclist_index (struct dwarf2_cu* cu, ULONGEST loclist_index);
+
+static ULONGEST lookup_loclist_base (struct dwarf2_cu* cu);
+
+static void read_loclist_header (struct dwarf2_cu* cu, struct dwarf2_section_info* section);
+
 /* How dwarf2_get_pc_bounds constructed its *LOWPC and *HIGHPC return
    values.  Keep the items ordered with increasing constraints compliance.  */
 enum pc_bounds_kind
@@ -9445,6 +9489,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_GNU_addr_index:
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_rnglistx:
+	case DW_FORM_loclistx:
 	  info_ptr = safe_skip_leb128 (info_ptr, buffer_end);
 	  break;
 	case DW_FORM_indirect:
@@ -12979,6 +13024,11 @@ dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
       dwo_sections->loc.s.section = sectp;
       dwo_sections->loc.size = bfd_section_size (sectp);
     }
+  else if (section_is_p (sectp->name, &names->loclists_dwo))
+    {
+      dwo_sections->loclists.s.section = sectp;
+      dwo_sections->loclists.size = bfd_section_size (sectp);
+    }
   else if (section_is_p (sectp->name, &names->macinfo_dwo))
     {
       dwo_sections->macinfo.s.section = sectp;
@@ -18453,6 +18503,9 @@ read_full_die_1 (const struct die_reader_specs *reader,
   struct attribute *attr = dwarf2_attr_no_follow (die, DW_AT_str_offsets_base);
   if (attr != nullptr)
     cu->str_offsets_base = DW_UNSND (attr);
+  attr = dwarf2_attr_no_follow (die, DW_AT_loclists_base);
+  if (attr)
+    cu->loclist_base = DW_UNSND (attr);
 
   auto maybe_addr_base = lookup_addr_base(die);
   if (maybe_addr_base.has_value ())
@@ -19409,6 +19462,81 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
   fixup_called = 1;
 }
 
+void
+read_loclist_header (struct dwarf2_cu* cu, struct dwarf2_section_info* section)
+{
+  unsigned int bytes_read;
+  bfd* abfd = get_section_bfd_owner (section);
+  const gdb_byte* info_ptr = section->buffer;
+  cu->loclist_header = new loclist_header();
+  cu->loclist_header->length = read_initial_length (abfd, info_ptr, &bytes_read);
+  info_ptr += bytes_read;
+  cu->loclist_header->version = read_2_bytes (abfd, info_ptr);
+  info_ptr += 2;
+  cu->loclist_header->addr_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  cu->loclist_header->segment_collector_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  cu->loclist_header->offset_entry_count = read_4_bytes (abfd, info_ptr);
+}
+
+
+ULONGEST
+lookup_loclist_base (struct dwarf2_cu* cu)
+{
+  /* For the .dwo unit, the loclist_base points to the first offset following
+     the header. The header consists of the following entities-
+     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
+     2. version (2 bytes)
+     3. address size (1 byte)
+     4. segment selector size (1 byte)
+     5. offset entry count (4 bytes)
+     These sizes are derived as per the DWARFv5 standard. */
+  if (cu->dwo_unit)
+  {
+    if (cu->header.initial_length_size == 4)
+      return LOCLIST_HEADER_SIZE32;
+    return LOCLIST_HEADER_SIZE64;
+  }
+  return *cu->loclist_base;
+}
+
+/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
+   of offsets in the .debug_loclists section. */
+CORE_ADDR
+read_loclist_index (struct dwarf2_cu* cu, ULONGEST loclist_index)
+{
+  struct dwarf2_per_objfile* dwarf2_per_objfile
+	= cu->per_cu->dwarf2_per_objfile;
+  struct objfile* objfile = dwarf2_per_objfile->objfile;
+  bfd* abfd = objfile->obfd;
+  ULONGEST loclist_base = lookup_loclist_base (cu);
+  const gdb_byte* info_ptr;
+  struct dwarf2_section_info* section = cu_debug_loc_section (cu);
+  dwarf2_read_section (objfile, section);
+  if (section->buffer == NULL)
+    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  read_loclist_header (cu, section);
+  if (loclist_index >= cu->loclist_header->offset_entry_count)
+    error(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist offset array [in module %s]"),
+	objfile_name (objfile));
+  if (loclist_base + loclist_index * cu->header.offset_size
+	>= section->size)
+    error(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  info_ptr = (section->buffer + loclist_base +
+	loclist_index * cu->header.offset_size);
+  delete cu->loclist_header;
+  cu->loclist_header = NULL;
+  if (cu->header.offset_size == 4)
+    return bfd_get_32 (abfd, info_ptr) + loclist_base;
+  else
+    return bfd_get_64 (abfd, info_ptr) + loclist_base;
+}
+
 /* Process the attributes that had to be skipped in the first round. These
    attributes are the ones that need str_offsets_base or addr_base attributes.
    They could not have been processed in the first round, because at the time
@@ -19423,6 +19551,9 @@ void read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_GNU_addr_index:
         DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
         break;
+      case DW_FORM_loclistx:
+	 DW_UNSND (attr) = read_loclist_index (cu, DW_UNSND (attr));
+	 break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
       case DW_FORM_strx2:
@@ -19526,6 +19657,13 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_UNSND (attr) = read_offset (abfd, info_ptr, &cu->header, &bytes_read);
       info_ptr += bytes_read;
       break;
+    case DW_FORM_loclistx:
+    {
+      *need_reprocess = true;
+      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      info_ptr += bytes_read;
+    }
+      break;
     case DW_FORM_string:
       DW_STRING (attr) = read_direct_string (abfd, info_ptr, &bytes_read);
       DW_STRING_IS_CANONICAL (attr) = 0;
@@ -25407,7 +25545,8 @@ attr_form_is_section_offset (const struct attribute *attr)
 {
   return (attr->form == DW_FORM_data4
           || attr->form == DW_FORM_data8
-	  || attr->form == DW_FORM_sec_offset);
+	  || attr->form == DW_FORM_sec_offset
+	  || attr->form == DW_FORM_loclistx);
 }
 
 /* Return non-zero if ATTR's value falls in the 'constant' class, or
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-01-20 10:36 [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx Achra, Nitika
@ 2020-01-23 22:34 ` Tom Tromey
  2020-01-31  8:59   ` Achra, Nitika
  0 siblings, 1 reply; 7+ messages in thread
From: Tom Tromey @ 2020-01-23 22:34 UTC (permalink / raw)
  To: Achra, Nitika; +Cc: gdb-patches, George, Jini Susan, Ali Tamur

>>>>> "Nitika" == Achra, Nitika <Nitika.Achra@amd.com> writes:

Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.

Thanks for the patch.

My comments below are primarily nits.

Nitika> Tested by running the testsuite before and after the patch and there
Nitika> is no increase in the number of test cases that fails. Tested with both
Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with
Nitika> -gdwarf-4 as well as -gdwarf5 flags.

I assume it fixed some tests with -gdwarf-5?  Or else we'll need a new
test case.

Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF format. */
Nitika> +#define LOCLIST_HEADER_SIZE32 12;
Nitika> +
Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF format. */
Nitika> +#define LOCLIST_HEADER_SIZE64 20;

The ";" at the end of these defines is weird.

Nitika> +  /* Header data from the location list section. */
Nitika> +  struct loclist_header* loclist_header;

gdb style puts the "*" on the other side of the " " like

    struct loclist_header *loclist_header;

Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct dwarf2_cu* cu);

Here too -- there are actually several instances in the patch.

Nitika> +void
Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct dwarf2_section_info* section)
Nitika> +{

New functions should have an introductory comment explaining their purpose.

The "static" should be repeated here, rather than rely on the
declaration.  This affects all of the new functions, I think.  Also,
there's no need to forward declare them if the uses come after the
definition, so probably some of those declarations can be removed as
well.

It might be nicer if read_loclist_header took a pointer to the
loclist_header rather than a dwarf2_cu, and did not allocate.  See
below.

Nitika> +ULONGEST
Nitika> +lookup_loclist_base (struct dwarf2_cu* cu)
Nitika> +{
Nitika> +  /* For the .dwo unit, the loclist_base points to the first offset following
Nitika> +     the header. The header consists of the following entities-
Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
Nitika> +     2. version (2 bytes)
Nitika> +     3. address size (1 byte)
Nitika> +     4. segment selector size (1 byte)
Nitika> +     5. offset entry count (4 bytes)
Nitika> +     These sizes are derived as per the DWARFv5 standard. */
Nitika> +  if (cu->dwo_unit)
Nitika> +  {
Nitika> +    if (cu->header.initial_length_size == 4)
Nitika> +      return LOCLIST_HEADER_SIZE32;
Nitika> +    return LOCLIST_HEADER_SIZE64;

Is there some way to avoid hard-coding sizes here?

Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
Nitika> +   of offsets in the .debug_loclists section. */
Nitika> +CORE_ADDR
Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST loclist_index)
Nitika> +{
...
Nitika> +  const gdb_byte* info_ptr;

This can be declared later, when it's first initialized.

Nitika> +  if (section->buffer == NULL)
Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
Nitika> +	objfile_name (objfile));

I wonder whether errors here will really do something good.  The problem
is that the DWARF reader, in general, doesn't handle errors very well.
It *should* -- but it doesn't.  I don't know about this spot, but in
other places, calling error will mean that reading all of the debuginfo
for the entire file will be aborted.  (It can even cause worse problems,
there's a bug in bugzilla about it ending a remote session.)

Maybe complaint() and then a fallback would be preferable.
Or, test the error() to make sure it's ok.

Nitika> +  delete cu->loclist_header;

This is created, only to be deleted in the same function.
I think it would be better to just stack-allocate this.

Or, should this be cached in the CU -- that is, read once and then reused?
If so then a different approach should be used.  It wasn't clear to me
how often read_loclist_index is called.

Nitika> +    case DW_FORM_loclistx:
Nitika> +    {
Nitika> +      *need_reprocess = true;
Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);

Space before the first "(".

Tom

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-01-23 22:34 ` Tom Tromey
@ 2020-01-31  8:59   ` Achra, Nitika
  2020-02-02  3:10     ` Ali Tamur via gdb-patches
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2020-01-31  8:59 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches, George, Jini Susan, Ali Tamur

[-- Attachment #1: Type: text/plain, Size: 5583 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Hello Tom,

Thanks for the review. I have incorporated the review comments. Please have a look.

Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.

Tom>Thanks for the patch.

Tom>My comments below are primarily nits.

Nitika> Tested by running the testsuite before and after the patch and 
Nitika> there is no increase in the number of test cases that fails. 
Nitika> Tested with both
Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along 
Nitika> with
Nitika> -gdwarf-4 as well as -gdwarf5 flags.

Tom> I assume it fixed some tests with -gdwarf-5?  Or else we'll need a new test case.

Added a new test case. This test checks if the file command passes without any error with -g-dwarf-5 and -gsplit-dwarf. 
Gcc emits DW_FORM_loclistx only with -gsplit-dwarf. I can also check by printing the variable value, but right now printing
the value is throwing  the error- " DWARF ERROR: Corrupted dwarf expression"  which is not related to this patch.  I have submitted 
another patch which will fix that.

Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF 
Nitika> +format. */ #define LOCLIST_HEADER_SIZE32 12;
Nitika> +
Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF 
Nitika> +format. */ #define LOCLIST_HEADER_SIZE64 20;

Tom> The ";" at the end of these defines is weird.
Removed

Nitika> +  /* Header data from the location list section. */  struct 
Nitika> + loclist_header* loclist_header;

Tom> gdb style puts the "*" on the other side of the " " like

Tom> struct loclist_header *loclist_header;

Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct 
Nitika> +dwarf2_cu* cu);

Tom> Here too -- there are actually several instances in the patch.

Done

Nitika> +void
Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct 
Nitika> +dwarf2_section_info* section) {

Tom> New functions should have an introductory comment explaining their purpose.
Done

Tom> The "static" should be repeated here, rather than rely on the declaration.  This affects all of the new functions, I think.  Also, there's no need to forward Tom>declare them if the uses come after the definition, so probably some of those declarations can be removed as well.
Tom>Added static in definitions also. 

Done

Tom>It might be nicer if read_loclist_header took a pointer to the loclist_header rather than a dwarf2_cu, and did not allocate.  See below.

Done

Nitika> +ULONGEST
Nitika> +lookup_loclist_base (struct dwarf2_cu* cu) {
Nitika> +  /* For the .dwo unit, the loclist_base points to the first offset following
Nitika> +     the header. The header consists of the following entities-
Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
Nitika> +     2. version (2 bytes)
Nitika> +     3. address size (1 byte)
Nitika> +     4. segment selector size (1 byte)
Nitika> +     5. offset entry count (4 bytes)
Nitika> +     These sizes are derived as per the DWARFv5 standard. */
Nitika> +  if (cu->dwo_unit)
Nitika> +  {
Nitika> +    if (cu->header.initial_length_size == 4)
Nitika> +      return LOCLIST_HEADER_SIZE32;
Nitika> +    return LOCLIST_HEADER_SIZE64;

Tom>Is there some way to avoid hard-coding sizes here?

I thought of using sizeof(struct loclist_header) in place of LOCLIST_HEADER_SIZE32 and sizeof(struct loclist_header) + cu->initial_length_size in place of 
LOCLIST_HEADER_SIZE_64. But I am not sure if this a correct way of doing this. Some compilers append some padding at the end of structure. So the
size of structure might not be equal to the sum of size of its members. Sizeof() is also compiler dependent. So, right now I cannot think of any other way.

Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
Nitika> +   of offsets in the .debug_loclists section. */ CORE_ADDR 
Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST 
Nitika> +loclist_index) {
...
Nitika> +  const gdb_byte* info_ptr;

Tom> This can be declared later, when it's first initialized.
Done

Nitika> +  if (section->buffer == NULL)
Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
Nitika> +       objfile_name (objfile));

Tom>I wonder whether errors here will really do something good.  The problem is that the DWARF reader, in general, doesn't handle errors very well.
Tom>It *should* -- but it doesn't.  I don't know about this spot, but in other places, calling error will mean that reading all of the debuginfo for the entire file Tom>will be aborted.  (It can even cause worse problems, there's a bug in bugzilla about it ending a remote session.)

Tom>Maybe complaint() and then a fallback would be preferable.
Tom>Or, test the error() to make sure it's ok.

Replaced with complaint()

Nitika> +  delete cu->loclist_header;

Tom>This is created, only to be deleted in the same function.
Tom>I think it would be better to just stack-allocate this.

Done

Tom>Or, should this be cached in the CU -- that is, read once and then reused?
Tom>If so then a different approach should be used.  It wasn't clear to me how often read_loclist_index is called.

Nitika> +    case DW_FORM_loclistx:
Nitika> +    {
Nitika> +      *need_reprocess = true;
Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr, 
Nitika> + &bytes_read);

Tom>Space before the first "(".
Done

Regards,
Nitika Achra

[-- Attachment #2: 0001-Support-for-DW_AT_loclists_base-and-DW_FORM_loclistx.patch --]
[-- Type: application/octet-stream, Size: 12495 bytes --]

From e643d4f59486790f00fc495f244dab970402fb56 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Mon, 20 Jan 2020 14:07:29 +0530
Subject: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

This patch handles DW_AT_loclists_base and DW_FORM_loclistx.
DW_AT_loclists_base is a new attribute added in DWARFv5 which
points to the beginning of the offset table of .debug_loclist
section. Reference to the location list (DW_FORM_loclistx) is
interpreted relative to this base. DW_FORM_loclistx is a new
form added in DWARFv5 which is used to access location list.

Tested by running the testsuite before and after the patch and there
is no increase in the number of test cases that fails. Tested with both
-gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with
-gdwarf-4 as well as -gdwarf5 flags. Used gcc-9.4 for testing.

gdb/ChangeLog:

   *dwarf2read.c (cu_debug_loc_section): Added the declaration for the function.
   (read_loclist_index): New function declaration.
   (lookup_loclist_base): New function declaration.
   (read_loclist_header): New function declaration
   (dwarf2_cu): Added loclist_base and loclist_header field.
   (dwarf2_locate_dwo_sections): Handle .debug_loclist.dwo section.
   (read_full_die_1): Read the value of DW_AT_loclists_base.
   (read_attribute_reprocess): Handle DW_FORM_loclistx.
   (read_attribute_value): Handle DW_FORM_loclistx.
   (skip_one_die): Handle DW_FORM_loclistx.
   (attr_form_is_section_offset): Handle DW_FORM_loclistx.
   (read_loclist_index): Function definition.
   (lookup_loclist_base): Function definition.
   (read_loclist_header): Function definition.
   (loclist_header): New structure declaration.

gdb/testsuite/ChangeLog:

   *gdb.dwarf2/dw5-form-loclistx.exp: New file.
   *gdb.dwarf2/dw5-form-loclistx.c: New file.
---
 gdb/dwarf2read.c                              | 130 +++++++++++++++++-
 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c  |  16 +++
 .../gdb.dwarf2/dw5-form-loclistx.exp          |  42 ++++++
 3 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index dfa2f91d45..f1096570fc 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -104,6 +104,12 @@ static int dwarf2_loclist_index;
 static int dwarf2_locexpr_block_index;
 static int dwarf2_loclist_block_index;
 
+/* Size of .debug_loclist section header for 32-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE32 12
+
+/* Size of .debug_loclist section header for 64-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE64 20
+
 /* An index into a (C++) symbol name component in a symbol name as
    recorded in the mapped_index's symbol table.  For each C++ symbol
    in the symbol table, we record one entry for the start of each
@@ -373,6 +379,30 @@ struct comp_unit_head
   cu_offset type_cu_offset_in_tu;
 };
 
+/* The location list section (.debug_loclist) begins with a header,
+   which contains the following information. */
+struct loclist_header
+{
+  /* A 4-byte or 12-byte length containing the length of the
+  set of entries for this compilation unit, not including the
+  length field itself. */
+  unsigned int length;
+
+  /* A 2-byte version identifier. */
+  short version;
+
+  /* A 1-byte unsigned integer containing the size in bytes of an address on
+     the target system. */
+  unsigned char addr_size;
+
+  /* A 1-byte unsigned integer containing the size in bytes of a segment selector
+     on the target system. */
+  unsigned char segment_collector_size;
+
+  /* A 4-byte count of the number of offsets that follow the header. */
+  unsigned int offset_entry_count;
+};
+
 /* Type used for delaying computation of method physnames.
    See comments for compute_delayed_physnames.  */
 struct delayed_method_info
@@ -523,6 +553,9 @@ public:
      whether the DW_AT_ranges attribute came from the skeleton or DWO.  */
   ULONGEST ranges_base = 0;
 
+  /* The DW_AT_loclists_base attribute if present. */
+  gdb::optional<ULONGEST> loclist_base;
+
   /* When reading debug info generated by older versions of rustc, we
      have to rewrite some union types to be struct types with a
      variant part.  This rewriting must be done after the CU is fully
@@ -1705,6 +1738,9 @@ static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
 static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
 			       struct dwarf2_cu *, struct partial_symtab *);
 
+/* Return the .debug_loclist section to use for cu. */
+static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
+
 /* How dwarf2_get_pc_bounds constructed its *LOWPC and *HIGHPC return
    values.  Keep the items ordered with increasing constraints compliance.  */
 enum pc_bounds_kind
@@ -9445,6 +9481,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_GNU_addr_index:
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_rnglistx:
+	case DW_FORM_loclistx:
 	  info_ptr = safe_skip_leb128 (info_ptr, buffer_end);
 	  break;
 	case DW_FORM_indirect:
@@ -12979,6 +13016,11 @@ dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
       dwo_sections->loc.s.section = sectp;
       dwo_sections->loc.size = bfd_section_size (sectp);
     }
+  else if (section_is_p (sectp->name, &names->loclists_dwo))
+    {
+      dwo_sections->loclists.s.section = sectp;
+      dwo_sections->loclists.size = bfd_section_size (sectp);
+    }
   else if (section_is_p (sectp->name, &names->macinfo_dwo))
     {
       dwo_sections->macinfo.s.section = sectp;
@@ -18453,6 +18495,9 @@ read_full_die_1 (const struct die_reader_specs *reader,
   struct attribute *attr = dwarf2_attr_no_follow (die, DW_AT_str_offsets_base);
   if (attr != nullptr)
     cu->str_offsets_base = DW_UNSND (attr);
+  attr = dwarf2_attr_no_follow (die, DW_AT_loclists_base);
+  if (attr)
+    cu->loclist_base = DW_UNSND (attr);
 
   auto maybe_addr_base = lookup_addr_base(die);
   if (maybe_addr_base.has_value ())
@@ -19409,6 +19454,78 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
   fixup_called = 1;
 }
 
+static void
+read_loclist_header (struct loclist_header *header, struct dwarf2_section_info *section)
+{
+  unsigned int bytes_read;
+  bfd *abfd = get_section_bfd_owner (section);
+  const gdb_byte *info_ptr = section->buffer;
+  header->length = read_initial_length (abfd, info_ptr, &bytes_read);
+  info_ptr += bytes_read;
+  header->version = read_2_bytes (abfd, info_ptr);
+  info_ptr += 2;
+  header->addr_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->segment_collector_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->offset_entry_count = read_4_bytes (abfd, info_ptr);
+}
+
+
+static ULONGEST
+lookup_loclist_base (struct dwarf2_cu *cu)
+{
+  /* For the .dwo unit, the loclist_base points to the first offset following
+     the header. The header consists of the following entities-
+     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
+     2. version (2 bytes)
+     3. address size (1 byte)
+     4. segment selector size (1 byte)
+     5. offset entry count (4 bytes)
+     These sizes are derived as per the DWARFv5 standard. */
+  if (cu->dwo_unit)
+  {
+    if (cu->header.initial_length_size == 4)
+      return LOCLIST_HEADER_SIZE32;
+    return LOCLIST_HEADER_SIZE64;
+  }
+  return *cu->loclist_base;
+}
+
+/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
+   of offsets in the .debug_loclists section. */
+static CORE_ADDR
+read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
+{
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+	= cu->per_cu->dwarf2_per_objfile;
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
+  bfd *abfd = objfile->obfd;
+  ULONGEST loclist_base = lookup_loclist_base (cu);
+  struct dwarf2_section_info *section = cu_debug_loc_section (cu);
+  dwarf2_read_section (objfile, section);
+  if (section->buffer == NULL)
+    complaint(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  struct loclist_header header;
+  read_loclist_header (&header, section);
+  if (loclist_index >= header.offset_entry_count)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist offset array [in module %s]"),
+	objfile_name (objfile));
+  if (loclist_base + loclist_index * cu->header.offset_size
+	>= section->size)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  const gdb_byte *info_ptr = (section->buffer + loclist_base +
+	loclist_index * cu->header.offset_size);
+  if (cu->header.offset_size == 4)
+    return bfd_get_32 (abfd, info_ptr) + loclist_base;
+  else
+    return bfd_get_64 (abfd, info_ptr) + loclist_base;
+}
+
 /* Process the attributes that had to be skipped in the first round. These
    attributes are the ones that need str_offsets_base or addr_base attributes.
    They could not have been processed in the first round, because at the time
@@ -19423,6 +19540,9 @@ void read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_GNU_addr_index:
         DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
         break;
+      case DW_FORM_loclistx:
+	 DW_UNSND (attr) = read_loclist_index (cu, DW_UNSND (attr));
+	 break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
       case DW_FORM_strx2:
@@ -19526,6 +19646,13 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_UNSND (attr) = read_offset (abfd, info_ptr, &cu->header, &bytes_read);
       info_ptr += bytes_read;
       break;
+    case DW_FORM_loclistx:
+    {
+      *need_reprocess = true;
+      DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      info_ptr += bytes_read;
+    }
+      break;
     case DW_FORM_string:
       DW_STRING (attr) = read_direct_string (abfd, info_ptr, &bytes_read);
       DW_STRING_IS_CANONICAL (attr) = 0;
@@ -25407,7 +25534,8 @@ attr_form_is_section_offset (const struct attribute *attr)
 {
   return (attr->form == DW_FORM_data4
           || attr->form == DW_FORM_data8
-	  || attr->form == DW_FORM_sec_offset);
+	  || attr->form == DW_FORM_sec_offset
+	  || attr->form == DW_FORM_loclistx);
 }
 
 /* Return non-zero if ATTR's value falls in the 'constant' class, or
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
new file mode 100644
index 0000000000..a2a14583be
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
@@ -0,0 +1,16 @@
+
+
+int foo(int a, int b)
+{
+  a = a+b;
+  b = a-b;
+  a = 2*b;
+  return a+b;
+}
+
+int main()
+{
+  int result,a,b;
+  result = foo(a,b);
+  return result;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
new file mode 100644
index 0000000000..15d9ac13e7
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
@@ -0,0 +1,42 @@
+# Copyright 2012-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile .c
+
+
+# We can't use prepare_for_testing here because we need to check the
+# 'file' command's output.
+
+if { [build_executable ${testfile}.exp ${testfile} ${srcfile}\
+   {additional_flags=-gdwarf-5 additional_flags=-gsplit-dwarf additional_flags=-O2}] == -1 } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Check that file command gives no error.
+gdb_test "file $binfile" \
+    "Reading symbols from $binfile\.\.\."
+
+
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-01-31  8:59   ` Achra, Nitika
@ 2020-02-02  3:10     ` Ali Tamur via gdb-patches
  2020-02-24  8:48       ` Achra, Nitika
  0 siblings, 1 reply; 7+ messages in thread
From: Ali Tamur via gdb-patches @ 2020-02-02  3:10 UTC (permalink / raw)
  To: Achra, Nitika; +Cc: Tom Tromey, gdb-patches, George, Jini Susan

Hi Nikita,

Thank you for this patch. I think you have a bug. (At least the version I
have seems to have a bug)

static CORE_ADDR
read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
{
...
  read_loclist_header (&header, section);   // (*)
}

I think (*) would be correct only when there is a single location list
header in the .debug_loclists section. In the more general case where there
are many, info_ptr should start reading the header at:
section->buffer + loclist_base - (LOCLIST_HEADER_SIZE32
or LOCLIST_HEADER_SIZE64);

Am I missing something?

My example executable is:
int main() {
  return 17;
}
compiled with clang++, -gdwarf-5 and no split dwarf but also linked with
libc++ library. With your code, gdb cannot read debug_info; with the
correction, everything seems fine.

I have another suggestion/question. Is DW_AT_location allowed to have a
DW_FORM_loclistx form? In the dwarf-dump of my example, I see:
0x00032ee5: DW_TAG_formal_parameter
            DW_AT_location      (indexed (0x85c) loclist = 0x000109d5:
              [0x000000000033b5c0, 0x000000000033b5d0): DW_OP_reg5 RDI
              [0x000000000033b5d0, 0x000000000033b62b): DW_OP_reg15 R15
              [0x000000000033b631, 0x000000000033b65e): DW_OP_reg15 R15)
              DW_AT_name       ("ptr")
              DW_AT_decl_file  ("/path/to/tcmalloc.cc")
              DW_AT_decl_line  (1385)
              DW_AT_type       (0x0003d6d6 "*")

But then:
partial_die_info::read (  ...
  switch (attr.name) case DW_AT_location:
      ...
      else if (attr_form_is_section_offset (&attr)) {
        dwarf2_complex_location_expr_complaint ();  // ***

*** fires, I think needlessly. Could this complaint be obsolete?

Thank you,
Ali


On Fri, Jan 31, 2020 at 12:55 AM Achra, Nitika <Nitika.Achra@amd.com> wrote:

> [AMD Official Use Only - Internal Distribution Only]
>
> Hello Tom,
>
> Thanks for the review. I have incorporated the review comments. Please
> have a look.
>
> Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.
>
> Tom>Thanks for the patch.
>
> Tom>My comments below are primarily nits.
>
> Nitika> Tested by running the testsuite before and after the patch and
> Nitika> there is no increase in the number of test cases that fails.
> Nitika> Tested with both
> Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along
> Nitika> with
> Nitika> -gdwarf-4 as well as -gdwarf5 flags.
>
> Tom> I assume it fixed some tests with -gdwarf-5?  Or else we'll need a
> new test case.
>
> Added a new test case. This test checks if the file command passes without
> any error with -g-dwarf-5 and -gsplit-dwarf.
> Gcc emits DW_FORM_loclistx only with -gsplit-dwarf. I can also check by
> printing the variable value, but right now printing
> the value is throwing  the error- " DWARF ERROR: Corrupted dwarf
> expression"  which is not related to this patch.  I have submitted
> another patch which will fix that.
>
> Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF
> Nitika> +format. */ #define LOCLIST_HEADER_SIZE32 12;
> Nitika> +
> Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF
> Nitika> +format. */ #define LOCLIST_HEADER_SIZE64 20;
>
> Tom> The ";" at the end of these defines is weird.
> Removed
>
> Nitika> +  /* Header data from the location list section. */  struct
> Nitika> + loclist_header* loclist_header;
>
> Tom> gdb style puts the "*" on the other side of the " " like
>
> Tom> struct loclist_header *loclist_header;
>
> Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct
> Nitika> +dwarf2_cu* cu);
>
> Tom> Here too -- there are actually several instances in the patch.
>
> Done
>
> Nitika> +void
> Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct
> Nitika> +dwarf2_section_info* section) {
>
> Tom> New functions should have an introductory comment explaining their
> purpose.
> Done
>
> Tom> The "static" should be repeated here, rather than rely on the
> declaration.  This affects all of the new functions, I think.  Also,
> there's no need to forward Tom>declare them if the uses come after the
> definition, so probably some of those declarations can be removed as well.
> Tom>Added static in definitions also.
>
> Done
>
> Tom>It might be nicer if read_loclist_header took a pointer to the
> loclist_header rather than a dwarf2_cu, and did not allocate.  See below.
>
> Done
>
> Nitika> +ULONGEST
> Nitika> +lookup_loclist_base (struct dwarf2_cu* cu) {
> Nitika> +  /* For the .dwo unit, the loclist_base points to the first
> offset following
> Nitika> +     the header. The header consists of the following entities-
> Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12
> bytes for the 64 bit format)
> Nitika> +     2. version (2 bytes)
> Nitika> +     3. address size (1 byte)
> Nitika> +     4. segment selector size (1 byte)
> Nitika> +     5. offset entry count (4 bytes)
> Nitika> +     These sizes are derived as per the DWARFv5 standard. */
> Nitika> +  if (cu->dwo_unit)
> Nitika> +  {
> Nitika> +    if (cu->header.initial_length_size == 4)
> Nitika> +      return LOCLIST_HEADER_SIZE32;
> Nitika> +    return LOCLIST_HEADER_SIZE64;
>
> Tom>Is there some way to avoid hard-coding sizes here?
>
> I thought of using sizeof(struct loclist_header) in place of
> LOCLIST_HEADER_SIZE32 and sizeof(struct loclist_header) +
> cu->initial_length_size in place of
> LOCLIST_HEADER_SIZE_64. But I am not sure if this a correct way of doing
> this. Some compilers append some padding at the end of structure. So the
> size of structure might not be equal to the sum of size of its members.
> Sizeof() is also compiler dependent. So, right now I cannot think of any
> other way.
>
> Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset
> from the array
> Nitika> +   of offsets in the .debug_loclists section. */ CORE_ADDR
> Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST
> Nitika> +loclist_index) {
> ...
> Nitika> +  const gdb_byte* info_ptr;
>
> Tom> This can be declared later, when it's first initialized.
> Done
>
> Nitika> +  if (section->buffer == NULL)
> Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section
> [in module %s]"),
> Nitika> +       objfile_name (objfile));
>
> Tom>I wonder whether errors here will really do something good.  The
> problem is that the DWARF reader, in general, doesn't handle errors very
> well.
> Tom>It *should* -- but it doesn't.  I don't know about this spot, but in
> other places, calling error will mean that reading all of the debuginfo for
> the entire file Tom>will be aborted.  (It can even cause worse problems,
> there's a bug in bugzilla about it ending a remote session.)
>
> Tom>Maybe complaint() and then a fallback would be preferable.
> Tom>Or, test the error() to make sure it's ok.
>
> Replaced with complaint()
>
> Nitika> +  delete cu->loclist_header;
>
> Tom>This is created, only to be deleted in the same function.
> Tom>I think it would be better to just stack-allocate this.
>
> Done
>
> Tom>Or, should this be cached in the CU -- that is, read once and then
> reused?
> Tom>If so then a different approach should be used.  It wasn't clear to me
> how often read_loclist_index is called.
>
> Nitika> +    case DW_FORM_loclistx:
> Nitika> +    {
> Nitika> +      *need_reprocess = true;
> Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr,
> Nitika> + &bytes_read);
>
> Tom>Space before the first "(".
> Done
>
> Regards,
> Nitika Achra
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-02-02  3:10     ` Ali Tamur via gdb-patches
@ 2020-02-24  8:48       ` Achra, Nitika
  2020-03-17 14:13         ` Achra, Nitika
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2020-02-24  8:48 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey, Ali Tamur; +Cc: George, Jini Susan

[AMD Official Use Only - Internal Distribution Only]

Hi Tom,
Hi Ali,

Sorry for the delayed response. I was in a very poor internet connectivity area. Please look for my comments below.



From: Ali Tamur <tamur@google.com>
Sent: Sunday, February 2, 2020 8:40 AM
To: Achra, Nitika <Nitika.Achra@amd.com>
Cc: Tom Tromey <tom@tromey.com>; gdb-patches@sourceware.org; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

Ali>Hi Nikita,

Ali>Thank you for this patch. I think you have a bug. (At least the version I have seems to have a bug)

Ali>static CORE_ADDR
Ali>read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
Ali>{
Ali>...
Ali>  read_loclist_header (&header, section);   // (*)
Ali>}

Ali>I think (*) would be correct only when there is a single location list header in the .debug_loclists section. In the more general case where there are many, Ali>info_ptr should start reading the header at:
Ali>section->buffer + loclist_base - (LOCLIST_HEADER_SIZE32 or LOCLIST_HEADER_SIZE64);

Ali>Am I missing something?

Ali>My example executable is:
Ali>int main() {
Ali> return 17;
Ali>}
Ali>compiled with clang++, -gdwarf-5 and no split dwarf but also linked with libc++ library. With your code, gdb cannot read debug_info; with the correction, Ali>everything seems fine.

I am not able to reproduce this bug using the example executable. The example is working fine with me. I have used the following command-
clang++ -gdwarf-5 -O3  -stdlib=libc++  test.cpp
Could you please look into this again? Am I doing something wrong here?
I have used clang version 10.0.0.

Ali>I have another suggestion/question. Is DW_AT_location allowed to have a DW_FORM_loclistx form? In the dwarf-dump of my example, I see:
Ali>0x00032ee5: DW_TAG_formal_parameter
Ali>    DW_AT_location      (indexed (0x85c) loclist = 0x000109d5:
Ali>     [0x000000000033b5c0, 0x000000000033b5d0): DW_OP_reg5 RDI
Ali>     [0x000000000033b5d0, 0x000000000033b62b): DW_OP_reg15 R15
Ali>              [0x000000000033b631, 0x000000000033b65e): DW_OP_reg15 R15)
Ali>              DW_AT_name       ("ptr")
Ali>              DW_AT_decl_file  ("/path/to/tcmalloc.cc")
Ali>              DW_AT_decl_line  (1385)
Ali>              DW_AT_type       (0x0003d6d6 "*")

Ali> But then:
Ali> partial_die_info::read (  ...
Ali>  switch (attr.name<https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fattr.name%2F&data=02%7C01%7CNitika.Achra%40amd.com%7Cdc148d3943894e037b6a08d7a78d7268%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637162098268321041&sdata=uO7qWPYfgVGf6wlo2QbqaRQTTOof945hTOrJFCGhoJM%3D&reserved=0>) case DW_AT_location:
Ali>      ...
Ali>      else if (attr_form_is_section_offset (&attr)) {
Ali>        dwarf2_complex_location_expr_complaint ();  // ***

Ali> *** fires, I think needlessly. Could this complaint be obsolete?

As per the DWARFv5 standard DW_AT_location is allowed to have DW_FORM_loclistx. So this complaint should be obsolete now if only DW_FORM_loclistx is being used. But whether the section offsets like DW_FORM_sec_offset, DW_FORM_data4 and DW_FORM_data8 can still be emitted under DWARFv5 or not? If yes then the complaint should get fired for them but not for DW_FORM_loclistx. Am I right?

Ali>Thank you,
Ali>Ali

Regards,
Nitika


On Fri, Jan 31, 2020 at 12:55 AM Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>> wrote:
[AMD Official Use Only - Internal Distribution Only]

Hello Tom,

Thanks for the review. I have incorporated the review comments. Please have a look.

Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.

Tom>Thanks for the patch.

Tom>My comments below are primarily nits.

Nitika> Tested by running the testsuite before and after the patch and
Nitika> there is no increase in the number of test cases that fails.
Nitika> Tested with both
Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along
Nitika> with
Nitika> -gdwarf-4 as well as -gdwarf5 flags.

Tom> I assume it fixed some tests with -gdwarf-5?  Or else we'll need a new test case.

Added a new test case. This test checks if the file command passes without any error with -g-dwarf-5 and -gsplit-dwarf.
Gcc emits DW_FORM_loclistx only with -gsplit-dwarf. I can also check by printing the variable value, but right now printing
the value is throwing  the error- " DWARF ERROR: Corrupted dwarf expression"  which is not related to this patch.  I have submitted
another patch which will fix that.

Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE32 12;
Nitika> +
Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE64 20;

Tom> The ";" at the end of these defines is weird.
Removed

Nitika> +  /* Header data from the location list section. */  struct
Nitika> + loclist_header* loclist_header;

Tom> gdb style puts the "*" on the other side of the " " like

Tom> struct loclist_header *loclist_header;

Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct
Nitika> +dwarf2_cu* cu);

Tom> Here too -- there are actually several instances in the patch.

Done

Nitika> +void
Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct
Nitika> +dwarf2_section_info* section) {

Tom> New functions should have an introductory comment explaining their purpose.
Done

Tom> The "static" should be repeated here, rather than rely on the declaration.  This affects all of the new functions, I think.  Also, there's no need to forward Tom>declare them if the uses come after the definition, so probably some of those declarations can be removed as well.
Tom>Added static in definitions also.

Done

Tom>It might be nicer if read_loclist_header took a pointer to the loclist_header rather than a dwarf2_cu, and did not allocate.  See below.

Done

Nitika> +ULONGEST
Nitika> +lookup_loclist_base (struct dwarf2_cu* cu) {
Nitika> +  /* For the .dwo unit, the loclist_base points to the first offset following
Nitika> +     the header. The header consists of the following entities-
Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
Nitika> +     2. version (2 bytes)
Nitika> +     3. address size (1 byte)
Nitika> +     4. segment selector size (1 byte)
Nitika> +     5. offset entry count (4 bytes)
Nitika> +     These sizes are derived as per the DWARFv5 standard. */
Nitika> +  if (cu->dwo_unit)
Nitika> +  {
Nitika> +    if (cu->header.initial_length_size == 4)
Nitika> +      return LOCLIST_HEADER_SIZE32;
Nitika> +    return LOCLIST_HEADER_SIZE64;

Tom>Is there some way to avoid hard-coding sizes here?

I thought of using sizeof(struct loclist_header) in place of LOCLIST_HEADER_SIZE32 and sizeof(struct loclist_header) + cu->initial_length_size in place of
LOCLIST_HEADER_SIZE_64. But I am not sure if this a correct way of doing this. Some compilers append some padding at the end of structure. So the
size of structure might not be equal to the sum of size of its members. Sizeof() is also compiler dependent. So, right now I cannot think of any other way.

Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
Nitika> +   of offsets in the .debug_loclists section. */ CORE_ADDR
Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST
Nitika> +loclist_index) {
...
Nitika> +  const gdb_byte* info_ptr;

Tom> This can be declared later, when it's first initialized.
Done

Nitika> +  if (section->buffer == NULL)
Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
Nitika> +       objfile_name (objfile));

Tom>I wonder whether errors here will really do something good.  The problem is that the DWARF reader, in general, doesn't handle errors very well.
Tom>It *should* -- but it doesn't.  I don't know about this spot, but in other places, calling error will mean that reading all of the debuginfo for the entire file Tom>will be aborted.  (It can even cause worse problems, there's a bug in bugzilla about it ending a remote session.)

Tom>Maybe complaint() and then a fallback would be preferable.
Tom>Or, test the error() to make sure it's ok.

Replaced with complaint()

Nitika> +  delete cu->loclist_header;

Tom>This is created, only to be deleted in the same function.
Tom>I think it would be better to just stack-allocate this.

Done

Tom>Or, should this be cached in the CU -- that is, read once and then reused?
Tom>If so then a different approach should be used.  It wasn't clear to me how often read_loclist_index is called.

Nitika> +    case DW_FORM_loclistx:
Nitika> +    {
Nitika> +      *need_reprocess = true;
Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr,
Nitika> + &bytes_read);

Tom>Space before the first "(".
Done

Regards,
Nitika Achra

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-02-24  8:48       ` Achra, Nitika
@ 2020-03-17 14:13         ` Achra, Nitika
  2020-03-18 14:09           ` Achra, Nitika
  0 siblings, 1 reply; 7+ messages in thread
From: Achra, Nitika @ 2020-03-17 14:13 UTC (permalink / raw)
  To: gdb-patches, Tom Tromey, Ali Tamur; +Cc: George, Jini Susan

[-- Attachment #1: Type: text/plain, Size: 9718 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Hi Ali,

Just a gentle reminder to look into the failing testcase as I am not able to reproduce the bug. Attached the new patch after rebasing.

Regards,
Nitika

From: Achra, Nitika
Sent: Monday, February 24, 2020 2:19 PM
To: gdb-patches@sourceware.org; Tom Tromey <tom@tromey.com>; Ali Tamur <tamur@google.com>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.


[AMD Official Use Only - Internal Distribution Only]

Hi Tom,
Hi Ali,

Sorry for the delayed response. I was in a very poor internet connectivity area. Please look for my comments below.



From: Ali Tamur <tamur@google.com<mailto:tamur@google.com>>
Sent: Sunday, February 2, 2020 8:40 AM
To: Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>>
Cc: Tom Tromey <tom@tromey.com<mailto:tom@tromey.com>>; gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>; George, Jini Susan <JiniSusan.George@amd.com<mailto:JiniSusan.George@amd.com>>
Subject: Re: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

Ali>Hi Nikita,

Ali>Thank you for this patch. I think you have a bug. (At least the version I have seems to have a bug)

Ali>static CORE_ADDR
Ali>read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
Ali>{
Ali>...
Ali>  read_loclist_header (&header, section);   // (*)
Ali>}

Ali>I think (*) would be correct only when there is a single location list header in the .debug_loclists section. In the more general case where there are many, Ali>info_ptr should start reading the header at:
Ali>section->buffer + loclist_base - (LOCLIST_HEADER_SIZE32 or LOCLIST_HEADER_SIZE64);

Ali>Am I missing something?

Ali>My example executable is:
Ali>int main() {
Ali> return 17;
Ali>}
Ali>compiled with clang++, -gdwarf-5 and no split dwarf but also linked with libc++ library. With your code, gdb cannot read debug_info; with the correction, Ali>everything seems fine.

I am not able to reproduce this bug using the example executable. The example is working fine with me. I have used the following command-
clang++ -gdwarf-5 -O3  -stdlib=libc++  test.cpp
Could you please look into this again? Am I doing something wrong here?
I have used clang version 10.0.0.

Ali>I have another suggestion/question. Is DW_AT_location allowed to have a DW_FORM_loclistx form? In the dwarf-dump of my example, I see:
Ali>0x00032ee5: DW_TAG_formal_parameter
Ali>    DW_AT_location      (indexed (0x85c) loclist = 0x000109d5:
Ali>     [0x000000000033b5c0, 0x000000000033b5d0): DW_OP_reg5 RDI
Ali>     [0x000000000033b5d0, 0x000000000033b62b): DW_OP_reg15 R15
Ali>              [0x000000000033b631, 0x000000000033b65e): DW_OP_reg15 R15)
Ali>              DW_AT_name       ("ptr")
Ali>              DW_AT_decl_file  ("/path/to/tcmalloc.cc")
Ali>              DW_AT_decl_line  (1385)
Ali>              DW_AT_type       (0x0003d6d6 "*")

Ali> But then:
Ali> partial_die_info::read (  ...
Ali>  switch (attr.name<https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fattr.name%2F&data=02%7C01%7CNitika.Achra%40amd.com%7Cdc148d3943894e037b6a08d7a78d7268%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637162098268321041&sdata=uO7qWPYfgVGf6wlo2QbqaRQTTOof945hTOrJFCGhoJM%3D&reserved=0>) case DW_AT_location:
Ali>      ...
Ali>      else if (attr_form_is_section_offset (&attr)) {
Ali>        dwarf2_complex_location_expr_complaint ();  // ***

Ali> *** fires, I think needlessly. Could this complaint be obsolete?

As per the DWARFv5 standard DW_AT_location is allowed to have DW_FORM_loclistx. So this complaint should be obsolete now if only DW_FORM_loclistx is being used. But whether the section offsets like DW_FORM_sec_offset, DW_FORM_data4 and DW_FORM_data8 can still be emitted under DWARFv5 or not? If yes then the complaint should get fired for them but not for DW_FORM_loclistx. Am I right?

Ali>Thank you,
Ali>Ali

Regards,
Nitika


On Fri, Jan 31, 2020 at 12:55 AM Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>> wrote:
[AMD Official Use Only - Internal Distribution Only]

Hello Tom,

Thanks for the review. I have incorporated the review comments. Please have a look.

Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.

Tom>Thanks for the patch.

Tom>My comments below are primarily nits.

Nitika> Tested by running the testsuite before and after the patch and
Nitika> there is no increase in the number of test cases that fails.
Nitika> Tested with both
Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along
Nitika> with
Nitika> -gdwarf-4 as well as -gdwarf5 flags.

Tom> I assume it fixed some tests with -gdwarf-5?  Or else we'll need a new test case.

Added a new test case. This test checks if the file command passes without any error with -g-dwarf-5 and -gsplit-dwarf.
Gcc emits DW_FORM_loclistx only with -gsplit-dwarf. I can also check by printing the variable value, but right now printing
the value is throwing  the error- " DWARF ERROR: Corrupted dwarf expression"  which is not related to this patch.  I have submitted
another patch which will fix that.

Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE32 12;
Nitika> +
Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE64 20;

Tom> The ";" at the end of these defines is weird.
Removed

Nitika> +  /* Header data from the location list section. */  struct
Nitika> + loclist_header* loclist_header;

Tom> gdb style puts the "*" on the other side of the " " like

Tom> struct loclist_header *loclist_header;

Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct
Nitika> +dwarf2_cu* cu);

Tom> Here too -- there are actually several instances in the patch.

Done

Nitika> +void
Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct
Nitika> +dwarf2_section_info* section) {

Tom> New functions should have an introductory comment explaining their purpose.
Done

Tom> The "static" should be repeated here, rather than rely on the declaration.  This affects all of the new functions, I think.  Also, there's no need to forward Tom>declare them if the uses come after the definition, so probably some of those declarations can be removed as well.
Tom>Added static in definitions also.

Done

Tom>It might be nicer if read_loclist_header took a pointer to the loclist_header rather than a dwarf2_cu, and did not allocate.  See below.

Done

Nitika> +ULONGEST
Nitika> +lookup_loclist_base (struct dwarf2_cu* cu) {
Nitika> +  /* For the .dwo unit, the loclist_base points to the first offset following
Nitika> +     the header. The header consists of the following entities-
Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
Nitika> +     2. version (2 bytes)
Nitika> +     3. address size (1 byte)
Nitika> +     4. segment selector size (1 byte)
Nitika> +     5. offset entry count (4 bytes)
Nitika> +     These sizes are derived as per the DWARFv5 standard. */
Nitika> +  if (cu->dwo_unit)
Nitika> +  {
Nitika> +    if (cu->header.initial_length_size == 4)
Nitika> +      return LOCLIST_HEADER_SIZE32;
Nitika> +    return LOCLIST_HEADER_SIZE64;

Tom>Is there some way to avoid hard-coding sizes here?

I thought of using sizeof(struct loclist_header) in place of LOCLIST_HEADER_SIZE32 and sizeof(struct loclist_header) + cu->initial_length_size in place of
LOCLIST_HEADER_SIZE_64. But I am not sure if this a correct way of doing this. Some compilers append some padding at the end of structure. So the
size of structure might not be equal to the sum of size of its members. Sizeof() is also compiler dependent. So, right now I cannot think of any other way.

Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
Nitika> +   of offsets in the .debug_loclists section. */ CORE_ADDR
Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST
Nitika> +loclist_index) {
...
Nitika> +  const gdb_byte* info_ptr;

Tom> This can be declared later, when it's first initialized.
Done

Nitika> +  if (section->buffer == NULL)
Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
Nitika> +       objfile_name (objfile));

Tom>I wonder whether errors here will really do something good.  The problem is that the DWARF reader, in general, doesn't handle errors very well.
Tom>It *should* -- but it doesn't.  I don't know about this spot, but in other places, calling error will mean that reading all of the debuginfo for the entire file Tom>will be aborted.  (It can even cause worse problems, there's a bug in bugzilla about it ending a remote session.)

Tom>Maybe complaint() and then a fallback would be preferable.
Tom>Or, test the error() to make sure it's ok.

Replaced with complaint()

Nitika> +  delete cu->loclist_header;

Tom>This is created, only to be deleted in the same function.
Tom>I think it would be better to just stack-allocate this.

Done

Tom>Or, should this be cached in the CU -- that is, read once and then reused?
Tom>If so then a different approach should be used.  It wasn't clear to me how often read_loclist_index is called.

Nitika> +    case DW_FORM_loclistx:
Nitika> +    {
Nitika> +      *need_reprocess = true;
Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr,
Nitika> + &bytes_read);

Tom>Space before the first "(".
Done

Regards,
Nitika Achra

[-- Attachment #2: 0001-Support-for-DW_AT_loclists_base-and-DW_FORM_loclistx.patch --]
[-- Type: application/octet-stream, Size: 12085 bytes --]

From f07479ea078cbb29ad14efed42bf39e5e8503800 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Tue, 17 Mar 2020 16:25:41 +0530
Subject: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

This patch handles DW_AT_loclists_base and DW_FORM_loclistx.
DW_AT_loclists_base is a new attribute added in DWARFv5 which
points to the beginning of the offset table of .debug_loclist
section. Reference to the location list (DW_FORM_loclistx) is
interpreted relative to this base. DW_FORM_loclistx is a new
form added in DWARFv5 which is used to access location list.

Tested by running the testsuite before and after the patch and there
is no increase in the number of test cases that fails. Tested with both
-gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with
-gdwarf-4 as well as -gdwarf5 flags. Used gcc-9.4 for testing.

gdb/ChangeLog:

   *dwarf2read.c (cu_debug_loc_section): Added the declaration for the function.
   (read_loclist_index): New function declaration.
   (lookup_loclist_base): New function declaration.
   (read_loclist_header): New function declaration
   (dwarf2_cu): Added loclist_base and loclist_header field.
   (dwarf2_locate_dwo_sections): Handle .debug_loclist.dwo section.
   (read_full_die_1): Read the value of DW_AT_loclists_base.
   (read_attribute_reprocess): Handle DW_FORM_loclistx.
   (read_attribute_value): Handle DW_FORM_loclistx.
   (skip_one_die): Handle DW_FORM_loclistx.
   (attr_form_is_section_offset): Handle DW_FORM_loclistx.
   (read_loclist_index): Function definition.
   (lookup_loclist_base): Function definition.
   (read_loclist_header): Function definition.
   (loclist_header): New structure declaration.

gdb/testsuite/ChangeLog:

   *gdb.dwarf2/dw5-form-loclistx.exp: New file.
   *gdb.dwarf2/dw5-form-loclistx.c: New file.
---
 gdb/dwarf2/read.c                             | 127 ++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c  |  16 +++
 .../gdb.dwarf2/dw5-form-loclistx.exp          |  42 ++++++
 3 files changed, 185 insertions(+)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 88a60c1c73..5265ef208d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -111,6 +111,12 @@ static int dwarf2_loclist_index;
 static int dwarf2_locexpr_block_index;
 static int dwarf2_loclist_block_index;
 
+/* Size of .debug_loclist section header for 32-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE32 12
+
+/* Size of .debug_loclist section header for 64-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE64 20
+
 /* An index into a (C++) symbol name component in a symbol name as
    recorded in the mapped_index's symbol table.  For each C++ symbol
    in the symbol table, we record one entry for the start of each
@@ -343,6 +349,30 @@ dwop_section_names =
 
 /* local data types */
 
+/* The location list section (.debug_loclist) begins with a header,
+   which contains the following information. */
+struct loclist_header
+{
+  /* A 4-byte or 12-byte length containing the length of the
+  set of entries for this compilation unit, not including the
+  length field itself. */
+  unsigned int length;
+
+  /* A 2-byte version identifier. */
+  short version;
+
+  /* A 1-byte unsigned integer containing the size in bytes of an address on
+     the target system. */
+  unsigned char addr_size;
+
+  /* A 1-byte unsigned integer containing the size in bytes of a segment selector
+     on the target system. */
+  unsigned char segment_collector_size;
+
+  /* A 4-byte count of the number of offsets that follow the header. */
+  unsigned int offset_entry_count;
+};
+
 /* Type used for delaying computation of method physnames.
    See comments for compute_delayed_physnames.  */
 struct delayed_method_info
@@ -493,6 +523,9 @@ struct dwarf2_cu
      whether the DW_AT_ranges attribute came from the skeleton or DWO.  */
   ULONGEST ranges_base = 0;
 
+  /* The DW_AT_loclists_base attribute if present. */
+  gdb::optional<ULONGEST> loclist_base;
+
   /* When reading debug info generated by older versions of rustc, we
      have to rewrite some union types to be struct types with a
      variant part.  This rewriting must be done after the CU is fully
@@ -1357,6 +1390,9 @@ static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
 static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
 			       struct dwarf2_cu *, dwarf2_psymtab *);
 
+/* Return the .debug_loclist section to use for cu. */
+static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
+
 /* How dwarf2_get_pc_bounds constructed its *LOWPC and *HIGHPC return
    values.  Keep the items ordered with increasing constraints compliance.  */
 enum pc_bounds_kind
@@ -8678,6 +8714,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_GNU_addr_index:
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_rnglistx:
+	case DW_FORM_loclistx:
 	  info_ptr = safe_skip_leb128 (info_ptr, buffer_end);
 	  break;
 	case DW_FORM_indirect:
@@ -12135,6 +12172,11 @@ dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
       dwo_sections->loc.s.section = sectp;
       dwo_sections->loc.size = bfd_section_size (sectp);
     }
+  else if (section_is_p (sectp->name, &names->loclists_dwo))
+    {
+      dwo_sections->loclists.s.section = sectp;
+      dwo_sections->loclists.size = bfd_section_size (sectp);
+    }
   else if (section_is_p (sectp->name, &names->macinfo_dwo))
     {
       dwo_sections->macinfo.s.section = sectp;
@@ -17608,6 +17650,9 @@ read_full_die_1 (const struct die_reader_specs *reader,
   struct attribute *attr = dwarf2_attr_no_follow (die, DW_AT_str_offsets_base);
   if (attr != nullptr)
     cu->str_offsets_base = DW_UNSND (attr);
+  attr = dwarf2_attr_no_follow (die, DW_AT_loclists_base);
+  if (attr)
+    cu->loclist_base = DW_UNSND (attr);
 
   auto maybe_addr_base = lookup_addr_base(die);
   if (maybe_addr_base.has_value ())
@@ -18411,6 +18456,78 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
   fixup_called = 1;
 }
 
+static void
+read_loclist_header (struct loclist_header *header, struct dwarf2_section_info *section)
+{
+  unsigned int bytes_read;
+  bfd *abfd = section->get_bfd_owner ();
+  const gdb_byte *info_ptr = section->buffer;
+  header->length = read_initial_length (abfd, info_ptr, &bytes_read);
+  info_ptr += bytes_read;
+  header->version = read_2_bytes (abfd, info_ptr);
+  info_ptr += 2;
+  header->addr_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->segment_collector_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->offset_entry_count = read_4_bytes (abfd, info_ptr);
+}
+
+
+static ULONGEST
+lookup_loclist_base (struct dwarf2_cu *cu)
+{
+  /* For the .dwo unit, the loclist_base points to the first offset following
+     the header. The header consists of the following entities-
+     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
+     2. version (2 bytes)
+     3. address size (1 byte)
+     4. segment selector size (1 byte)
+     5. offset entry count (4 bytes)
+     These sizes are derived as per the DWARFv5 standard. */
+  if (cu->dwo_unit)
+  {
+    if (cu->header.initial_length_size == 4)
+      return LOCLIST_HEADER_SIZE32;
+    return LOCLIST_HEADER_SIZE64;
+  }
+  return *cu->loclist_base;
+}
+
+/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
+   of offsets in the .debug_loclists section. */
+static CORE_ADDR
+read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
+{
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+	= cu->per_cu->dwarf2_per_objfile;
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
+  bfd *abfd = objfile->obfd;
+  ULONGEST loclist_base = lookup_loclist_base (cu);
+  struct dwarf2_section_info *section = cu_debug_loc_section (cu);
+  section->read (objfile);
+  if (section->buffer == NULL)
+    complaint(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  struct loclist_header header;
+  read_loclist_header (&header, section);
+  if (loclist_index >= header.offset_entry_count)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist offset array [in module %s]"),
+	objfile_name (objfile));
+  if (loclist_base + loclist_index * cu->header.offset_size
+	>= section->size)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  const gdb_byte *info_ptr = (section->buffer + loclist_base +
+	loclist_index * cu->header.offset_size);
+  if (cu->header.offset_size == 4)
+    return bfd_get_32 (abfd, info_ptr) + loclist_base;
+  else
+    return bfd_get_64 (abfd, info_ptr) + loclist_base;
+}
+
 /* Process the attributes that had to be skipped in the first round. These
    attributes are the ones that need str_offsets_base or addr_base attributes.
    They could not have been processed in the first round, because at the time
@@ -18425,6 +18542,9 @@ void read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_GNU_addr_index:
         DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
         break;
+      case DW_FORM_loclistx:
+	 DW_UNSND (attr) = read_loclist_index (cu, DW_UNSND (attr));
+	 break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
       case DW_FORM_strx2:
@@ -18529,6 +18649,13 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
       break;
+    case DW_FORM_loclistx:
+    {
+      *need_reprocess = true;
+      DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      info_ptr += bytes_read;
+    }
+      break;
     case DW_FORM_string:
       DW_STRING (attr) = read_direct_string (abfd, info_ptr, &bytes_read);
       DW_STRING_IS_CANONICAL (attr) = 0;
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
new file mode 100644
index 0000000000..a2a14583be
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
@@ -0,0 +1,16 @@
+
+
+int foo(int a, int b)
+{
+  a = a+b;
+  b = a-b;
+  a = 2*b;
+  return a+b;
+}
+
+int main()
+{
+  int result,a,b;
+  result = foo(a,b);
+  return result;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
new file mode 100644
index 0000000000..15d9ac13e7
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
@@ -0,0 +1,42 @@
+# Copyright 2012-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile .c
+
+
+# We can't use prepare_for_testing here because we need to check the
+# 'file' command's output.
+
+if { [build_executable ${testfile}.exp ${testfile} ${srcfile}\
+   {additional_flags=-gdwarf-5 additional_flags=-gsplit-dwarf additional_flags=-O2}] == -1 } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Check that file command gives no error.
+gdb_test "file $binfile" \
+    "Reading symbols from $binfile\.\.\."
+
+
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.
  2020-03-17 14:13         ` Achra, Nitika
@ 2020-03-18 14:09           ` Achra, Nitika
  0 siblings, 0 replies; 7+ messages in thread
From: Achra, Nitika @ 2020-03-18 14:09 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 10332 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Updating the patch. Missed updating the filenames in commit message and the changes in dwarf2/attribute.c after rebasing.

From: Achra, Nitika
Sent: Tuesday, March 17, 2020 7:43 PM
To: 'gdb-patches@sourceware.org' <gdb-patches@sourceware.org>; 'Tom Tromey' <tom@tromey.com>; 'Ali Tamur' <tamur@google.com>
Cc: George, Jini Susan <JiniSusan.George@amd.com>
Subject: RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.


[AMD Official Use Only - Internal Distribution Only]

Hi Ali,

Just a gentle reminder to look into the failing testcase as I am not able to reproduce the bug. Attached the new patch after rebasing.

Regards,
Nitika

From: Achra, Nitika
Sent: Monday, February 24, 2020 2:19 PM
To: gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>; Tom Tromey <tom@tromey.com<mailto:tom@tromey.com>>; Ali Tamur <tamur@google.com<mailto:tamur@google.com>>
Cc: George, Jini Susan <JiniSusan.George@amd.com<mailto:JiniSusan.George@amd.com>>
Subject: RE: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.


[AMD Official Use Only - Internal Distribution Only]

Hi Tom,
Hi Ali,

Sorry for the delayed response. I was in a very poor internet connectivity area. Please look for my comments below.



From: Ali Tamur <tamur@google.com<mailto:tamur@google.com>>
Sent: Sunday, February 2, 2020 8:40 AM
To: Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>>
Cc: Tom Tromey <tom@tromey.com<mailto:tom@tromey.com>>; gdb-patches@sourceware.org<mailto:gdb-patches@sourceware.org>; George, Jini Susan <JiniSusan.George@amd.com<mailto:JiniSusan.George@amd.com>>
Subject: Re: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

Ali>Hi Nikita,

Ali>Thank you for this patch. I think you have a bug. (At least the version I have seems to have a bug)

Ali>static CORE_ADDR
Ali>read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
Ali>{
Ali>...
Ali>  read_loclist_header (&header, section);   // (*)
Ali>}

Ali>I think (*) would be correct only when there is a single location list header in the .debug_loclists section. In the more general case where there are many, Ali>info_ptr should start reading the header at:
Ali>section->buffer + loclist_base - (LOCLIST_HEADER_SIZE32 or LOCLIST_HEADER_SIZE64);

Ali>Am I missing something?

Ali>My example executable is:
Ali>int main() {
Ali> return 17;
Ali>}
Ali>compiled with clang++, -gdwarf-5 and no split dwarf but also linked with libc++ library. With your code, gdb cannot read debug_info; with the correction, Ali>everything seems fine.

I am not able to reproduce this bug using the example executable. The example is working fine with me. I have used the following command-
clang++ -gdwarf-5 -O3  -stdlib=libc++  test.cpp
Could you please look into this again? Am I doing something wrong here?
I have used clang version 10.0.0.

Ali>I have another suggestion/question. Is DW_AT_location allowed to have a DW_FORM_loclistx form? In the dwarf-dump of my example, I see:
Ali>0x00032ee5: DW_TAG_formal_parameter
Ali>    DW_AT_location      (indexed (0x85c) loclist = 0x000109d5:
Ali>     [0x000000000033b5c0, 0x000000000033b5d0): DW_OP_reg5 RDI
Ali>     [0x000000000033b5d0, 0x000000000033b62b): DW_OP_reg15 R15
Ali>              [0x000000000033b631, 0x000000000033b65e): DW_OP_reg15 R15)
Ali>              DW_AT_name       ("ptr")
Ali>              DW_AT_decl_file  ("/path/to/tcmalloc.cc")
Ali>              DW_AT_decl_line  (1385)
Ali>              DW_AT_type       (0x0003d6d6 "*")

Ali> But then:
Ali> partial_die_info::read (  ...
Ali>  switch (attr.name<https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fattr.name%2F&data=02%7C01%7CNitika.Achra%40amd.com%7Cdc148d3943894e037b6a08d7a78d7268%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637162098268321041&sdata=uO7qWPYfgVGf6wlo2QbqaRQTTOof945hTOrJFCGhoJM%3D&reserved=0>) case DW_AT_location:
Ali>      ...
Ali>      else if (attr_form_is_section_offset (&attr)) {
Ali>        dwarf2_complex_location_expr_complaint ();  // ***

Ali> *** fires, I think needlessly. Could this complaint be obsolete?

As per the DWARFv5 standard DW_AT_location is allowed to have DW_FORM_loclistx. So this complaint should be obsolete now if only DW_FORM_loclistx is being used. But whether the section offsets like DW_FORM_sec_offset, DW_FORM_data4 and DW_FORM_data8 can still be emitted under DWARFv5 or not? If yes then the complaint should get fired for them but not for DW_FORM_loclistx. Am I right?

Ali>Thank you,
Ali>Ali

Regards,
Nitika


On Fri, Jan 31, 2020 at 12:55 AM Achra, Nitika <Nitika.Achra@amd.com<mailto:Nitika.Achra@amd.com>> wrote:
[AMD Official Use Only - Internal Distribution Only]

Hello Tom,

Thanks for the review. I have incorporated the review comments. Please have a look.

Nitika> *Support for DW_AT_loclists_base and DW_FORM_loclistx.

Tom>Thanks for the patch.

Tom>My comments below are primarily nits.

Nitika> Tested by running the testsuite before and after the patch and
Nitika> there is no increase in the number of test cases that fails.
Nitika> Tested with both
Nitika> -gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along
Nitika> with
Nitika> -gdwarf-4 as well as -gdwarf5 flags.

Tom> I assume it fixed some tests with -gdwarf-5?  Or else we'll need a new test case.

Added a new test case. This test checks if the file command passes without any error with -g-dwarf-5 and -gsplit-dwarf.
Gcc emits DW_FORM_loclistx only with -gsplit-dwarf. I can also check by printing the variable value, but right now printing
the value is throwing  the error- " DWARF ERROR: Corrupted dwarf expression"  which is not related to this patch.  I have submitted
another patch which will fix that.

Nitika> +/* Size of .debug_loclist section header for 32-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE32 12;
Nitika> +
Nitika> +/* Size of .debug_loclist section header for 64-bit DWARF
Nitika> +format. */ #define LOCLIST_HEADER_SIZE64 20;

Tom> The ";" at the end of these defines is weird.
Removed

Nitika> +  /* Header data from the location list section. */  struct
Nitika> + loclist_header* loclist_header;

Tom> gdb style puts the "*" on the other side of the " " like

Tom> struct loclist_header *loclist_header;

Nitika> +static struct dwarf2_section_info* cu_debug_loc_section (struct
Nitika> +dwarf2_cu* cu);

Tom> Here too -- there are actually several instances in the patch.

Done

Nitika> +void
Nitika> +read_loclist_header (struct dwarf2_cu* cu, struct
Nitika> +dwarf2_section_info* section) {

Tom> New functions should have an introductory comment explaining their purpose.
Done

Tom> The "static" should be repeated here, rather than rely on the declaration.  This affects all of the new functions, I think.  Also, there's no need to forward Tom>declare them if the uses come after the definition, so probably some of those declarations can be removed as well.
Tom>Added static in definitions also.

Done

Tom>It might be nicer if read_loclist_header took a pointer to the loclist_header rather than a dwarf2_cu, and did not allocate.  See below.

Done

Nitika> +ULONGEST
Nitika> +lookup_loclist_base (struct dwarf2_cu* cu) {
Nitika> +  /* For the .dwo unit, the loclist_base points to the first offset following
Nitika> +     the header. The header consists of the following entities-
Nitika> +     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
Nitika> +     2. version (2 bytes)
Nitika> +     3. address size (1 byte)
Nitika> +     4. segment selector size (1 byte)
Nitika> +     5. offset entry count (4 bytes)
Nitika> +     These sizes are derived as per the DWARFv5 standard. */
Nitika> +  if (cu->dwo_unit)
Nitika> +  {
Nitika> +    if (cu->header.initial_length_size == 4)
Nitika> +      return LOCLIST_HEADER_SIZE32;
Nitika> +    return LOCLIST_HEADER_SIZE64;

Tom>Is there some way to avoid hard-coding sizes here?

I thought of using sizeof(struct loclist_header) in place of LOCLIST_HEADER_SIZE32 and sizeof(struct loclist_header) + cu->initial_length_size in place of
LOCLIST_HEADER_SIZE_64. But I am not sure if this a correct way of doing this. Some compilers append some padding at the end of structure. So the
size of structure might not be equal to the sum of size of its members. Sizeof() is also compiler dependent. So, right now I cannot think of any other way.

Nitika> +/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
Nitika> +   of offsets in the .debug_loclists section. */ CORE_ADDR
Nitika> +read_loclist_index (struct dwarf2_cu* cu, ULONGEST
Nitika> +loclist_index) {
...
Nitika> +  const gdb_byte* info_ptr;

Tom> This can be declared later, when it's first initialized.
Done

Nitika> +  if (section->buffer == NULL)
Nitika> +    error(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
Nitika> +       objfile_name (objfile));

Tom>I wonder whether errors here will really do something good.  The problem is that the DWARF reader, in general, doesn't handle errors very well.
Tom>It *should* -- but it doesn't.  I don't know about this spot, but in other places, calling error will mean that reading all of the debuginfo for the entire file Tom>will be aborted.  (It can even cause worse problems, there's a bug in bugzilla about it ending a remote session.)

Tom>Maybe complaint() and then a fallback would be preferable.
Tom>Or, test the error() to make sure it's ok.

Replaced with complaint()

Nitika> +  delete cu->loclist_header;

Tom>This is created, only to be deleted in the same function.
Tom>I think it would be better to just stack-allocate this.

Done

Tom>Or, should this be cached in the CU -- that is, read once and then reused?
Tom>If so then a different approach should be used.  It wasn't clear to me how often read_loclist_index is called.

Nitika> +    case DW_FORM_loclistx:
Nitika> +    {
Nitika> +      *need_reprocess = true;
Nitika> +      DW_UNSND(attr) = read_unsigned_leb128 (abfd, info_ptr,
Nitika> + &bytes_read);

Tom>Space before the first "(".
Done

Regards,
Nitika Achra

[-- Attachment #2: 0001-Support-for-DW_AT_loclists_base-and-DW_FORM_loclistx.patch --]
[-- Type: application/octet-stream, Size: 12452 bytes --]

From ea865f5bc43024bbbbe16d49ee9a63d850477c5f Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Tue, 17 Mar 2020 16:25:41 +0530
Subject: [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx.

This patch handles DW_AT_loclists_base and DW_FORM_loclistx.
DW_AT_loclists_base is a new attribute added in DWARFv5 which
points to the beginning of the offset table of .debug_loclist
section. Reference to the location list (DW_FORM_loclistx) is
interpreted relative to this base. DW_FORM_loclistx is a new
form added in DWARFv5 which is used to access location list.

Tested by running the testsuite before and after the patch and there
is no increase in the number of test cases that fails. Tested with both
-gdwarf-4 and -gdwarf-5 flags. Also tested -gslit-dwarf along with
-gdwarf-4 as well as -gdwarf5 flags. Used clang 10.0.0 for testing.

gdb/dwarf2/ChangeLog:

   *read.c (cu_debug_loc_section): Added the declaration for the function.
   (read_loclist_index): New function declaration.
   (lookup_loclist_base): New function declaration.
   (read_loclist_header): New function declaration
   (dwarf2_cu): Added loclist_base and loclist_header field.
   (dwarf2_locate_dwo_sections): Handle .debug_loclist.dwo section.
   (read_full_die_1): Read the value of DW_AT_loclists_base.
   (read_attribute_reprocess): Handle DW_FORM_loclistx.
   (read_attribute_value): Handle DW_FORM_loclistx.
   (skip_one_die): Handle DW_FORM_loclistx.
   (loclist_header): New structure declaration.
   *attribute.c (form_is_section_offset): Handle DW_FORM_loclistx.

gdb/testsuite/ChangeLog:

   *gdb.dwarf2/dw5-form-loclistx.exp: New file.
   *gdb.dwarf2/dw5-form-loclistx.c: New file.
---
 gdb/dwarf2/attribute.c                        |   3 +-
 gdb/dwarf2/read.c                             | 127 ++++++++++++++++++
 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c  |  16 +++
 .../gdb.dwarf2/dw5-form-loclistx.exp          |  42 ++++++
 4 files changed, 187 insertions(+), 1 deletion(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index 6efff3e2c0..ec78db95fe 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -76,7 +76,8 @@ attribute::form_is_section_offset () const
 {
   return (form == DW_FORM_data4
           || form == DW_FORM_data8
-	  || form == DW_FORM_sec_offset);
+	  || form == DW_FORM_sec_offset
+	  || form == DW_FORM_loclistx);
 }
 
 /* See attribute.h.  */
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 88a60c1c73..5265ef208d 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -111,6 +111,12 @@ static int dwarf2_loclist_index;
 static int dwarf2_locexpr_block_index;
 static int dwarf2_loclist_block_index;
 
+/* Size of .debug_loclist section header for 32-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE32 12
+
+/* Size of .debug_loclist section header for 64-bit DWARF format. */
+#define LOCLIST_HEADER_SIZE64 20
+
 /* An index into a (C++) symbol name component in a symbol name as
    recorded in the mapped_index's symbol table.  For each C++ symbol
    in the symbol table, we record one entry for the start of each
@@ -343,6 +349,30 @@ dwop_section_names =
 
 /* local data types */
 
+/* The location list section (.debug_loclist) begins with a header,
+   which contains the following information. */
+struct loclist_header
+{
+  /* A 4-byte or 12-byte length containing the length of the
+  set of entries for this compilation unit, not including the
+  length field itself. */
+  unsigned int length;
+
+  /* A 2-byte version identifier. */
+  short version;
+
+  /* A 1-byte unsigned integer containing the size in bytes of an address on
+     the target system. */
+  unsigned char addr_size;
+
+  /* A 1-byte unsigned integer containing the size in bytes of a segment selector
+     on the target system. */
+  unsigned char segment_collector_size;
+
+  /* A 4-byte count of the number of offsets that follow the header. */
+  unsigned int offset_entry_count;
+};
+
 /* Type used for delaying computation of method physnames.
    See comments for compute_delayed_physnames.  */
 struct delayed_method_info
@@ -493,6 +523,9 @@ struct dwarf2_cu
      whether the DW_AT_ranges attribute came from the skeleton or DWO.  */
   ULONGEST ranges_base = 0;
 
+  /* The DW_AT_loclists_base attribute if present. */
+  gdb::optional<ULONGEST> loclist_base;
+
   /* When reading debug info generated by older versions of rustc, we
      have to rewrite some union types to be struct types with a
      variant part.  This rewriting must be done after the CU is fully
@@ -1357,6 +1390,9 @@ static void read_variable (struct die_info *die, struct dwarf2_cu *cu);
 static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
 			       struct dwarf2_cu *, dwarf2_psymtab *);
 
+/* Return the .debug_loclist section to use for cu. */
+static struct dwarf2_section_info *cu_debug_loc_section (struct dwarf2_cu *cu);
+
 /* How dwarf2_get_pc_bounds constructed its *LOWPC and *HIGHPC return
    values.  Keep the items ordered with increasing constraints compliance.  */
 enum pc_bounds_kind
@@ -8678,6 +8714,7 @@ skip_one_die (const struct die_reader_specs *reader, const gdb_byte *info_ptr,
 	case DW_FORM_GNU_addr_index:
 	case DW_FORM_GNU_str_index:
 	case DW_FORM_rnglistx:
+	case DW_FORM_loclistx:
 	  info_ptr = safe_skip_leb128 (info_ptr, buffer_end);
 	  break;
 	case DW_FORM_indirect:
@@ -12135,6 +12172,11 @@ dwarf2_locate_dwo_sections (bfd *abfd, asection *sectp, void *dwo_sections_ptr)
       dwo_sections->loc.s.section = sectp;
       dwo_sections->loc.size = bfd_section_size (sectp);
     }
+  else if (section_is_p (sectp->name, &names->loclists_dwo))
+    {
+      dwo_sections->loclists.s.section = sectp;
+      dwo_sections->loclists.size = bfd_section_size (sectp);
+    }
   else if (section_is_p (sectp->name, &names->macinfo_dwo))
     {
       dwo_sections->macinfo.s.section = sectp;
@@ -17608,6 +17650,9 @@ read_full_die_1 (const struct die_reader_specs *reader,
   struct attribute *attr = dwarf2_attr_no_follow (die, DW_AT_str_offsets_base);
   if (attr != nullptr)
     cu->str_offsets_base = DW_UNSND (attr);
+  attr = dwarf2_attr_no_follow (die, DW_AT_loclists_base);
+  if (attr)
+    cu->loclist_base = DW_UNSND (attr);
 
   auto maybe_addr_base = lookup_addr_base(die);
   if (maybe_addr_base.has_value ())
@@ -18411,6 +18456,78 @@ partial_die_info::fixup (struct dwarf2_cu *cu)
   fixup_called = 1;
 }
 
+static void
+read_loclist_header (struct loclist_header *header, struct dwarf2_section_info *section)
+{
+  unsigned int bytes_read;
+  bfd *abfd = section->get_bfd_owner ();
+  const gdb_byte *info_ptr = section->buffer;
+  header->length = read_initial_length (abfd, info_ptr, &bytes_read);
+  info_ptr += bytes_read;
+  header->version = read_2_bytes (abfd, info_ptr);
+  info_ptr += 2;
+  header->addr_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->segment_collector_size = read_1_byte (abfd, info_ptr);
+  info_ptr += 1;
+  header->offset_entry_count = read_4_bytes (abfd, info_ptr);
+}
+
+
+static ULONGEST
+lookup_loclist_base (struct dwarf2_cu *cu)
+{
+  /* For the .dwo unit, the loclist_base points to the first offset following
+     the header. The header consists of the following entities-
+     1. Unit Length (4 bytes for 32 bit DWARF format, and 12 bytes for the 64 bit format)
+     2. version (2 bytes)
+     3. address size (1 byte)
+     4. segment selector size (1 byte)
+     5. offset entry count (4 bytes)
+     These sizes are derived as per the DWARFv5 standard. */
+  if (cu->dwo_unit)
+  {
+    if (cu->header.initial_length_size == 4)
+      return LOCLIST_HEADER_SIZE32;
+    return LOCLIST_HEADER_SIZE64;
+  }
+  return *cu->loclist_base;
+}
+
+/* Given a DW_FORM_loclistx value loclist_index, fetch the offset from the array
+   of offsets in the .debug_loclists section. */
+static CORE_ADDR
+read_loclist_index (struct dwarf2_cu *cu, ULONGEST loclist_index)
+{
+  struct dwarf2_per_objfile *dwarf2_per_objfile
+	= cu->per_cu->dwarf2_per_objfile;
+  struct objfile *objfile = dwarf2_per_objfile->objfile;
+  bfd *abfd = objfile->obfd;
+  ULONGEST loclist_base = lookup_loclist_base (cu);
+  struct dwarf2_section_info *section = cu_debug_loc_section (cu);
+  section->read (objfile);
+  if (section->buffer == NULL)
+    complaint(_("DW_FORM_loclistx used without .debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  struct loclist_header header;
+  read_loclist_header (&header, section);
+  if (loclist_index >= header.offset_entry_count)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist offset array [in module %s]"),
+	objfile_name (objfile));
+  if (loclist_base + loclist_index * cu->header.offset_size
+	>= section->size)
+    complaint(_("DW_FORM_loclistx pointing outside of "
+	".debug_loclist section [in module %s]"),
+	objfile_name (objfile));
+  const gdb_byte *info_ptr = (section->buffer + loclist_base +
+	loclist_index * cu->header.offset_size);
+  if (cu->header.offset_size == 4)
+    return bfd_get_32 (abfd, info_ptr) + loclist_base;
+  else
+    return bfd_get_64 (abfd, info_ptr) + loclist_base;
+}
+
 /* Process the attributes that had to be skipped in the first round. These
    attributes are the ones that need str_offsets_base or addr_base attributes.
    They could not have been processed in the first round, because at the time
@@ -18425,6 +18542,9 @@ void read_attribute_reprocess (const struct die_reader_specs *reader,
       case DW_FORM_GNU_addr_index:
         DW_ADDR (attr) = read_addr_index (cu, DW_UNSND (attr));
         break;
+      case DW_FORM_loclistx:
+	 DW_UNSND (attr) = read_loclist_index (cu, DW_UNSND (attr));
+	 break;
       case DW_FORM_strx:
       case DW_FORM_strx1:
       case DW_FORM_strx2:
@@ -18529,6 +18649,13 @@ read_attribute_value (const struct die_reader_specs *reader,
       DW_UNSND (attr) = cu->header.read_offset (abfd, info_ptr, &bytes_read);
       info_ptr += bytes_read;
       break;
+    case DW_FORM_loclistx:
+    {
+      *need_reprocess = true;
+      DW_UNSND (attr) = read_unsigned_leb128 (abfd, info_ptr, &bytes_read);
+      info_ptr += bytes_read;
+    }
+      break;
     case DW_FORM_string:
       DW_STRING (attr) = read_direct_string (abfd, info_ptr, &bytes_read);
       DW_STRING_IS_CANONICAL (attr) = 0;
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
new file mode 100644
index 0000000000..a2a14583be
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.c
@@ -0,0 +1,16 @@
+
+
+int foo(int a, int b)
+{
+  a = a+b;
+  b = a-b;
+  a = 2*b;
+  return a+b;
+}
+
+int main()
+{
+  int result,a,b;
+  result = foo(a,b);
+  return result;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
new file mode 100644
index 0000000000..15d9ac13e7
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw5-form-loclistx.exp
@@ -0,0 +1,42 @@
+# Copyright 2012-2020 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+if {![dwarf2_support]} {
+    return 0  
+}
+
+standard_testfile .c
+
+
+# We can't use prepare_for_testing here because we need to check the
+# 'file' command's output.
+
+if { [build_executable ${testfile}.exp ${testfile} ${srcfile}\
+   {additional_flags=-gdwarf-5 additional_flags=-gsplit-dwarf additional_flags=-O2}] == -1 } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+
+# Check that file command gives no error.
+gdb_test "file $binfile" \
+    "Reading symbols from $binfile\.\.\."
+
+
-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-03-18 14:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 10:36 [PATCH] Support for DW_AT_loclists_base and DW_FORM_loclistx Achra, Nitika
2020-01-23 22:34 ` Tom Tromey
2020-01-31  8:59   ` Achra, Nitika
2020-02-02  3:10     ` Ali Tamur via gdb-patches
2020-02-24  8:48       ` Achra, Nitika
2020-03-17 14:13         ` Achra, Nitika
2020-03-18 14:09           ` Achra, Nitika

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).