[PATCH] Support for DWARF5 location lists entries

[PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

[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.



diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 976d453d3c..0de956ab66 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;
     }

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

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

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4632 bytes --]

[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
\x16º&Öéj×!zÊÞ¶êç×­»ó©b²Ö«r\x18\x1dn–­r\x17¬

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 7665 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Hi Simon,

Sorry. I sent the wrong patch by mistake. Updating the patch.

*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..e0940689ee 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: Achra, Nitika 
Sent: Monday, January 6, 2020 12:52 PM
To: Simon Marchi <simark@simark.ca>; 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

[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
\x16º&Öéj×!zÊÞ¶êç×­»óÙb²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-06 5:14 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hi Simon,
> 
> Sorry. I sent the wrong patch by mistake. Updating the patch.
> 
> *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.

Hi Nitika,

Thanks, the patch LGTM, with a few formatting comments addressed.

> gdb/ChangeLog:
>         *gdb/dwarf2loc.c (decode_debug_loclists_addresses): Handle DW_LLE_base_addressx, DW_LLE_startx_length, DW_LLE_start_length.

The file in the ChangeLog entry should be relative to the ChangeLog file
it will end up in.  Since this will go in gdb/ChangeLog, it should be
"dwarf2loc.c", not "gdb/dwarf2loc.c".  Also, it should be wrapped to 80
columns.  So, something like this:

	* 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..e0940689ee 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;

Not sure if it's your email client that changed them to spaces, but the lines
at this indentation level (there are a few in the patch) should be indented
with a tab.

> +      *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;

Something I noticed while reviewing, which could be a possible cleanup made
in a separate patch if you'd like.  I don't really see why DEBUG_LOC_START_LENGTH
exists.  As we return the low and high addresses to the caller, it's no different
than DEBUG_LOC_START_END.  And indeed, the callers treat both DEBUG_LOC_START_END
and DEBUG_LOC_START_LENGTH the same way.  So I think _LENGTH could be removed.

And since `enum debug_loc_kind` is only used internally in GDB to communicate
between functions (it's not used for parsing anything), it doesn't really need
to have explicit values for enumerators.  The documentation about the encoding
also seems irrelevant to me, since, again, they are not even used for parsing
anything.

> +    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:

Did you add them here because you don't intend to add support for them, at least
for the moment?  If so, please add a comment, like:

/* Not supported yet.  */

Thanks,

Simon

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

[AMD Official Use Only - Internal Distribution Only]



Hi Simon,

Thanks for the detailed review. 

***
> +        return DEBUG_LOC_BUFFER_OVERFLOW;

> Not sure if it's your email client that changed them to spaces, but the lines at this indentation level (there are a few in the patch) should be indented with a tab.

I was sending the mail in HTML format which is converting tabs to spaces while sending. Now, I am sending in plain text format. It should fix the issue.

***
> +      *high += u64;
> +      *new_ptr = loc_ptr;
> +      return DEBUG_LOC_START_LENGTH;

> Something I noticed while reviewing, which could be a possible cleanup made in a separate patch if you'd like.  I don't really see why DEBUG_LOC_START_LENGTH exists.  As we return > > the low and high addresses to the caller, it's no different than DEBUG_LOC_START_END.  And indeed, the callers treat both DEBUG_LOC_START_END and DEBUG_LOC_START_LENGTH > > the same way.  So I think _LENGTH could be removed.

> And since `enum debug_loc_kind` is only used internally in GDB to communicate between functions (it's not used for parsing anything), it doesn't really need to have explicit values for > enumerators.  The documentation about the encoding also seems irrelevant to me, since, again, they are not even used for parsing anything.

I will made this cleanup in the separate patch.


***
> +    case DW_LLE_startx_endx:
> +    case DW_LLE_start_end:
> +    case DW_LLE_default_location:

> Did you add them here because you don't intend to add support for them, at least for the moment?  If so, please add a comment, like:

> /* Not supported yet.  */

Done. Clang and gcc are not emitting the above three entries as of now. So, I didn't add the support for them.

---
*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:
        *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 Achra

-----Original Message-----
From: Simon Marchi <simark@simark.ca> 
Sent: Wednesday, January 8, 2020 8:57 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 2020-01-06 5:14 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Simon,
>
> Sorry. I sent the wrong patch by mistake. Updating the patch.
>
> *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.

Hi Nitika,

Thanks, the patch LGTM, with a few formatting comments addressed.

> gdb/ChangeLog:
>         *gdb/dwarf2loc.c (decode_debug_loclists_addresses): Handle DW_LLE_base_addressx, DW_LLE_startx_length, DW_LLE_start_length.

The file in the ChangeLog entry should be relative to the ChangeLog file it will end up in.  Since this will go in gdb/ChangeLog, it should be "dwarf2loc.c", not "gdb/dwarf2loc.c".  Also, it should be wrapped to 80 columns.  So, something like this:

        * 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..e0940689ee 
> 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;

Not sure if it's your email client that changed them to spaces, but the lines at this indentation level (there are a few in the patch) should be indented with a tab.

> +      *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;

Something I noticed while reviewing, which could be a possible cleanup made in a separate patch if you'd like.  I don't really see why DEBUG_LOC_START_LENGTH exists.  As we return the low and high addresses to the caller, it's no different than DEBUG_LOC_START_END.  And indeed, the callers treat both DEBUG_LOC_START_END and DEBUG_LOC_START_LENGTH the same way.  So I think _LENGTH could be removed.

And since `enum debug_loc_kind` is only used internally in GDB to communicate between functions (it's not used for parsing anything), it doesn't really need to have explicit values for enumerators.  The documentation about the encoding also seems irrelevant to me, since, again, they are not even used for parsing anything.

> +    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:

Did you add them here because you don't intend to add support for them, at least for the moment?  If so, please add a comment, like:

/* Not supported yet.  */

Thanks,

Simon

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-13 5:45 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> 
> 
> Hi Simon,
> 
> Thanks for the detailed review. 
> 
> ***
>> +        return DEBUG_LOC_BUFFER_OVERFLOW;
> 
>> Not sure if it's your email client that changed them to spaces, but the lines at this indentation level (there are a few in the patch) should be indented with a tab.
> 
> I was sending the mail in HTML format which is converting tabs to spaces while sending. Now, I am sending in plain text format. It should fix the issue.

Thanks.  The best is to use git-send-email, which makes git send the patch by
email directly, in the right format.  It requires a bit of setup, since you
need to specify your SMTP server settings, but it's really worth it.

If you can't use git-send-email for some reason, the second best option is
git-format-patch.  git-format-patch will produce a complete patch, including
the subject and the commit message.  So if you want to send your patch as an
attachment or paste it in an email, please use git-format-patch instead of
git-diff, which just produces the diff, without the commit message.

Finally, if you manually paste your patch in an email, then you need to make
sure the email client doesn't change anything (like break long lines), otherwise
the patch will be corrupted.

> ***
>> +      *high += u64;
>> +      *new_ptr = loc_ptr;
>> +      return DEBUG_LOC_START_LENGTH;
> 
>> Something I noticed while reviewing, which could be a possible cleanup made in a separate patch if you'd like.  I don't really see why DEBUG_LOC_START_LENGTH exists.  As we return > > the low and high addresses to the caller, it's no different than DEBUG_LOC_START_END.  And indeed, the callers treat both DEBUG_LOC_START_END and DEBUG_LOC_START_LENGTH > > the same way.  So I think _LENGTH could be removed.
> 
>> And since `enum debug_loc_kind` is only used internally in GDB to communicate between functions (it's not used for parsing anything), it doesn't really need to have explicit values for > enumerators.  The documentation about the encoding also seems irrelevant to me, since, again, they are not even used for parsing anything.
> 
> I will made this cleanup in the separate patch.

Thanks!

> ***
>> +    case DW_LLE_startx_endx:
>> +    case DW_LLE_start_end:
>> +    case DW_LLE_default_location:
> 
>> Did you add them here because you don't intend to add support for them, at least for the moment?  If so, please add a comment, like:
> 
>> /* Not supported yet.  */
> 
> Done. Clang and gcc are not emitting the above three entries as of now. So, I didn't add the support for them.

Hmm, I don't see the comment in the new version of the patch.

Simon

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 5913 bytes --]

[AMD Official Use Only - Internal Distribution Only]

Hi Simon,

Sorry, I missed the comment. And I will try to setup the git-send-email before the next patch. Thank you.

*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:
        *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;
+    /* Following cases are not supported yet. */
+    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: Monday, January 13, 2020 10:07 PM
To: Achra, Nitika <Nitika.Achra@amd.com>; gdb-patches@sourceware.org
Cc: Ali Tamur <tamur@google.com>; George, Jini Susan <JiniSusan.George@amd.com>
Subject: Re: [PATCH] Support for DWARF5 location lists entries

[CAUTION: External Email]

On 2020-01-13 5:45 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
>
>
> Hi Simon,
>
> Thanks for the detailed review.
>
> ***
>> +        return DEBUG_LOC_BUFFER_OVERFLOW;
>
>> Not sure if it's your email client that changed them to spaces, but the lines at this indentation level (there are a few in the patch) should be indented with a tab.
>
> I was sending the mail in HTML format which is converting tabs to spaces while sending. Now, I am sending in plain text format. It should fix the issue.

Thanks.  The best is to use git-send-email, which makes git send the patch by email directly, in the right format.  It requires a bit of setup, since you need to specify your SMTP server settings, but it's really worth it.

If you can't use git-send-email for some reason, the second best option is git-format-patch.  git-format-patch will produce a complete patch, including the subject and the commit message.  So if you want to send your patch as an attachment or paste it in an email, please use git-format-patch instead of git-diff, which just produces the diff, without the commit message.

Finally, if you manually paste your patch in an email, then you need to make sure the email client doesn't change anything (like break long lines), otherwise the patch will be corrupted.

> ***
>> +      *high += u64;
>> +      *new_ptr = loc_ptr;
>> +      return DEBUG_LOC_START_LENGTH;
>
>> Something I noticed while reviewing, which could be a possible cleanup made in a separate patch if you'd like.  I don't really see why DEBUG_LOC_START_LENGTH exists.  As we return > > the low and high addresses to the caller, it's no different than DEBUG_LOC_START_END.  And indeed, the callers treat both DEBUG_LOC_START_END and DEBUG_LOC_START_LENGTH > > the same way.  So I think _LENGTH could be removed.
>
>> And since `enum debug_loc_kind` is only used internally in GDB to communicate between functions (it's not used for parsing anything), it doesn't really need to have explicit values for > enumerators.  The documentation about the encoding also seems irrelevant to me, since, again, they are not even used for parsing anything.
>
> I will made this cleanup in the separate patch.

Thanks!

> ***
>> +    case DW_LLE_startx_endx:
>> +    case DW_LLE_start_end:
>> +    case DW_LLE_default_location:
>
>> Did you add them here because you don't intend to add support for them, at least for the moment?  If so, please add a comment, like:
>
>> /* Not supported yet.  */
>
> Done. Clang and gcc are not emitting the above three entries as of now. So, I didn't add the support for them.

Hmm, I don't see the comment in the new version of the patch.

Simon\x16º&Öéj×!zÊÞ¶êç×­ôçyb²Ö«r\x18\x1dn–­r\x17¬

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-14 2:02 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hi Simon,
> 
> Sorry, I missed the comment. And I will try to setup the git-send-email before the next patch. Thank you.

Thanks.

I could push this patch on your behalf, but I am having trouble applying it, probably
because your client (I'm guessing outlook?) has messed with it.  Could you please
generate it with git-format-patch and send it as an attachment?  This way it should
arrive intact.

Simon

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

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

[AMD Official Use Only - Internal Distribution Only]

Hi Simon,

Please find the attachment.


*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:
        *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.

Regards,
Nitika Achra

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

[CAUTION: External Email]

On 2020-01-14 2:02 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Simon,
>
> Sorry, I missed the comment. And I will try to setup the git-send-email before the next patch. Thank you.

Thanks.

I could push this patch on your behalf, but I am having trouble applying it, probably because your client (I'm guessing outlook?) has messed with it.  Could you please generate it with git-format-patch and send it as an attachment?  This way it should arrive intact.

Simon

[-- Attachment #2: 0001-Support-for-DWARF5-location-lists-entries.patch --]
[-- Type: application/octet-stream, Size: 2314 bytes --]

From 34e3bb9ba58ee73713574c14df718943d5ee50f3 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Thu, 16 Jan 2020 16:43:21 +0530
Subject: [PATCH] Support for DWARF5 location lists entries

---
 gdb/dwarf2loc.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1fe6829100..cf358fbe61 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,10 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
       *high = u64;
       *new_ptr = loc_ptr;
       return DEBUG_LOC_START_END;
+    /* Following cases are not supported yet. */
+    case DW_LLE_startx_endx:
+    case DW_LLE_start_end:
+    case DW_LLE_default_location:
     default:
       return DEBUG_LOC_INVALID_ENTRY;
     }
-- 
2.17.1

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-16 6:29 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hi Simon,
> 
> Please find the attachment.
> 
> 
> *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:
>         *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.

Hi Achra,

The commit message of the patch is empty.  The text that is above, which describes what
your patch does, should be placed in your git commit, such that it will appear in the
patch file when you use "git format-patch".

This is the text that people will see when they do "git log", so you can do "git log"
to see how it's usually formatted.

Thanks,

Simon

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-16 9:15 a.m., Simon Marchi wrote:
> On 2020-01-16 6:29 a.m., Achra, Nitika wrote:
>> [AMD Official Use Only - Internal Distribution Only]
>>
>> Hi Simon,
>>
>> Please find the attachment.
>>
>>
>> *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:
>>         *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.
> 
> Hi Achra,

Err sorry, I meant "Hi Nitika"!

RE: [PATCH] Support for DWARF5 location lists entries

[Achra, Nitika]

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

[AMD Official Use Only - Internal Distribution Only]

Hi Simon,

I have modified the commit message. Please find the attachment.

Regards,
Nitika Achra

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

[CAUTION: External Email]

On 2020-01-16 6:29 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Simon,
>
> Please find the attachment.
>
>
> *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:
>         *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.

Hi Achra,

The commit message of the patch is empty.  The text that is above, which describes what your patch does, should be placed in your git commit, such that it will appear in the patch file when you use "git format-patch".

This is the text that people will see when they do "git log", so you can do "git log"
to see how it's usually formatted.

Thanks,

Simon

[-- Attachment #2: 0001-Support-for-DWARF5-location-lists-entries.patch --]
[-- Type: application/octet-stream, Size: 2836 bytes --]

From f11af3868b64db169a7c5ac5ddf931e868252195 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Thu, 16 Jan 2020 16:43:21 +0530
Subject: [PATCH] Support for DWARF5 location lists entries

This patch handles 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.

This is an effort to support DWARF5 in gdb.

gdb/ChangeLog:

   *dwarf2loc.c (decode_debug_loclists_addresses): Handle
    DW_LLE_base_addressx, DW_LLE_startx_length, DW_LLE_start_length.
---
 gdb/dwarf2loc.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 1fe6829100..cf358fbe61 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,10 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
       *high = u64;
       *new_ptr = loc_ptr;
       return DEBUG_LOC_START_END;
+    /* Following cases are not supported yet. */
+    case DW_LLE_startx_endx:
+    case DW_LLE_start_end:
+    case DW_LLE_default_location:
     default:
       return DEBUG_LOC_INVALID_ENTRY;
     }
-- 
2.17.1

Re: [PATCH] Support for DWARF5 location lists entries

[Simon Marchi]

On 2020-01-16 10:30 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hi Simon,
> 
> I have modified the commit message. Please find the attachment.
> 
> Regards,
> Nitika Achra

Thanks!  I have pushed the patch on your behalf.

Simon