Re: [RFA] dwarf2 reader: Clarify DW_FORM_flag/DW_FORM_flag_present

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>    Dwarf-2 specs say that 
Pierre> if an attribute ATTR is of form DW_FORM_flag,
Pierre> then it is present of DW_UNSND (&ATTR) is non zero
Pierre> and absent otherwise.
Pierre>   This patch modifies dwarf2_attr and dwarf2_no_follow
Pierre> to return NULL if an attribute is of form
Pierre> DW_FORM_flag but with value zero.
Pierre>   Dwarf-4 specifications say that for DW_FORM_flag_present
Pierre> the value of DW_UNSND is irrelevant, but that is not handled correctly
Pierre> in dwarf2_flag_true_p.
Pierre>   I added a error if the form of the attribute is
Pierre> not DW_FORM_flag nor DW_FORM_flag_present.

Pierre> @@ -7674,7 +7674,13 @@ dwarf2_attr (struct die_info *die, unsig
Pierre>    for (i = 0; i < die->num_attrs; ++i)
Pierre>      {
Pierre>        if (die->attrs[i].name == name)
Pierre> -	return &die->attrs[i];
Pierre> +	{
Pierre> +	  if (die->attrs[i].form == DW_FORM_flag
Pierre> +	      && DW_UNSND (&die->attrs[i]) == 0)
Pierre> +	    return NULL;
Pierre> +	  else
Pierre> +	    return &die->attrs[i];
Pierre> +	}

I'm ambivalent about this idea.  Perhaps potential users of this should
be using dwarf2_flag_true_p instead.

Pierre> @@ -7717,7 +7729,13 @@ dwarf2_flag_true_p (struct die_info *die
Pierre> -  return (attr && DW_UNSND (attr));
Pierre> +  if (!attr)
Pierre> +    return 0;
Pierre> +  if ((attr->form == DW_FORM_flag && DW_UNSND (attr))
Pierre> +      || attr->form == DW_FORM_flag_present)
Pierre> +    return 1;
Pierre> +  error (_("dwarf2_flag_true_p called for wrong DIE form %s"),
Pierre> +	 dwarf_form_name (attr->form));
Pierre>  }
 
I don't think this is needed.
IIUC, read_attribute_value already does the needed magic:

    case DW_FORM_flag_present:
      DW_UNSND (attr) = 1;
      break;

Also, I am not at all sure about this new error.  If it is invalid DWARF
then it should be a complaint.  If it is a bug in gdb, it should be
internal_error.

Tom