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

[Tom de Vries <tdevries@suse.de>]

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