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

Jan Kratochvil
2012-08-26T08:12:44Z

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