Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5
@ 2021-07-25  7:22 Tom de Vries
  2021-07-26 13:49 ` Simon Marchi via Gdb-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2021-07-25  7:22 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Hi,

[ I've confused things by forgetting to add -gdwarf-4 in $subject of
commit 0057a7ee0d9 "[gdb/testsuite] Add KFAILs for gdb.ada FAILs with
gcc-11".  So I'm adding here -gdwarf-5 in $subject, even though -gdwarf-5 is
the default for gcc-11.  I keep getting confused because of working with a
system gcc-11 compiler that was patched to switch the default back to
-gdwarf-4. ]

When running test-case gdb.ada/arrayptr.exp with gcc-11 (and default
-gdwarf-5), I run into:
...
(gdb) print pa_ptr.all^M
Unhandled dwarf expression opcode 0xff^M
(gdb) FAIL: gdb.ada/arrayptr.exp: scenario=all: print pa_ptr.all
...

What happens is that pa_ptr:
...
 <2><1523>: Abbrev Number: 3 (DW_TAG_variable)
    <1524>   DW_AT_name        : pa_ptr
    <1529>   DW_AT_type        : <0x14fa>
...
has type:
...
 <2><14fa>: Abbrev Number: 2 (DW_TAG_typedef)
    <14fb>   DW_AT_name        : foo__packed_array_ptr
    <1500>   DW_AT_type        : <0x1504>
 <2><1504>: Abbrev Number: 4 (DW_TAG_pointer_type)
    <1505>   DW_AT_byte_size   : 8
    <1505>   DW_AT_type        : <0x1509>
...
which is a pointer to a subrange:
...
 <2><1509>: Abbrev Number: 12 (DW_TAG_subrange_type)
    <150a>   DW_AT_lower_bound : 0
    <150b>   DW_AT_upper_bound : 0x3fffffffffffffffff
    <151b>   DW_AT_name        : foo__packed_array
    <151f>   DW_AT_type        : <0x15cc>
    <1523>   DW_AT_artificial  : 1
 <1><15cc>: Abbrev Number: 5 (DW_TAG_base_type)
    <15cd>   DW_AT_byte_size   : 16
    <15ce>   DW_AT_encoding    : 7      (unsigned)
    <15cf>   DW_AT_name        : long_long_long_unsigned
    <15d3>   DW_AT_artificial  : 1
...
with upper bound of form DW_FORM_data16.

In gdb/dwarf/attribute.h we have:
...
  /* Return non-zero if ATTR's value falls in the 'constant' class, or
     zero otherwise.  When this function returns true, you can apply
     the constant_value method to it.
     ...
     DW_FORM_data16 is not considered as constant_value cannot handle
     that.  */
  bool form_is_constant () const;
...
so instead we have attribute::form_is_block (DW_FORM_data16) == true.

Then in attr_to_dynamic_prop for the upper bound, we get a PROC_LOCEXPR
instead of a PROP_CONST and end up trying to evaluate the constant
0x3fffffffffffffffff as if it were a locexpr, which causes the
"Unhandled dwarf expression opcode 0xff".

In contrast, with -gdwarf-4 we have:
...
    <164c>   DW_AT_upper_bound : 18 byte block: \
      9e 10 ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 \
      (DW_OP_implicit_value 16 byte block: \
        ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 )
...

Fix the dwarf error by translating the DW_FORM_data16 constant into a
PROC_LOCEXPR, effectively by prepending 0x9e 0x10, such that we have same
result as with -gdwarf-4:
...
(gdb) print pa_ptr.all^M
That operation is not available on integers of more than 8 bytes.^M
(gdb) KFAIL: gdb.ada/arrayptr.exp: scenario=all: print pa_ptr.all \
  (PRMS: gdb/20991)
...

Tested on x86_64-linux, with gcc-11 and target board
unix/gdb:debug_flags=-gdwarf-5.

I'd like to commit this to master and then backport to gdb-11-branch.

Any comments?

Thanks,
- Tom

[gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5

gdb/ChangeLog:

2021-07-25  Tom de Vries  <tdevries@suse.de>

	* dwarf2/read.c (attr_to_dynamic_prop): Handle DW_FORM_data16.

---
 gdb/dwarf2/read.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 029b8bfad04..6f1b453ef45 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18254,7 +18254,22 @@ attr_to_dynamic_prop (const struct attribute *attr, struct die_info *die,
       baton->locexpr.per_cu = cu->per_cu;
       baton->locexpr.per_objfile = per_objfile;
 
-      struct dwarf_block *block = attr->as_block ();
+      struct dwarf_block *block;
+      if (attr->form == DW_FORM_data16)
+	{
+	  size_t data_size = 16;
+	  block = XOBNEW (obstack, struct dwarf_block);
+	  block->size = (data_size
+			 + 2 /* Extra bytes for DW_OP and arg.  */);
+	  gdb_byte *data = XOBNEWVEC (obstack, gdb_byte, block->size);
+	  data[0] = DW_OP_implicit_value;
+	  data[1] = data_size;
+	  memcpy (&data[2], attr->as_block ()->data, data_size);
+	  block->data = data;
+	}
+      else
+	block = attr->as_block ();
+
       baton->locexpr.size = block->size;
       baton->locexpr.data = block->data;
       switch (attr->name)

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

* Re: [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5
  2021-07-25  7:22 [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5 Tom de Vries
@ 2021-07-26 13:49 ` Simon Marchi via Gdb-patches
  2021-07-26 14:41   ` Tom de Vries
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Marchi via Gdb-patches @ 2021-07-26 13:49 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches; +Cc: Tom Tromey

On 2021-07-25 3:22 a.m., Tom de Vries wrote:
> Hi,
> 
> [ I've confused things by forgetting to add -gdwarf-4 in $subject of
> commit 0057a7ee0d9 "[gdb/testsuite] Add KFAILs for gdb.ada FAILs with
> gcc-11".  So I'm adding here -gdwarf-5 in $subject, even though -gdwarf-5 is
> the default for gcc-11.  I keep getting confused because of working with a
> system gcc-11 compiler that was patched to switch the default back to
> -gdwarf-4. ]
> 
> When running test-case gdb.ada/arrayptr.exp with gcc-11 (and default
> -gdwarf-5), I run into:
> ...
> (gdb) print pa_ptr.all^M
> Unhandled dwarf expression opcode 0xff^M
> (gdb) FAIL: gdb.ada/arrayptr.exp: scenario=all: print pa_ptr.all
> ...
> 
> What happens is that pa_ptr:
> ...
>  <2><1523>: Abbrev Number: 3 (DW_TAG_variable)
>     <1524>   DW_AT_name        : pa_ptr
>     <1529>   DW_AT_type        : <0x14fa>
> ...
> has type:
> ...
>  <2><14fa>: Abbrev Number: 2 (DW_TAG_typedef)
>     <14fb>   DW_AT_name        : foo__packed_array_ptr
>     <1500>   DW_AT_type        : <0x1504>
>  <2><1504>: Abbrev Number: 4 (DW_TAG_pointer_type)
>     <1505>   DW_AT_byte_size   : 8
>     <1505>   DW_AT_type        : <0x1509>
> ...
> which is a pointer to a subrange:
> ...
>  <2><1509>: Abbrev Number: 12 (DW_TAG_subrange_type)
>     <150a>   DW_AT_lower_bound : 0
>     <150b>   DW_AT_upper_bound : 0x3fffffffffffffffff
>     <151b>   DW_AT_name        : foo__packed_array
>     <151f>   DW_AT_type        : <0x15cc>
>     <1523>   DW_AT_artificial  : 1
>  <1><15cc>: Abbrev Number: 5 (DW_TAG_base_type)
>     <15cd>   DW_AT_byte_size   : 16
>     <15ce>   DW_AT_encoding    : 7      (unsigned)
>     <15cf>   DW_AT_name        : long_long_long_unsigned
>     <15d3>   DW_AT_artificial  : 1
> ...
> with upper bound of form DW_FORM_data16.
> 
> In gdb/dwarf/attribute.h we have:
> ...
>   /* Return non-zero if ATTR's value falls in the 'constant' class, or
>      zero otherwise.  When this function returns true, you can apply
>      the constant_value method to it.
>      ...
>      DW_FORM_data16 is not considered as constant_value cannot handle
>      that.  */
>   bool form_is_constant () const;
> ...
> so instead we have attribute::form_is_block (DW_FORM_data16) == true.
> 
> Then in attr_to_dynamic_prop for the upper bound, we get a PROC_LOCEXPR
> instead of a PROP_CONST and end up trying to evaluate the constant
> 0x3fffffffffffffffff as if it were a locexpr, which causes the
> "Unhandled dwarf expression opcode 0xff".
> 
> In contrast, with -gdwarf-4 we have:
> ...
>     <164c>   DW_AT_upper_bound : 18 byte block: \
>       9e 10 ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 \
>       (DW_OP_implicit_value 16 byte block: \
>         ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 )
> ...
> 
> Fix the dwarf error by translating the DW_FORM_data16 constant into a
> PROC_LOCEXPR, effectively by prepending 0x9e 0x10, such that we have same
> result as with -gdwarf-4:

Why is DW_FORM_data16 is handled as a block at the moment?

It just looks wrong that DW_FORM_data16 is treated as a block and not a
constant.  It would be more logical to have this end up as a constant
dynamic property, it would be more efficient than evaluating a location
expression.  Ah, but the const_val field is a LONGEST, we can't fit a 16
bytes number in there.  But we can encode that value as a location
expression, I see.

However, this high bounds value stored as a location expression won't be
very useful anyway.  In most places (see get_discrete_high_bound), we
just return 0 if the property is not constant.  But we did evaluate it,
the current interfaces that evaluate dynamic properties return CORE_ADDR
or LONGEST, all 64-bit values, so we could not return that value.  So if
the property that you create was ever evaluated, it wouldn't yield a
valid result anyway.  I quickly tried to find a way to make GDB evaluate
it to see what happens, but couldn't find one.

If we ever want such a large high bound value to be useful, I think that
some interfaces and some code would need to be converted to use
arbitrary precision integers (using GMP maybe).  And then
dynamic_prop_data::const_val could be a GMP type instead of a LONGEST,
allowing it to store that 16 bytes value.  In which case we would
probably undo your patch here, because, if we can store the 16-byte
value as a constant directly, there's no need to convert it to a
location expression.

Simon

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

* Re: [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5
  2021-07-26 13:49 ` Simon Marchi via Gdb-patches
