Re: RFC: fix DW_AT_data_member_location buglet

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Fri, 06 May 2011 20:52:15 +0200, Tom Tromey wrote:
> +static int
> +handle_data_member_location (struct die_info *die, struct dwarf2_cu *cu,
> +			     int *offset)
> +{
> +  struct attribute *attr;
> +
> +  attr = dwarf2_attr (die, DW_AT_data_member_location, cu);
> +  if (attr != NULL)
> +    {
> +      *offset = 0;
> +
> +      /* Note that we do not check for a section offset first here.
> +	 This is because DW_AT_data_member_location is new in DWARF 4,
> +	 so if we see it, we can assume that a constant form is really
> +	 a constant and not a section offset.  */
> +      if (attr_form_is_constant (attr))
> +	*offset = dwarf2_get_attr_constant_value (attr, 0);

LONGEST stored to `int'

> +      else if (attr_form_is_section_offset (attr))
> +	dwarf2_complex_location_expr_complaint ();
> +      else if (attr_form_is_block (attr))
> +	*offset = decode_locdesc (DW_BLOCK (attr), cu);

CORE_ADDR stored to `int'

> +      else
> +	dwarf2_complex_location_expr_complaint ();
> +
> +      return 1;
> +    }
> +
> +  return 0;
> +}
> +
[...]
> @@ -7618,22 +7629,13 @@ read_common_block (struct die_info *die, struct dwarf2_cu *cu)
>        child_die = die->child;
>        while (child_die && child_die->tag)
>  	{
> +	  int offset;
> +
>  	  sym = new_symbol (child_die, NULL, cu);
> -	  attr = dwarf2_attr (child_die, DW_AT_data_member_location, cu);
> -	  if (sym != NULL && attr != NULL)
> +	  if (sym != NULL &&
> +	      handle_data_member_location (child_die, cu, &offset))
>  	    {
> -	      CORE_ADDR byte_offset = 0;

`CORE_ADDR' changed to `int'

> -
> -	      if (attr_form_is_section_offset (attr))
> -		dwarf2_complex_location_expr_complaint ();
> -	      else if (attr_form_is_constant (attr))
> -		byte_offset = dwarf2_get_attr_constant_value (attr, 0);
> -	      else if (attr_form_is_block (attr))
> -		byte_offset = decode_locdesc (DW_BLOCK (attr), cu);
> -	      else
> -		dwarf2_complex_location_expr_complaint ();
> -
> -	      SYMBOL_VALUE_ADDRESS (sym) = base + byte_offset;
> +	      SYMBOL_VALUE_ADDRESS (sym) = base + offset;

This ia a 64-bit compatibility regression.

In the SET_FIELD_BITPOS case it is a problem the `bitpos' field is already
only `int' but that should be fixed one day anyway.  I would prefer LONGEST
everywhere but sure GDB is far from 64-bit inferior compatibility.


Otherwise the patch looks OK to me.


Thanks,
Jan