[PATCH]: DWARFv5: Handle location list for split dwarf

[PATCH]: DWARFv5: Handle location list for split dwarf

[Achra, Nitika]

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

[AMD Official Use Only - Internal Distribution Only]

    DWARFv5: Handle location list for split dwarf

    GDB throws the error '<error reading variable: dwarf2_find_location_
    expression: Corrupted DWARF expression.>' while printing the variable
    value with -gdwarf-5, -O3 and -gdwarf-split=split flags. This patch fixes this error.

    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 -gdwarf-5 flags.

    gdb/ChangeLog:

       *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
        (dwarf2_find_location_expression): Call the function decode_debug_loclists_
        addresses if DWARF version is 5 or more. Add applicable base address if the
        entry is DW_LLE_OFFSET_PAIR from DWO.
        (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
        DEBUG_LOC_START_END in case of DE_LLE_offset_pair.

Regards,
Nitika Achra
     

[-- Attachment #2: 0001-DWARFv5-Handle-location-list-for-split-dwarf.patch --]
[-- Type: application/octet-stream, Size: 3705 bytes --]

From 8c484b20ad9b4671e906b0090016161e63b8bd65 Mon Sep 17 00:00:00 2001
From: nitachra <Nitika.Achra@amd.com>
Date: Fri, 24 Jan 2020 16:20:18 +0530
Subject: [PATCH] DWARFv5: Handle location list for split dwarf.

GDB throws the error '<error reading variable: dwarf2_find_location_
expression: Corrupted DWARF expression.>' while printing the variable
value with executable file compiled with -gdwarf-5, -O3 and -gdwarf-split=
split flags. This patch fixes this error.

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 -gdwarf-5 flags.

gdb/ChangeLog:

   *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
    (dwarf2_find_location_expression): Call the function decode_debug_loclists_
    addresses if DWARF version is 5 or more. Add applicable base address if the
    entry is DW_LLE_OFFSET_PAIR from DWO.
    (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
    DEBUG_LOC_START_END in case of DE_LLE_offset_pair.
---
 gdb/dwarf2loc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 405b239ed4..34795b70f9 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -90,6 +90,11 @@ enum debug_loc_kind
      number that specifies the length, and then a normal location expression
      as in .debug_loc.  */
   DEBUG_LOC_START_LENGTH = 3,
+  
+  /* This is followed by two unsigned LEB128 operands. The values of these 
+     operands are the starting and ending offsets, respectively, relative to 
+     the applicable base address. */
+  DEBUG_LOC_OFFSET_PAIR = 4,
 
   /* An internal value indicating there is insufficient data.  */
   DEBUG_LOC_BUFFER_OVERFLOW = -1,
@@ -231,7 +236,7 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
 	return DEBUG_LOC_BUFFER_OVERFLOW;
       *high = u64;
       *new_ptr = loc_ptr;
-      return DEBUG_LOC_START_END;
+      return DEBUG_LOC_OFFSET_PAIR;
     /* Following cases are not supported yet.  */
     case DW_LLE_startx_endx:
     case DW_LLE_start_end:
@@ -331,7 +336,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->from_dwo)
+      if (dwarf2_version(baton->per_cu) < 5 && baton->from_dwo)
 	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
 					       loc_ptr, buf_end, &new_ptr,
 					       &low, &high, byte_order);
@@ -357,6 +362,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  continue;
 	case DEBUG_LOC_START_END:
 	case DEBUG_LOC_START_LENGTH:
+	case DEBUG_LOC_OFFSET_PAIR:
 	  break;
 	case DEBUG_LOC_BUFFER_OVERFLOW:
 	case DEBUG_LOC_INVALID_ENTRY:
@@ -368,9 +374,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 
       /* Otherwise, a location expression entry.
 	 If the entry is from a DWO, don't add base address: the entry is from
-	 .debug_addr which already has the DWARF "base address".  We still add
-	 base_offset in case we're debugging a PIE executable.  */
-      if (baton->from_dwo)
+	 .debug_addr which already has the DWARF "base address". We still add
+	 base_offset in case we're debugging a PIE executable. However, if the 
+	 entry is DW_LLE_offset_pair from a DWO, add the base address as the 
+	 operands are offsets relative to the applicable base address.  */
+      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
 	{
 	  low += base_offset;
 	  high += base_offset;
-- 
2.17.1

Re: [PATCH]: DWARFv5: Handle location list for split dwarf

[Simon Marchi]

On 2020-01-25 11:55 a.m., Achra, Nitika wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
>     DWARFv5: Handle location list for split dwarf
> 
>     GDB throws the error '<error reading variable: dwarf2_find_location_
>     expression: Corrupted DWARF expression.>' while printing the variable
>     value with -gdwarf-5, -O3 and -gdwarf-split=split flags. This patch fixes this error.
> 
>     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 -gdwarf-5 flags.
> 
>     gdb/ChangeLog:
> 
>        *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
>         (dwarf2_find_location_expression): Call the function decode_debug_loclists_
>         addresses if DWARF version is 5 or more. Add applicable base address if the
>         entry is DW_LLE_OFFSET_PAIR from DWO.
>         (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
>         DEBUG_LOC_START_END in case of DE_LLE_offset_pair.
> 
> Regards,
> Nitika Achra

Hi Nitika,

Could you please provide a test snippet, along with the compiler version and command line
you use, that generates this error?  Please also put it in the commit message so that anyone
inspecting this commit later on can reproduce it.

Finally, would you mind trying to send the upcoming patches using git-send-email?  It makes
it easier to read and comment them.

> From 8c484b20ad9b4671e906b0090016161e63b8bd65 Mon Sep 17 00:00:00 2001
> From: nitachra <Nitika.Achra@amd.com>
> Date: Fri, 24 Jan 2020 16:20:18 +0530
> Subject: [PATCH] DWARFv5: Handle location list for split dwarf.
>
> GDB throws the error '<error reading variable: dwarf2_find_location_
> expression: Corrupted DWARF expression.>' while printing the variable
> value with executable file compiled with -gdwarf-5, -O3 and -gdwarf-split=
> split flags. This patch fixes this error.
>
> 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 -gdwarf-5 flags.
>
> gdb/ChangeLog:
>
>    *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
>     (dwarf2_find_location_expression): Call the function decode_debug_loclists_
>     addresses if DWARF version is 5 or more. Add applicable base address if the
>     entry is DW_LLE_OFFSET_PAIR from DWO.
>     (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
>     DEBUG_LOC_START_END in case of DE_LLE_offset_pair.
> ---
>  gdb/dwarf2loc.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
> index 405b239ed4..34795b70f9 100644
> --- a/gdb/dwarf2loc.c
> +++ b/gdb/dwarf2loc.c
> @@ -90,6 +90,11 @@ enum debug_loc_kind
>       number that specifies the length, and then a normal location expression
>       as in .debug_loc.  */
>    DEBUG_LOC_START_LENGTH = 3,
> +
> +  /* This is followed by two unsigned LEB128 operands. The values of these
> +     operands are the starting and ending offsets, respectively, relative to
> +     the applicable base address. */

Two spaces after both periods.

> +  DEBUG_LOC_OFFSET_PAIR = 4,
>
>    /* An internal value indicating there is insufficient data.  */
>    DEBUG_LOC_BUFFER_OVERFLOW = -1,
> @@ -231,7 +236,7 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
>  	return DEBUG_LOC_BUFFER_OVERFLOW;
>        *high = u64;
>        *new_ptr = loc_ptr;
> -      return DEBUG_LOC_START_END;
> +      return DEBUG_LOC_OFFSET_PAIR;
>      /* Following cases are not supported yet.  */
>      case DW_LLE_startx_endx:
>      case DW_LLE_start_end:
> @@ -331,7 +336,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>        enum debug_loc_kind kind;
>        const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
>
> -      if (baton->from_dwo)
> +      if (dwarf2_version(baton->per_cu) < 5 && baton->from_dwo)

This needs to be replaced with:

  baton->per_cu->version ()

IIUC, both non-dwo and dwo cases for DWARF 5 are handled by decode_debug_loclists_addresses ?

>  	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
>  					       loc_ptr, buf_end, &new_ptr,
>  					       &low, &high, byte_order);
> @@ -357,6 +362,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>  	  continue;
>  	case DEBUG_LOC_START_END:
>  	case DEBUG_LOC_START_LENGTH:
> +	case DEBUG_LOC_OFFSET_PAIR:
>  	  break;
>  	case DEBUG_LOC_BUFFER_OVERFLOW:
>  	case DEBUG_LOC_INVALID_ENTRY:
> @@ -368,9 +374,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>
>        /* Otherwise, a location expression entry.
>  	 If the entry is from a DWO, don't add base address: the entry is from
> -	 .debug_addr which already has the DWARF "base address".  We still add
> -	 base_offset in case we're debugging a PIE executable.  */
> -      if (baton->from_dwo)
> +	 .debug_addr which already has the DWARF "base address". We still add
> +	 base_offset in case we're debugging a PIE executable. However, if the
> +	 entry is DW_LLE_offset_pair from a DWO, add the base address as the
> +	 operands are offsets relative to the applicable base address.  */
> +      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)

Two spaces after periods.

Thanks,

Simon

[PATCH] DWARFv5: Handle location list for split dwarf.

[nitachra]

Hi Simon,


>>On 2020-03-18 10:15 a.m., nitachra wrote:
>>
>> Hi Simon,
>>
>> Thanks for the review.
>>
>>> @@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>>>        enum debug_loc_kind kind;
>>>        const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
>>>
>>> -      if (baton->from_dwo)
>>> +      if (baton->per_cu->version () < 5 && baton->from_dwo)
>>>
>>> Can you please explain the rationale of this change?  It's hard to tell whether it is correct or not without an explanation (and I prefer not having to guess your intentions).
>>
>> I made this change because clang and gcc are not generating the DW_LLE_GNU* entries with DWARFv5 and -gsplit-dwarf.
>> Instead they are generating DW_LLE_start* or DW_LLE_offset_pair with DW_LLE_base_addressx.

>Thanks.  This is the kind of information that is useful to include in the commit message, that explains why you needed to do such and such change.

Modified the commit message.

>> if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
>>       {
>>         low += base_offset;
>>         high += base_offset;
>>       }
>>       else
>>       {
>>         low += base_address;
>>         high += base_address;
>>       }
>>
>>
>> I made the above change because clang is emitting DW_LLE_offset_pair and DW_LLE_base_addressx for DWARFv5 and -gsplit-dwarf.
>> So we need to add the base address in the case of dwo also, otherwise
>> the low and high address will be incorrect and gdb will give <optimized out> when printing the variable values.
>>
>> These two changes combined will print the correct variable values.
>> Overall the intent for this patch was to support the location list for split dwarf and print the correct variable values.

>Hmm, all I get is <optimized out>.  I checked the generated binary a bit closer (test.out and test.dwo), am I supposed to see a .debug_loclists or .debug_loclists.dwo section in there?
>Because I don't see any... Again, this is with this clang version, take from apt.llvm.org:

>clang version 10.0.0-++20200317072632+135744ce689-1~exp1~20200317063228.125

I have included a new test case in the commit message. I was actually using the older clang 10.0.0-rc1 from here https://github.com/llvm/llvm-project/releases. With the new test case
you can reproduce the error with clang-10.0.0 also. You can see the debug_loclist.dwo with this test case.

Regards,
Nitika
---
GDB throws the error '<error reading variable: dwarf2_find_location_
expression: Corrupted DWARF expression.>' while printing the variable
value with executable file compiled with -gdwarf-5, -O1 and -gdwarf-split
flags. This is because DW_LLE_start* or DW_LLE_offset_pair
with DW_LLE_base_addressx are being emitted in DWARFv5 location list
instead of DW_LLE_GNU*. This patch fixes this error.

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 -gdwarf-5 flags. Used clang version 10.0.0. This is the test case used-

void bar(int arr[], int l, int m, int r)
{
    int i, j, k, n1= m - l + 1, n2= r - m, L[n1], R[n2];
    for (i = 0; i < n1; i++)
        L[i] = arr[l + i];
    for (j = 0; j < n2; j++)
        R[j] = arr[m + 1+ j];
}

int main()
{
    int arr[] = {12, 11};
    bar(arr,0,1,2);
    return 0;

clang -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
gdb test.out
gdb> start
gdb> step
gdb> step
gdb> step
gdb> step
gdb> p L[0]
dwarf2_find_location_expression: Corrupted DWARF expression.

gdb/ChangeLog:

   *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
    (dwarf2_find_location_expression): Call the function decode_debug_loclists_
    addresses if DWARF version is 5 or more. DW_LLE_start* or DW_LLE_offset_pair
    with DW_LLE_base_addressx are being emitted in DWARFv5 instead of DW_LLE_GNU*.
    Add applicable base address if the entry is DW_LLE_offset_pair from DWO.
    (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
    DEBUG_LOC_START_END in case of DW_LLE_offset_pair.
---
 gdb/dwarf2/loc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 5155cff60d..08c315a381 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -92,6 +92,11 @@ enum debug_loc_kind
      as in .debug_loc.  */
   DEBUG_LOC_START_LENGTH = 3,
 
+   /* This is followed by two unsigned LEB128 operands. The values of these
+      operands are the starting and ending offsets, respectively, relative to
+      the applicable base address. */
+  DEBUG_LOC_OFFSET_PAIR = 4,
+
   /* An internal value indicating there is insufficient data.  */
   DEBUG_LOC_BUFFER_OVERFLOW = -1,
 
@@ -232,7 +237,7 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
 	return DEBUG_LOC_BUFFER_OVERFLOW;
       *high = u64;
       *new_ptr = loc_ptr;
-      return DEBUG_LOC_START_END;
+      return DEBUG_LOC_OFFSET_PAIR;
     /* Following cases are not supported yet.  */
     case DW_LLE_startx_endx:
     case DW_LLE_start_end:
@@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->from_dwo)
+      if (baton->per_cu->version () < 5 && baton->from_dwo)
 	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
 					       loc_ptr, buf_end, &new_ptr,
 					       &low, &high, byte_order);
@@ -358,6 +363,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  continue;
 	case DEBUG_LOC_START_END:
 	case DEBUG_LOC_START_LENGTH:
+	case DEBUG_LOC_OFFSET_PAIR:
 	  break;
 	case DEBUG_LOC_BUFFER_OVERFLOW:
 	case DEBUG_LOC_INVALID_ENTRY:
@@ -369,9 +375,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 
       /* Otherwise, a location expression entry.
 	 If the entry is from a DWO, don't add base address: the entry is from
-	 .debug_addr which already has the DWARF "base address".  We still add
-	 base_offset in case we're debugging a PIE executable.  */
-      if (baton->from_dwo)
+	 .debug_addr which already has the DWARF "base address". We still add
+	 base_offset in case we're debugging a PIE executable. However, if the
+	 entry is DW_LLE_offset_pair from a DWO, add the base address as the
+	 operands are offsets relative to the applicable base address. */
+      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
 	{
 	  low += base_offset;
 	  high += base_offset;
-- 
2.17.1

Re: [PATCH] DWARFv5: Handle location list for split dwarf.

[Simon Marchi]

On 2020-03-18 10:15 a.m., nitachra wrote:
> 
> Hi Simon,
> 
> Thanks for the review.
> 
>> @@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>>        enum debug_loc_kind kind;
>>        const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
>>
>> -      if (baton->from_dwo)
>> +      if (baton->per_cu->version () < 5 && baton->from_dwo)
> 
>> Can you please explain the rationale of this change?  It's hard to tell whether it is correct or not without an explanation (and I prefer not having to guess your intentions).
> 
> I made this change because clang and gcc are not generating the DW_LLE_GNU* entries with DWARFv5 and -gsplit-dwarf.
> Instead they are generating DW_LLE_start* or DW_LLE_offset_pair with DW_LLE_base_addressx.

Thanks.  This is the kind of information that is useful to include in the
commit message, that explains why you needed to do such and such change.

> if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
> 	{
> 	  low += base_offset;
> 	  high += base_offset;
> 	}
>       else
> 	{
> 	  low += base_address;
> 	  high += base_address;
> 	}
> 
> 
> I made the above change because clang is emitting DW_LLE_offset_pair and DW_LLE_base_addressx for DWARFv5 and -gsplit-dwarf.
> So we need to add the base address in the case of dwo also, otherwise the low and high address will be incorrect
> and gdb will give <optimized out> when printing the variable values.
> 
> These two changes combined will print the correct variable values. Overall the intent for this patch was to support the location
> list for split dwarf and print the correct variable values.

Hmm, all I get is <optimized out>.  I checked the generated binary a bit closer (test.out and
test.dwo), am I supposed to see a .debug_loclists or .debug_loclists.dwo section in there?
Because I don't see any... Again, this is with this clang version, take from apt.llvm.org:

    clang version 10.0.0-++20200317072632+135744ce689-1~exp1~20200317063228.125

> 
>> I am not able to reproduce to get this error message, there must be something else to trigger this bug:
> 
> To reproduce you have to apply the patch at https://sourceware.org/pipermail/gdb-patches/2020-January/164841.html
> I updated the patch right now. Reapply the patch if you have already applied. You will be able to regenerate the error then.

Ok.

Note that if you have multiple closely related patches (and even more importantly if
they have interdependencies), you can send them as a series, it then becomes much easier
to track them.

Also, please version your patches (if you look at other patches on the list, you'll see
the subject may start with "[PATCH v2]").  That makes it much easier to know which one
is the latest, especially if you start a new thread.

Simon

[PATCH] DWARFv5: Handle location list for split dwarf.

[nitachra]

Hi Simon,

Thanks for the review.

> @@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>        enum debug_loc_kind kind;
>        const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
>
> -      if (baton->from_dwo)
> +      if (baton->per_cu->version () < 5 && baton->from_dwo)

>Can you please explain the rationale of this change?  It's hard to tell whether it is correct or not without an explanation (and I prefer not having to guess your intentions).

I made this change because clang and gcc are not generating the DW_LLE_GNU* entries with DWARFv5 and -gsplit-dwarf.
Instead they are generating DW_LLE_start* or DW_LLE_offset_pair with DW_LLE_base_addressx.



if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
	{
	  low += base_offset;
	  high += base_offset;
	}
      else
	{
	  low += base_address;
	  high += base_address;
	}


I made the above change because clang is emitting DW_LLE_offset_pair and DW_LLE_base_addressx for DWARFv5 and -gsplit-dwarf.
So we need to add the base address in the case of dwo also, otherwise the low and high address will be incorrect
and gdb will give <optimized out> when printing the variable values.

These two changes combined will print the correct variable values. Overall the intent for this patch was to support the location
list for split dwarf and print the correct variable values.

>I am not able to reproduce to get this error message, there must be something else to trigger this bug:

To reproduce you have to apply the patch at https://sourceware.org/pipermail/gdb-patches/2020-January/164841.html
I updated the patch right now. Reapply the patch if you have already applied. You will be able to regenerate the error then.

>I don't really understand the patch. You add DEBUG_LOC_OFFSET_PAIR, but no function actually returns this kind.

My mistake. I sent the wrong patch before. 

Regards,
Nitika
---

GDB throws the error '<error reading variable: dwarf2_find_location_
expression: Corrupted DWARF expression.>' while printing the variable
value with executable file compiled with -gdwarf-5, -O1 and -gdwarf-split
flags. This patch fixes this error.

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 -gdwarf-5 flags. Used clang version 10.0.0. This is the test case used-

int main()
{
    int arr[] = {1,2,3};
    for(int i = 0; i<3; i++)
        printf("%d",arr[i]);
    return 0;
}

clang -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
gdb test.out
gdb> start
gdb> step
gdb> p i
dwarf2_find_location_expression: Corrupted DWARF expression.

gdb/ChangeLog:

   *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
    (dwarf2_find_location_expression): Call the function decode_debug_loclists_
    addresses if DWARF version is 5 or more. Add applicable base address if the
    entry is DW_LLE_OFFSET_PAIR from DWO.
    (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
    DEBUG_LOC_START_END in case of DW_LLE_offset_pair.
---
 gdb/dwarf2/loc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 5155cff60d..08c315a381 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -92,6 +92,11 @@ enum debug_loc_kind
      as in .debug_loc.  */
   DEBUG_LOC_START_LENGTH = 3,
 
+   /* This is followed by two unsigned LEB128 operands. The values of these
+      operands are the starting and ending offsets, respectively, relative to
+      the applicable base address. */
+  DEBUG_LOC_OFFSET_PAIR = 4,
+
   /* An internal value indicating there is insufficient data.  */
   DEBUG_LOC_BUFFER_OVERFLOW = -1,
 
@@ -232,7 +237,7 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
 	return DEBUG_LOC_BUFFER_OVERFLOW;
       *high = u64;
       *new_ptr = loc_ptr;
-      return DEBUG_LOC_START_END;
+      return DEBUG_LOC_OFFSET_PAIR;
     /* Following cases are not supported yet.  */
     case DW_LLE_startx_endx:
     case DW_LLE_start_end:
@@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->from_dwo)
+      if (baton->per_cu->version () < 5 && baton->from_dwo)
 	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
 					       loc_ptr, buf_end, &new_ptr,
 					       &low, &high, byte_order);
@@ -358,6 +363,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  continue;
 	case DEBUG_LOC_START_END:
 	case DEBUG_LOC_START_LENGTH:
+	case DEBUG_LOC_OFFSET_PAIR:
 	  break;
 	case DEBUG_LOC_BUFFER_OVERFLOW:
 	case DEBUG_LOC_INVALID_ENTRY:
@@ -369,9 +375,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 
       /* Otherwise, a location expression entry.
 	 If the entry is from a DWO, don't add base address: the entry is from
-	 .debug_addr which already has the DWARF "base address".  We still add
-	 base_offset in case we're debugging a PIE executable.  */
-      if (baton->from_dwo)
+	 .debug_addr which already has the DWARF "base address". We still add
+	 base_offset in case we're debugging a PIE executable. However, if the
+	 entry is DW_LLE_offset_pair from a DWO, add the base address as the
+	 operands are offsets relative to the applicable base address. */
+      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
 	{
 	  low += base_offset;
 	  high += base_offset;
-- 
2.17.1

Re: [PATCH] DWARFv5: Handle location list for split dwarf.

[Simon Marchi]

I don't really understand the patch.  You add DEBUG_LOC_OFFSET_PAIR, but no function actually
returns this kind.

On 2020-03-17 11:13 a.m., nitachra wrote:
> This is the updated patch after rebasing.
> 
> Regards,
> Nitika
> ---
> GDB throws the error '<error reading variable: dwarf2_find_location_
> expression: Corrupted DWARF expression.>' while printing the variable
> value with executable file compiled with -gdwarf-5, -O1 and -gdwarf-split
> flags. This patch fixes this error.
> 
> 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 -gdwarf-5 flags. Used clang version 10.0.0. This is the test case used-
> 
> int main()
> {
>     int arr[] = {1,2,3};
>     for(int i = 0; i<3; i++)
>         printf("%d",arr[i]);
>     return 0;
> }
> 
> clang -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
> gdb test.out
> gdb> start
> gdb> step
> gdb> p i
> dwarf2_find_location_expression: Corrupted DWARF expression.

I am not able to reproduce to get this error message, there must be something else to
trigger this bug:

$ cat test.c
#include <stdio.h>

int main()
{
  int arr[] = {1,2,3};
  for(int i = 0; i<3; i++)
    printf("%d",arr[i]);
  return 0;
}
$ clang-10 --version
clang version 10.0.0-++20200317072632+135744ce689-1~exp1~20200317063228.125
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
$ clang-10 -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
$ ./gdb -q -nx --data-directory=data-directory test.out
Reading symbols from test.out...
(gdb) start
Temporary breakpoint 1 at 0x400500: file test.c, line 7.
Starting program: /home/simark/build/binutils-gdb/gdb/test.out

Temporary breakpoint 1, main () at test.c:7
7           printf("%d",arr[i]);
(gdb) step
6         for(int i = 0; i<3; i++)
(gdb) p i
$1 = <optimized out>

> @@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
>        enum debug_loc_kind kind;
>        const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
>  
> -      if (baton->from_dwo)
> +      if (baton->per_cu->version () < 5 && baton->from_dwo)

Can you please explain the rationale of this change?  It's hard to tell whether it is correct
or not without an explanation (and I prefer not having to guess your intentions).

Simon

[PATCH] DWARFv5: Handle location list for split dwarf.

[nitachra]

This is the updated patch after rebasing.

Regards,
Nitika
---
GDB throws the error '<error reading variable: dwarf2_find_location_
expression: Corrupted DWARF expression.>' while printing the variable
value with executable file compiled with -gdwarf-5, -O1 and -gdwarf-split
flags. This patch fixes this error.

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 -gdwarf-5 flags. Used clang version 10.0.0. This is the test case used-

int main()
{
    int arr[] = {1,2,3};
    for(int i = 0; i<3; i++)
        printf("%d",arr[i]);
    return 0;
}

clang -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
gdb test.out
gdb> start
gdb> step
gdb> p i
dwarf2_find_location_expression: Corrupted DWARF expression.

gdb/ChangeLog:

   *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
    (dwarf2_find_location_expression): Call the function decode_debug_loclists_
    addresses if DWARF version is 5 or more. Add applicable base address if the
    entry is DW_LLE_OFFSET_PAIR from DWO.
    (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
    DEBUG_LOC_START_END in case of DW_LLE_offset_pair.
---
 gdb/dwarf2/loc.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 5155cff60d..cd8a6d33b0 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -92,6 +92,11 @@ enum debug_loc_kind
      as in .debug_loc.  */
   DEBUG_LOC_START_LENGTH = 3,
 
+   /* This is followed by two unsigned LEB128 operands. The values of these
+      operands are the starting and ending offsets, respectively, relative to
+      the applicable base address. */
+  DEBUG_LOC_OFFSET_PAIR = 4,
+
   /* An internal value indicating there is insufficient data.  */
   DEBUG_LOC_BUFFER_OVERFLOW = -1,
 
@@ -332,7 +337,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->from_dwo)
+      if (baton->per_cu->version () < 5 && baton->from_dwo)
 	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
 					       loc_ptr, buf_end, &new_ptr,
 					       &low, &high, byte_order);
@@ -358,6 +363,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  continue;
 	case DEBUG_LOC_START_END:
 	case DEBUG_LOC_START_LENGTH:
+	case DEBUG_LOC_OFFSET_PAIR:
 	  break;
 	case DEBUG_LOC_BUFFER_OVERFLOW:
 	case DEBUG_LOC_INVALID_ENTRY:
@@ -369,9 +375,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 
       /* Otherwise, a location expression entry.
 	 If the entry is from a DWO, don't add base address: the entry is from
-	 .debug_addr which already has the DWARF "base address".  We still add
-	 base_offset in case we're debugging a PIE executable.  */
-      if (baton->from_dwo)
+	 .debug_addr which already has the DWARF "base address". We still add
+	 base_offset in case we're debugging a PIE executable. However, if the
+	 entry is DW_LLE_offset_pair from a DWO, add the base address as the
+	 operands are offsets relative to the applicable base address. */
+      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
 	{
 	  low += base_offset;
 	  high += base_offset;
-- 
2.17.1

[PATCH] DWARFv5: Handle location list for split dwarf.

[nitachra]

>Could you please provide a test snippet, along with the compiler version and command line you use, that generates this error?  Please also put it in the commit message so that anyone inspecting this commit later on can reproduce it.

>Finally, would you mind trying to send the upcoming patches using git-send-email?  It makes it easier to read and comment them.

Hi Simon,

Added the compiler verion, test snippet and the command line used in the commit messaage. However, this error cannot be reproduced right now. First the patch 
https://sourceware.org/pipermail/gdb-patches/2020-January/164841.html needs to go in which has the support for DW_FORM_loclistx.

---
GDB throws the error '<error reading variable: dwarf2_find_location_
expression: Corrupted DWARF expression.>' while printing the variable
value with executable file compiled with -gdwarf-5, -O1 and -gdwarf-split
flags. This patch fixes this error.

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 -gdwarf-5 flags. Used clang version 10.0.0. This is the test case used-

int main()
{
    int arr[] = {1,2,3};
    for(int i = 0; i<3; i++)
        printf("%d",arr[i]);
    return 0;
}

clang -gdwarf-5 -O1 -gsplit-dwarf test.c -o test.out
gdb test.out
gdb> start
gdb> step
gdb> p i
dwarf2_find_location_expression: Corrupted DWARF expression.

gdb/ChangeLog:

   *dwarf2loc.c (enum debug_loc_kind): Added a new kind DEBUG_LOC_OFFSET_PAIR.
    (dwarf2_find_location_expression): Call the function decode_debug_loclists_
    addresses if DWARF version is 5 or more. Add applicable base address if the
    entry is DW_LLE_OFFSET_PAIR from DWO.
    (decode_debug_loclists_addresses): Return DEBUG_LOC_OFFSET_PAIR instead of
    DEBUG_LOC_START_END in case of DW_LLE_offset_pair.
---
 gdb/dwarf2loc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 405b239ed4..34795b70f9 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -90,6 +90,11 @@ enum debug_loc_kind
      number that specifies the length, and then a normal location expression
      as in .debug_loc.  */
   DEBUG_LOC_START_LENGTH = 3,
+  
+  /* This is followed by two unsigned LEB128 operands. The values of these 
+     operands are the starting and ending offsets, respectively, relative to 
+     the applicable base address. */
+  DEBUG_LOC_OFFSET_PAIR = 4,
 
   /* An internal value indicating there is insufficient data.  */
   DEBUG_LOC_BUFFER_OVERFLOW = -1,
@@ -231,7 +236,7 @@ decode_debug_loclists_addresses (struct dwarf2_per_cu_data *per_cu,
 	return DEBUG_LOC_BUFFER_OVERFLOW;
       *high = u64;
       *new_ptr = loc_ptr;
-      return DEBUG_LOC_START_END;
+      return DEBUG_LOC_OFFSET_PAIR;
     /* Following cases are not supported yet.  */
     case DW_LLE_startx_endx:
     case DW_LLE_start_end:
@@ -331,7 +336,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
       enum debug_loc_kind kind;
       const gdb_byte *new_ptr = NULL; /* init for gcc -Wall */
 
-      if (baton->from_dwo)
+      if (dwarf2_version(baton->per_cu) < 5 && baton->from_dwo)
 	kind = decode_debug_loc_dwo_addresses (baton->per_cu,
 					       loc_ptr, buf_end, &new_ptr,
 					       &low, &high, byte_order);
@@ -357,6 +362,7 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 	  continue;
 	case DEBUG_LOC_START_END:
 	case DEBUG_LOC_START_LENGTH:
+	case DEBUG_LOC_OFFSET_PAIR:
 	  break;
 	case DEBUG_LOC_BUFFER_OVERFLOW:
 	case DEBUG_LOC_INVALID_ENTRY:
@@ -368,9 +374,11 @@ dwarf2_find_location_expression (struct dwarf2_loclist_baton *baton,
 
       /* Otherwise, a location expression entry.
 	 If the entry is from a DWO, don't add base address: the entry is from
-	 .debug_addr which already has the DWARF "base address".  We still add
-	 base_offset in case we're debugging a PIE executable.  */
-      if (baton->from_dwo)
+	 .debug_addr which already has the DWARF "base address". We still add
+	 base_offset in case we're debugging a PIE executable. However, if the 
+	 entry is DW_LLE_offset_pair from a DWO, add the base address as the 
+	 operands are offsets relative to the applicable base address.  */
+      if (baton->from_dwo && kind != DEBUG_LOC_OFFSET_PAIR)
 	{
 	  low += base_offset;
 	  high += base_offset;
-- 
2.17.1