@ 2021-07-26 14:41   ` Tom de Vries
  2021-07-26 15:55     ` Simon Marchi via Gdb-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Tom de Vries @ 2021-07-26 14:41 UTC (permalink / raw)
  To: Simon Marchi, gdb-patches; +Cc: Tom Tromey

On 7/26/21 3:49 PM, Simon Marchi wrote:
> On 2021-07-25 3:22 a.m., Tom de Vries wrote:
>> Hi,
>>
>> [ I've confused things by forgetting to add -gdwarf-4 in $subject of
>> commit 0057a7ee0d9 "[gdb/testsuite] Add KFAILs for gdb.ada FAILs with
>> gcc-11".  So I'm adding here -gdwarf-5 in $subject, even though -gdwarf-5 is
>> the default for gcc-11.  I keep getting confused because of working with a
>> system gcc-11 compiler that was patched to switch the default back to
>> -gdwarf-4. ]
>>
>> When running test-case gdb.ada/arrayptr.exp with gcc-11 (and default
>> -gdwarf-5), I run into:
>> ...
>> (gdb) print pa_ptr.all^M
>> Unhandled dwarf expression opcode 0xff^M
>> (gdb) FAIL: gdb.ada/arrayptr.exp: scenario=all: print pa_ptr.all
>> ...
>>
>> What happens is that pa_ptr:
>> ...
>>  <2><1523>: Abbrev Number: 3 (DW_TAG_variable)
>>     <1524>   DW_AT_name        : pa_ptr
>>     <1529>   DW_AT_type        : <0x14fa>
>> ...
>> has type:
>> ...
>>  <2><14fa>: Abbrev Number: 2 (DW_TAG_typedef)
>>     <14fb>   DW_AT_name        : foo__packed_array_ptr
>>     <1500>   DW_AT_type        : <0x1504>
>>  <2><1504>: Abbrev Number: 4 (DW_TAG_pointer_type)
>>     <1505>   DW_AT_byte_size   : 8
>>     <1505>   DW_AT_type        : <0x1509>
>> ...
>> which is a pointer to a subrange:
>> ...
>>  <2><1509>: Abbrev Number: 12 (DW_TAG_subrange_type)
>>     <150a>   DW_AT_lower_bound : 0
>>     <150b>   DW_AT_upper_bound : 0x3fffffffffffffffff
>>     <151b>   DW_AT_name        : foo__packed_array
>>     <151f>   DW_AT_type        : <0x15cc>
>>     <1523>   DW_AT_artificial  : 1
>>  <1><15cc>: Abbrev Number: 5 (DW_TAG_base_type)
>>     <15cd>   DW_AT_byte_size   : 16
>>     <15ce>   DW_AT_encoding    : 7      (unsigned)
>>     <15cf>   DW_AT_name        : long_long_long_unsigned
>>     <15d3>   DW_AT_artificial  : 1
>> ...
>> with upper bound of form DW_FORM_data16.
>>
>> In gdb/dwarf/attribute.h we have:
>> ...
>>   /* Return non-zero if ATTR's value falls in the 'constant' class, or
>>      zero otherwise.  When this function returns true, you can apply
>>      the constant_value method to it.
>>      ...
>>      DW_FORM_data16 is not considered as constant_value cannot handle
>>      that.  */
>>   bool form_is_constant () const;
>> ...
>> so instead we have attribute::form_is_block (DW_FORM_data16) == true.
>>
>> Then in attr_to_dynamic_prop for the upper bound, we get a PROC_LOCEXPR
>> instead of a PROP_CONST and end up trying to evaluate the constant
>> 0x3fffffffffffffffff as if it were a locexpr, which causes the
>> "Unhandled dwarf expression opcode 0xff".
>>
>> In contrast, with -gdwarf-4 we have:
>> ...
>>     <164c>   DW_AT_upper_bound : 18 byte block: \
>>       9e 10 ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 \
>>       (DW_OP_implicit_value 16 byte block: \
>>         ff ff ff ff ff ff ff ff 3f 0 0 0 0 0 0 0 )
>> ...
>>
>> Fix the dwarf error by translating the DW_FORM_data16 constant into a
>> PROC_LOCEXPR, effectively by prepending 0x9e 0x10, such that we have same
>> result as with -gdwarf-4:
> 
> Why is DW_FORM_data16 is handled as a block at the moment?
> 
> It just looks wrong that DW_FORM_data16 is treated as a block and not a
> constant.  It would be more logical to have this end up as a constant
> dynamic property, it would be more efficient than evaluating a location
> expression.  Ah, but the const_val field is a LONGEST, we can't fit a 16
> bytes number in there.  But we can encode that value as a location
> expression, I see.
> 

