RE: [PATCH] Support for DWARF5 location lists entries

["Achra, Nitika" <Nitika dot Achra at amd dot com>]

[AMD Official Use Only - Internal Distribution Only]

Hi Simon,

Thanks for the review. I have reformatted it as per the GNU style. Please have a look at it.



*Handle DW_LLE_base_addressx, DW_LLE_startx_length and DW_LLE_start_length.

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:
        *gdb/dwarf2loc.c (decode_debug_loclists_addresses): Handle DW_LLE_base_addressx, DW_LLE_startx_length, DW_LLE_start_length.

This is an effort to support DWARF5 in gdb.


diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 99cac03a54..dd99ea35c0 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -173,6 +173,41 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,

   switch (*loc_ptr++)
     {
+    case DW_LLE_base_addressx:
+      *low = 0;
+      loc_ptr = gdb_read_uleb128 (loc_ptr, buf_end, &u64);
+      if (loc_ptr == NULL)
+       return DEBUG_LOC_BUFFER_OVERFLOW;
+      *high = dwarf2_read_addr_index (per_cu, u64);
+      *new_ptr = loc_ptr;
+      return DEBUG_LOC_BASE_ADDRESS;
+    case DW_LLE_startx_length:
+      loc_ptr = gdb_read_uleb128 (loc_ptr, buf_end, &u64);
+      if (loc_ptr == NULL)
+       return DEBUG_LOC_BUFFER_OVERFLOW;
+      *low = dwarf2_read_addr_index (per_cu, u64);
+      *high = *low;
+      loc_ptr = gdb_read_uleb128 (loc_ptr, buf_end, &u64);
+      if (loc_ptr == NULL)
+       return DEBUG_LOC_BUFFER_OVERFLOW;
+      *high += u64;
+      *new_ptr = loc_ptr;
+      return DEBUG_LOC_START_LENGTH;
+    case DW_LLE_start_length:
+      if (buf_end - loc_ptr < addr_size)
+       return DEBUG_LOC_BUFFER_OVERFLOW;
+      if (signed_addr_p)
+       *low = extract_signed_integer (loc_ptr, addr_size, byte_order);
+      else
+       *low = extract_unsigned_integer (loc_ptr, addr_size, byte_order);
+      loc_ptr += addr_size;
+      *high = *low;
+      loc_ptr = gdb_read_uleb128 (loc_ptr, buf_end, &u64);
+      if (loc_ptr == NULL)
+       return DEBUG_LOC_BUFFER_OVERFLOW;
+      *high += u64;
+      *new_ptr = loc_ptr;
+      return DEBUG_LOC_START_LENGTH;
     case DW_LLE_end_of_list:
       *new_ptr = loc_ptr;
       return DEBUG_LOC_END_OF_LIST;
@@ -197,6 +232,9 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
       *high = u64;
       *new_ptr = loc_ptr;
       return DEBUG_LOC_START_END;
+    case DW_LLE_startx_endx:
+    case DW_LLE_start_end:
+    case DW_LLE_default_location:
     default:
       return DEBUG_LOC_INVALID_ENTRY;
     }


Regards,
Nitika

-----Original Message-----
From: Simon Marchi <simark@simark.ca> 
Sent: Wednesday, January 1, 2020 12:03 AM
To: Achra, Nitika <Nitika.Achra@amd.com>; gdb-patches@sourceware.org
Cc: George, Jini Susan <JiniSusan.George@amd.com>; Ali Tamur <tamur@google.com>
Subject: Re: [PATCH] Support for DWARF5 location lists entries

[CAUTION: External Email]

On 2019-12-31 4:41 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> * Handle DW_LLE_base_addressx, DW_LLE_startx_length and DW_LLE_start_length.
>
>
>
> 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.
>
>
>
>
> gdb/ChangeLog:
>         *gdb/dwarf2loc.c (decode_debug_loclists_addresses): Handle DW_LLE_base_addressx, DW_LLE_startx_length, DW_LLE_start_length.

Hi Nitika,

Thanks for the patch.  I don't have time to do an in-depth review right now, but I just wanted to point out the common formatting quirks of the GNU style, so you can apply it to this patch.

In terms of indentation, we use two columns for each indent.  However, leading groups of 8 spaces get converted to a tab.  So instead of 14 spaces, you would have one tab followed by 6 spaces to reach the desired column.  Most editors have a way to be set up like this.  Unfortunately, VSCode can't.

We use a space before opening parenthesis in function declaration/definitions/calls.
So:

  loc_ptr·=·gdb_read_uleb128(loc_ptr,·buf_end,·&u64);

would become

  loc_ptr·=·gdb_read_uleb128 (loc_ptr,·buf_end,·&u64);

This:

      if·(loc_ptr·==·NULL)
               return·DEBUG_LOC_BUFFER_OVERFLOW;

should become:

      if·(loc_ptr·==·NULL)
        return·DEBUG_LOC_BUFFER_OVERFLOW;

Simon