Re: [3/3] RFC - optional patch to restore DW_AT_data_member_location

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Tue, 21 Aug 2012 18:37:33 +0200, Tom Tromey wrote:
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -11060,6 +11060,71 @@ read_set_type (struct die_info *die, struct dwarf2_cu *cu)
>    return set_die_type (die, set_type, cu);
>  }
>  
> +/* A helper for read_common_block that creates a locexpr baton.  */

I believe still the parameters should be documented in this case.

> +
> +static void
> +mark_common_block_symbol_computed (struct symbol *sym,
> +				   struct die_info *common_die,
> +				   struct attribute *common_loc,
> +				   struct attribute *member_loc,
> +				   struct dwarf2_cu *cu)
> +{
> +  struct objfile *objfile = dwarf2_per_objfile->objfile;
> +  struct dwarf2_locexpr_baton *baton;
> +  gdb_byte *ptr;
> +  unsigned int cu_off;
> +  enum bfd_endian byte_order = gdbarch_byte_order (get_objfile_arch (objfile));
> +  LONGEST offset;
> +
> +  gdb_assert (common_loc && member_loc);
> +  gdb_assert (attr_form_is_block (common_loc));
> +  gdb_assert (attr_form_is_block (member_loc)
> +	      || attr_form_is_constant (member_loc));
> +
> +  baton = obstack_alloc (&objfile->objfile_obstack,
> +			 sizeof (struct dwarf2_locexpr_baton));
> +  baton->per_cu = cu->per_cu;
> +  gdb_assert (baton->per_cu);
> +
> +  baton->size = 5 /* DW_OP_call4 */ + 1 /* DW_OP_plus */;
> +
> +  if (attr_form_is_constant (member_loc))
> +    {
> +      offset = dwarf2_get_attr_constant_value (member_loc, 0);
> +      baton->size += 1 /* DW_OP_addr */ + cu->header.addr_size;
> +    }
> +  else
> +    baton->size += DW_BLOCK (member_loc)->size;
> +
> +  ptr = obstack_alloc (&objfile->objfile_obstack, baton->size);
> +  baton->data = ptr;
> +
> +  *ptr++ = DW_OP_call4;
> +  cu_off = common_die->offset.sect_off - cu->per_cu->offset.sect_off;
> +  store_unsigned_integer (ptr, 4, byte_order, cu_off);
> +  ptr += 4;
> +
> +  if (attr_form_is_constant (member_loc))
> +    {
> +      *ptr++ = DW_OP_addr;
> +      store_unsigned_integer (ptr, cu->header.addr_size, byte_order, offset);
> +      ptr += cu->header.addr_size;
> +    }
> +  else
> +    {
> +      /* We have to copy the data here, because DW_OP_call4 will only
> +	 use a DW_AT_location attribute.  */
> +      memcpy (ptr, DW_BLOCK (member_loc)->data, DW_BLOCK (member_loc)->size);
> +      ptr += DW_BLOCK (member_loc)->size;
> +    }
> +
> +  *ptr = DW_OP_plus;

I think this code is complicted enough to be worth an assertion:
     *ptr++ = DW_OP_plus;
     gdb_assert (ptr - baton->data == baton->size);


> +
> +  SYMBOL_COMPUTED_OPS (sym) = &dwarf2_locexpr_funcs;
> +  SYMBOL_LOCATION_BATON (sym) = baton;
> +  SYMBOL_CLASS (sym) = LOC_COMPUTED;
> +}
> +
>  /* Create appropriate locally-scoped variables for all the
>     DW_TAG_common_block entries.  Also create a struct
>     fortran_common_block listing all such variables for `info common'.
[...]
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-common-block.S
[...]
> +.LASF13:
> +	.string	"GNU Fortran 4.6.3 20120306 (Red Hat 4.6.3-2) -mtune=generic -march=x86-64 -g -fintrinsic-modules-path /usr/lib/gcc/x86_64-redhat-linux/4.6.3/finclude"

Probably not needing to be rebuilt but I think for FSF GDB it would be better
to precompile such .S files using FSF GCC, which could be possibly (more
easily) reproduced in the future.


[...]
> +.LASF14:
> +	.string	"../gdb.fortran/common-block.f90"
[...]
> +.LASF15:
> +	.string	"/home/tromey/gnu/archer/archer/gdb/testsuite/gdb.dwarf2"

(gdb) l
48  ../gdb.fortran/common-block.f90: No such file or directory.

While with:
-       .string "/home/tromey/gnu/archer/archer/gdb/testsuite/gdb.dwarf2"
+       .string "gdb.dwarf2"

it would work:
(gdb) l
48     REAL*8               iz
[...]


> +.LASF6:
> +	.string	"real(kind=4)"
> +	.ident	"GCC: (GNU) 4.6.3 20120306 (Red Hat 4.6.3-2)"
> +	.section	.note.GNU-stack,"",@progbits


Thanks,
Jan