Indeed.  See PR20991.

> However, this high bounds value stored as a location expression won't be
> very useful anyway.  In most places (see get_discrete_high_bound), we
> just return 0 if the property is not constant.  But we did evaluate it,
> the current interfaces that evaluate dynamic properties return CORE_ADDR
> or LONGEST, all 64-bit values, so we could not return that value.  So if
> the property that you create was ever evaluated, it wouldn't yield a
> valid result anyway.  I quickly tried to find a way to make GDB evaluate
> it to see what happens, but couldn't find one.
> 
> If we ever want such a large high bound value to be useful, I think that
> some interfaces and some code would need to be converted to use
> arbitrary precision integers (using GMP maybe).  And then
> dynamic_prop_data::const_val could be a GMP type instead of a LONGEST,
> allowing it to store that 16 bytes value.  In which case we would
> probably undo your patch here, because, if we can store the 16-byte
> value as a constant directly, there's no need to convert it to a
> location expression.
> 

Yes, if we'd address PR20991 then this patch might be reverted.  I don't
see that as a problem.

What I see as a problem is that we currently give the user the confusing
"Unhandled dwarf expression opcode 0xff" which suggests either:
- there's a compiler problem, or
- gdb needs to handle the dwarf expression opcode 0xff,
and neither is correct.

