Re: [PATCH][debug] Handle references to skipped params in remap_ssa_name

[Tom de Vries <tdevries at suse dot de>]

On 07/06/2018 12:28 PM, Richard Biener wrote:
> On Thu, Jul 5, 2018 at 4:12 PM Tom de Vries <tdevries@suse.de> wrote:
>>
>> On 07/05/2018 01:39 PM, Richard Biener wrote:
>>> On Thu, Jul 5, 2018 at 1:25 PM Tom de Vries <tdevries@suse.de> wrote:
>>>>
>>>> [ was: Re: [testsuite/guality, committed] Prevent optimization of local in
>>>> vla-1.c ]
>>>>
>>>> On Wed, Jul 04, 2018 at 02:32:27PM +0200, Tom de Vries wrote:
>>>>> On 07/03/2018 11:05 AM, Tom de Vries wrote:
>>>>>> On 07/02/2018 10:16 AM, Jakub Jelinek wrote:
>>>>>>> On Mon, Jul 02, 2018 at 09:44:04AM +0200, Richard Biener wrote:
>>>>>>>> Given the array has size i + 1 it's upper bound should be 'i' and 'i'
>>>>>>>> should be available via DW_OP_[GNU_]entry_value.
>>>>>>>>
>>>>>>>> I see it is
>>>>>>>>
>>>>>>>>     <175>   DW_AT_upper_bound : 10 byte block: 75 1 8 20 24 8 20 26 31
>>>>>>>> 1c       (DW_OP_breg5 (rdi): 1; DW_OP_const1u: 32; DW_OP_shl;
>>>>>>>> DW_OP_const1u: 32; DW_OP_shra; DW_OP_lit1; DW_OP_minus)
>>>>>>>>
>>>>>>>> and %rdi is 1.  Not sure why gdb fails to print it's length.  Yes, the
>>>>>>>> storage itself doesn't have a location but the
>>>>>>>> type specifies the size.
>>>>>>>>
>>>>>>>> (gdb) ptype a
>>>>>>>> type = char [variable length]
>>>>>>>> (gdb) p sizeof(a)
>>>>>>>> $3 = 0
>>>>>>>>
>>>>>>>> this looks like a gdb bug to me?
>>>>>>>>
>>>>>>
>>>>>> With gdb patch:
>>>>>> ...
>>>>>> diff --git a/gdb/findvar.c b/gdb/findvar.c
>>>>>> index 8ad5e25cb2..ebaff923a1 100644
>>>>>> --- a/gdb/findvar.c
>>>>>> +++ b/gdb/findvar.c
>>>>>> @@ -789,6 +789,8 @@ default_read_var_value
>>>>>>        break;
>>>>>>
>>>>>>      case LOC_OPTIMIZED_OUT:
>>>>>> +      if (is_dynamic_type (type))
>>>>>> +       type = resolve_dynamic_type (type, NULL,
>>>>>> +                                    /* Unused address.  */ 0);
>>>>>>        return allocate_optimized_out_value (type);
>>>>>>
>>>>>>      default:
>>>>>> ...
>>>>>>
>>>>>> I get:
>>>>>> ...
>>>>>> $ ./gdb -batch -ex "b f1" -ex "r" -ex "p sizeof (a)" vla-1.exe
>>>>>> Breakpoint 1 at 0x4004a8: file vla-1.c, line 17.
>>>>>>
>>>>>> Breakpoint 1, f1 (i=i@entry=5) at vla-1.c:17
>>>>>> 17        return a[0];
>>>>>> $1 = 6
>>>>>> ...
>>>>>>
>>>>>
>>>>> Well, for -O1 and -O2.
>>>>>
>>>>> For O3, I get instead:
>>>>> ...
>>>>> $ ./gdb vla-1.exe -q -batch -ex "b f1" -ex "run" -ex "p sizeof (a)"
>>>>> Breakpoint 1 at 0x4004b0: f1. (2 locations)
>>>>>
>>>>> Breakpoint 1, f1 (i=5) at vla-1.c:17
>>>>> 17        return a[0];
>>>>> $1 = 0
>>>>> ...
>>>>>
>>>>
>>>> Hi,
>>>>
>>>> When compiling guality/vla-1.c with -O3 -g, vla 'a[i + 1]' in f1 is optimized
>>>> away, but f1 still contains a debug expression describing the upper bound of the
>>>> vla (D.1914):
>>>> ...
>>>>      __attribute__((noinline))
>>>>      f1 (intD.6 iD.1900)
>>>>      {
>>>>        <bb 2>
>>>>        saved_stack.1_2 = __builtin_stack_save ();
>>>>        # DEBUG BEGIN_STMT
>>>>        # DEBUG D#3 => i_1(D) + 1
>>>>        # DEBUG D#2 => (long intD.8) D#3
>>>>        # DEBUG D#1 => D#2 + -1
>>>>        # DEBUG D.1914 => (sizetype) D#1
>>>> ...
>>>>
>>>> Then f1 is cloned to a version f1.constprop with no parameters, eliminating
>>>> parameter i, and 'DEBUG D#3 => i_1(D) + 1' turns into 'D#3 => NULL'.
>>>> Consequently, 'print sizeof (a)' yields '0' in gdb.
>>>
>>> So does gdb correctly recognize there isn't any size available or do we somehow
>>> generate invalid debug info, not recognizing that D#3 => NULL means
>>> "optimized out" and thus all dependent expressions are "optimized out" as well?
>>>
>>> That is, shouldn't gdb do
>>>
>>> (gdb) print sizeof (a)
>>> <optimized out>
>>>
>>> ?
>>
>> The type for the vla gcc is emitting is an DW_TAG_array_type with
>> DW_TAG_subrange_type without DW_AT_upper_bound or DW_AT_count, which
>> makes the upper bound value 'unknown'. So I'd say the debug info is valid.
> 
> OK, that sounds reasonable.  I wonder if languages like Ada have a way
> to declare an array type with unknown upper bound but known lower bound.
> For
> 
> typedef int arr[];
> arr *x;
> 
> we generate just
> 
>  <1><2d>: Abbrev Number: 2 (DW_TAG_typedef)
>     <2e>   DW_AT_name        : arr
>     <32>   DW_AT_decl_file   : 1
>     <33>   DW_AT_decl_line   : 1
>     <34>   DW_AT_decl_column : 13
>     <35>   DW_AT_type        : <0x39>
>  <1><39>: Abbrev Number: 3 (DW_TAG_array_type)
>     <3a>   DW_AT_type        : <0x44>
>     <3e>   DW_AT_sibling     : <0x44>
>  <2><42>: Abbrev Number: 4 (DW_TAG_subrange_type)
>  <2><43>: Abbrev Number: 0
> 
> which does
> 
> (gdb) ptype arr
> type = int []
> (gdb) ptype x
> type = int (*)[]
> (gdb) p sizeof (arr)
> $1 = 0
> 
> so I wonder whether the patch makes it print <optimized out>
> instead?  I think both 0 and <optimized out> are less than ideal
> and maybe <variable size> would be better.  In the type case
> above it's certainly not "optimized out".
> 

I ran into trouble with the earlier posted gdb patch, in
gdb/testsuite/gdb.fortran/vla-sizeof.exp.

I've submitted a second try to gdb-patches ("[PATCH][exp] Interpret size
of vla with unknown size as <optimized out>" at
https://sourceware.org/ml/gdb-patches/2018-07/msg00541.html ). It
handles the zero-sized array in the C example above correctly.

Thanks,
- Tom