RE: [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for icc/ifort

["Kempke, Nils-Christian" <nils-christian.kempke@intel.com>]

Hi Simon,

Thanks for the review!

> -----Original Message-----
> From: Simon Marchi <simark@simark.ca>
> Sent: Monday, September 26, 2022 6:02 PM
> To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; gdb-
> patches@sourceware.org
> Cc: tom@tromey.com
> Subject: Re: [PATCH 3/4] gdb, typeprint: fix pointer/reference typeprint for
> icc/ifort
> 
> 
> 
> On 2022-09-20 03:26, Nils-Christian Kempke via Gdb-patches wrote:
> > Intel classic compilers (icc/icpc/ifort) for references and pointers
> > to arrays generate DWARF that looks like
> >
> >  <2><17d>: Abbrev Number: 22 (DW_TAG_variable)
> >     <17e>   DW_AT_decl_line   : 41
> >     <17f>   DW_AT_decl_file   : 1
> >     <180>   DW_AT_name        : (indirect string, offset: 0x1f1): vlaref
> >     <184>   DW_AT_type        : <0x214>
> >     <188>   DW_AT_location    : 2 byte block: 76 50
> >       (DW_OP_breg6 (rbp): -48)
> >  ...
> >  <1><214>: Abbrev Number: 12 (DW_TAG_reference_type)
> >     <215>   DW_AT_type        : <0x219>
> >  <1><219>: Abbrev Number: 27 (DW_TAG_array_type)
> >     <21a>   DW_AT_type        : <0x10e>
> >     <21e>   DW_AT_data_location: 2 byte block: 97 6
> >       (DW_OP_push_object_address; DW_OP_deref)
> >  <2><221>: Abbrev Number: 28 (DW_TAG_subrange_type)
> >     <222>   DW_AT_upper_bound : <0x154>
> >  <2><226>: Abbrev Number: 0
> >
> > (for pointers replace the DW_TAG_reference_type with a
> > DW_TAG_pointer_type).  This is, to my knowledge, allowed and corret
> DWARF
> > but posed a problem in GDB.  Usually, GDB would deal with gcc references
> > (or pointers) that look like
> >
> >  <2><96>: Abbrev Number: 8 (DW_TAG_variable)
> >     <97>   DW_AT_location    : 2 byte block: 91 50
> >       (DW_OP_fbreg: -48)
> >     <9a>   DW_AT_name        : (indirect string, offset: 0x2aa): vlaref
> >     <9e>   DW_AT_decl_file   : 3
> >     <9f>   DW_AT_decl_line   : 41
> >     <a0>   DW_AT_type        : <0x1de>
> >  ...
> >  <1><1de>: Abbrev Number: 17 (DW_TAG_reference_type)
> >     <1df>   DW_AT_type        : <0x1e3>
> >  <1><1e3>: Abbrev Number: 22 (DW_TAG_array_type)
> >     <1e4>   DW_AT_type        : <0x1bf>
> >  <2><1e8>: Abbrev Number: 23 (DW_TAG_subrange_type)
> >     <1e9>   DW_AT_type        : <0x1f2>
> >     <1ed>   DW_AT_count       : <0x8a>
> >  <2><1f1>: Abbrev Number: 0
> >
> > The DWARF above describes a reference with an address.  At the address
> > of this reference, so evaluating what lies at the DW_AT_location of the
> > DW_TAG_variable, we find the address we need to use to resolve the
> > array under the reference and print the array's values.
> > This is also exactly what GDB does per default when printing a
> > reference-to-array type.  It evaluates the reference, and then takes that
> > address to resolve the array (implicitly assuming that the value of the
> > reference coincides with the address of the array).
> >
> > The difference to the icc output is icc's usage of DW_AT_data_location.
> > This attribute can be used to specify a location for the actual data of
> > an array that is different from the object's address.  If it is present
> > in addition to a DW_AT_location (as is the case above) the array values
> > are actually located at whatever DW_AT_data_location is evaluated to
> > and not at the address DW_AT_location of the variable points to.
> >
> > When dealing with the icc output this posed a problem in GDB.  It would
> > still evaluate the reference.  It would then forget all about the reference
> > and use the address obtained that way to resolve the array type (which,
> > as mentioned, works fine with the gcc output).  It would then discover the
> > DW_AT_data_location attribute in the array's DIE and it would try to
> > resolve it.
> > Here is where GDB ran into a problem: according to DWARF5 and as seen in
> > this example as well, DW_AT_data_location usually starts with a
> > DW_OP_push_object_address.  This operation pushes the address of the
> > object currently being evaluated on the stack.  The object currently being
> > evaluated however in this case is the reference, not the
> > array.
> 
> This is where I block.  My understanding is that the
> DW_OP_push_object_address within the array's DIE always pushes the
> address of the array variable itself.  Regardless of where we are coming
> from.  Evaluating the reference involves evaluating the array, now the
> "current" object is the array.
> 
> This DWARF, that you quoted above:
> 
>   <1><219>: Abbrev Number: 27 (DW_TAG_array_type)
>      <21a>   DW_AT_type        : <0x10e>
>      <21e>   DW_AT_data_location: 2 byte block: 97 6
>        (DW_OP_push_object_address; DW_OP_deref)
> 
> Would make sense if the array variable itself was a descriptor whose
> first field is a pointer to the actual array data.  The DW_OP_deref
> would read that pointer and push it on the the stack.  But if no array
> descriptor is used (the array variable is the data directly), then
> either no DW_AT_data_location should be used, or a DW_AT_data_location
> consisting of only DW_OP_push_object_address should be used).
> 
> The DW_AT_data_location including a DW_OP_deref is not meant to deal
> with the fact that the array is accessed through another DIE that is a
> reference.
> 
> This is my understanding after reading section D.2.1 (Fortran Simple
> Array Example) in DWARF5.
> 
> Simon

Yes, so this part in the DWARF is also where I am quite unsure about what the
DWARF actually allows.  I agree that the DWARF icc/ifort emit is a bit weird.
And I also prefer the way gfortran/ifx emit Fortran/C pointers..
And I agree that the original thought behind DW_AT_data_location was different.

My main confusion is here:

looking at the example D.2.1, if I follow your argument, why would we then use
the address of arrayvar to evaluate the bounds? Would DW_OP_push_object_address
within the DW_TAG_subrange_type then not also need to push the subrange's address
(and indicate some offset to the address of its base type, the DW_TAG_subrange_type)?

The example uses the address of 'arrayvar'.  I find the example a bit confusing and
I do not clearly understand what the 'current object' is.  As we only have a chain of
DWARF DIE's I tend to assume that the innermost variable in the chain is meant.
Everything else seems not well defined, e.g., why assume that the DW_TAG_array_type
changes the 'current object' but the DW_TAG_subrange_type does not?

I get your point however - and I agree - if there was no pointer/reference construct
wrapped around the array type I would also assume that DW_AT_data_location looks
like you pointed out.

A bit more about this patch:
icc/ifort will likely not change their DWARF anymore (before they are EOLed).  I do find
the patch a bit hacky, but at least currently I feel like the DWARF is not technically
wrong - even though unusual.  Whether or not it should go into GDB - I don't know.  It is
on the more vital side for enabling pointer printing in icc/ifort (which elsewise would just
print non-sense).

From my point of view we can also drop it from this series.  It is tailored to icc/ifort and unless
some other compiler decides to emit the same (weird-ish) DWARF we will not need it in
GDB.  We just wanted to at least put it out there once.

Thanks again!
Nils
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928