With this patch, we give:
...
That operation is not available on integers of more than 8 bytes.
...
which points nicely to PR20991.

Thanks,
- Tom

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

* Re: [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5
  2021-07-26 14:41   ` Tom de Vries
@ 2021-07-26 15:55     ` Simon Marchi via Gdb-patches
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Marchi via Gdb-patches @ 2021-07-26 15:55 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches; +Cc: Tom Tromey

On 2021-07-26 10:41 a.m., Tom de Vries wrote:
>> However, this high bounds value stored as a location expression won't be
>> very useful anyway.  In most places (see get_discrete_high_bound), we
>> just return 0 if the property is not constant.  But we did evaluate it,
>> the current interfaces that evaluate dynamic properties return CORE_ADDR
>> or LONGEST, all 64-bit values, so we could not return that value.  So if
>> the property that you create was ever evaluated, it wouldn't yield a
>> valid result anyway.  I quickly tried to find a way to make GDB evaluate
>> it to see what happens, but couldn't find one.
>>
>> If we ever want such a large high bound value to be useful, I think that
>> some interfaces and some code would need to be converted to use
>> arbitrary precision integers (using GMP maybe).  And then
>> dynamic_prop_data::const_val could be a GMP type instead of a LONGEST,
>> allowing it to store that 16 bytes value.  In which case we would
>> probably undo your patch here, because, if we can store the 16-byte
>> value as a constant directly, there's no need to convert it to a
>> location expression.
>>
> 
> Yes, if we'd address PR20991 then this patch might be reverted.  I don't
> see that as a problem.
> 
> What I see as a problem is that we currently give the user the confusing
> "Unhandled dwarf expression opcode 0xff" which suggests either:
> - there's a compiler problem, or
> - gdb needs to handle the dwarf expression opcode 0xff,
> and neither is correct.
> 
> With this patch, we give:
> ...
> That operation is not available on integers of more than 8 bytes.
> ...
> which points nicely to PR20991.

I agree with the user-visible behavior / change, so even though the
implementation does not look pretty, I can live with it.

Simon

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

end of thread, other threads:[~2021-07-26 15:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-25  7:22 [PATCH][gdb/symtab] Fix unhandled dwarf expression opcode with gcc-11 -gdwarf-5 Tom de Vries
2021-07-26 13:49 ` Simon Marchi via Gdb-patches
2021-07-26 14:41   ` Tom de Vries
2021-07-26 15:55     ` Simon Marchi via Gdb-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox