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

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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