[RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Pierre Muller]

  Free Pascal compiler has support for
DW_AT_variable_parameter attribute, but this is disabled because
GDB does not support it yet.

  The value of the attribute must be a flag according to
DWARF specs. The only point I am unsure about is 
what does a zero value of this flag mean:
  Should zero be considered as FALSE and
thus the should say that this parameter is NOT a variable parameter,
or does the simple presence of this attribute condfirm
this it is a variable parameter, no matter the value of the flag.
I took the first approach here, but have the impression
that it is not always the case inside dwarf2read.c code.

  The following patch adds support for this
inside dwarf2read.c source.
  Tested on gcc16, no regression found.

  Is this OK?


Pierre Muller
Pascal language support maintainer for GDB

2010-05-12  Pierre Muller  <muller@ics.u-strasbg.fr>

	* dwarf2read.c (new_symbol): Handle DW_AT_variable_parameter
	attribute.

Index: src/gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.383
diff -u -p -r1.383 dwarf2read.c
--- src/gdb/dwarf2read.c	8 May 2010 04:58:45 -0000	1.383
+++ src/gdb/dwarf2read.c	12 May 2010 13:00:17 -0000
@@ -8630,6 +8630,10 @@ new_symbol (struct die_info *die, struct
 	    {
 	      dwarf2_const_value (attr, sym, cu);
 	    }
+	  attr = dwarf2_attr (die, DW_AT_variable_parameter, cu);
+	  if (attr && DW_UNSND (attr))
+	    SYMBOL_CLASS (sym) = LOC_REF_ARG;
+
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
 	case DW_TAG_unspecified_parameters:

Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Tom Tromey]

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

Pierre>   Free Pascal compiler has support for
Pierre> DW_AT_variable_parameter attribute, but this is disabled because
Pierre> GDB does not support it yet.

Pierre>   The value of the attribute must be a flag according to
Pierre> DWARF specs. The only point I am unsure about is 
Pierre> what does a zero value of this flag mean:
Pierre>   Should zero be considered as FALSE and
Pierre> thus the should say that this parameter is NOT a variable parameter,
Pierre> or does the simple presence of this attribute condfirm
Pierre> this it is a variable parameter, no matter the value of the flag.
Pierre> I took the first approach here, but have the impression
Pierre> that it is not always the case inside dwarf2read.c code.

I think you made the correct choice.

Pierre>   Is this OK?

I wonder whether it is possible for such a parameter to be marked as
LOC_COMPUTED or LOC_OPTIMIZED_OUT by var_decode_location.  If so then
changing the class to LOC_REF_ARG seems incorrect.

Tom

Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Joel Brobecker]

> Pierre>   Should zero be considered as FALSE and
> Pierre> thus the should say that this parameter is NOT a variable parameter,
> Pierre> or does the simple presence of this attribute condfirm
> Pierre> this it is a variable parameter, no matter the value of the flag.
> Pierre> I took the first approach here, but have the impression
> Pierre> that it is not always the case inside dwarf2read.c code.
> 
> I think you made the correct choice.

FWIW: I also double-checked the DWARF3 standard, and it says that the
value of this attribute is a flag, and following the hyperlinks in
the PDF, one gets to:

    flag
        A flag is represented as a single byte of data (DW_FORM_flag).
        If the flag has value zero, it indicates the absence of the
        attribute. If the flag has a non-zero value, it indicates the
        presence of the attribute.

-- 
Joel

RE: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Monday, May 17, 2010 7:28 PM
> À : Tom Tromey
> Cc : Pierre Muller; gdb-patches@sourceware.org
> Objet : Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter
> attribute
> 
> > Pierre>   Should zero be considered as FALSE and
> > Pierre> thus the should say that this parameter is NOT a variable
> parameter,
> > Pierre> or does the simple presence of this attribute condfirm
> > Pierre> this it is a variable parameter, no matter the value of the
> flag.
> > Pierre> I took the first approach here, but have the impression
> > Pierre> that it is not always the case inside dwarf2read.c code.
> >
> > I think you made the correct choice.
> 
> FWIW: I also double-checked the DWARF3 standard, and it says that the
> value of this attribute is a flag, and following the hyperlinks in
> the PDF, one gets to:
> 
>     flag
>         A flag is represented as a single byte of data (DW_FORM_flag).
>         If the flag has value zero, it indicates the absence of the
>         attribute. If the flag has a non-zero value, it indicates the
>         presence of the attribute.

So, may I commit this patch?

Pierre

Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Joel Brobecker]

> So, may I commit this patch?

As far as I can tell, there was a question from Tom that is still
unanswered...

-- 
Joel

RE: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Pierre Muller]

 Whoops,
you are right:

this was Tom's question:
>I wonder whether it is possible for such a parameter to be marked as
>LOC_COMPUTED or LOC_OPTIMIZED_OUT by var_decode_location.  If so then
>changing the class to LOC_REF_ARG seems incorrect.

  I looked a little more into the sources
and now understand that my patch is indeed
wrong :(
  I hoped that simple case like variables in registers
or at a fixed offset relative to stack would simply
be treated as in stabs and assign a LOC_REGISTER
or LOC_ARG SYMBOL_CLASS,
but there is no such conversion ...
See note line 8459 from dwarf2read.c source.
  /* NOTE drow/2002-01-30: It might be worthwhile to have a static
     expression evaluator, and use LOC_COMPUTED only when necessary
     (i.e. when the value of a register or memory location is
     referenced, or a thread-local block, etc.).  Then again, it might
     not be worthwhile.  I'm assuming that it isn't unless performance
     or memory numbers show me otherwise.  */

  I fear that my patch will have to wait until 
this static expression evaluator is implemented...

  Let's drop this patch for now.

  Thanks for the clever question, Tom!

Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Monday, May 17, 2010 7:47 PM
> À : Pierre Muller
> Cc : 'Tom Tromey'; gdb-patches@sourceware.org
> Objet : Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter
> attribute
> 
> > So, may I commit this patch?
> 
> As far as I can tell, there was a question from Tom that is still
> unanswered...
> 
> --
> Joel

Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Tom Tromey]

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

Pierre>   I looked a little more into the sources
Pierre> and now understand that my patch is indeed
Pierre> wrong :(

[...]

Pierre> See note line 8459 from dwarf2read.c source.
Pierre>   /* NOTE drow/2002-01-30: It might be worthwhile to have a static
Pierre>      expression evaluator, and use LOC_COMPUTED only when necessary
Pierre>      (i.e. when the value of a register or memory location is
Pierre>      referenced, or a thread-local block, etc.).  Then again, it might
Pierre>      not be worthwhile.  I'm assuming that it isn't unless performance
Pierre>      or memory numbers show me otherwise.  */

Pierre>   I fear that my patch will have to wait until 
Pierre> this static expression evaluator is implemented...

I'm not totally convinced that we would want such an evaluator.

Could you change the type of a parameter marked with
DW_AT_variable_parameter to be a reference type instead?  It seems to me
that this would give the same result, or nearly so.

Tom

RE: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Pierre Muller]

> Pierre>   I fear that my patch will have to wait until
> Pierre> this static expression evaluator is implemented...
> 
> I'm not totally convinced that we would want such an evaluator.
> 
> Could you change the type of a parameter marked with
> DW_AT_variable_parameter to be a reference type instead?  It seems to
> me
> that this would give the same result, or nearly so.

  That is what is done for now on Free Pascal,
i.e. the debug information generates a reference_type for
parameters passed by var.
  It's not ideal yet, but this is just a matter of
twicks inside p-* files...

  The following patch does make the same as
if the reference_type is created by Free Pascal.

  I am unsure about the type allocation procedure,
could you just tell me if this looks OK
before I resubmit an RFA.

Pierre 

$ cvs diff -u -p   dwarf2read.c
Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.386
diff -u -p -r1.386 dwarf2read.c
--- dwarf2read.c        17 May 2010 15:55:01 -0000      1.386
+++ dwarf2read.c        19 May 2010 20:52:42 -0000
@@ -8662,6 +8662,20 @@ new_symbol (struct die_info *die, struct
            {
              dwarf2_const_value (attr, sym, cu);
            }
+         attr = dwarf2_attr (die, DW_AT_variable_parameter, cu);
+         if (attr && DW_UNSND (attr))
+           {
+             struct type *ref_type;
+
+             ref_type = TYPE_REFERENCE_TYPE (SYMBOL_TYPE (sym));
+             if (!ref_type)
+               {
+                 ref_type = alloc_type (objfile);
+                 ref_type = make_reference_type (SYMBOL_TYPE (sym),
&ref_type);
+               }
+             SYMBOL_TYPE (sym) = ref_type;
+           }
+
          add_symbol_to_list (sym, cu->list_in_scope);
          break;
        case DW_TAG_unspecified_parameters:

Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter attribute

[Tom Tromey]

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

Pierre>   That is what is done for now on Free Pascal,
Pierre> i.e. the debug information generates a reference_type for
Pierre> parameters passed by var.
Pierre>   It's not ideal yet, but this is just a matter of
Pierre> twicks inside p-* files...

I suppose it is less than ideal in that the type is exposed to the user.

Another idea would be to augment the DWARF expression evaluator to do an
additional indirection when this bit is set.  Though then one must
wonder why the compiler does not simply emit an expression to that
effect.

Pierre>   I am unsure about the type allocation procedure,
Pierre> could you just tell me if this looks OK
Pierre> before I resubmit an RFA.

Pierre> +             struct type *ref_type;
Pierre> +
Pierre> +             ref_type = TYPE_REFERENCE_TYPE (SYMBOL_TYPE (sym));
Pierre> +             if (!ref_type)
Pierre> +               {
Pierre> +                 ref_type = alloc_type (objfile);
Pierre> +                 ref_type = make_reference_type (SYMBOL_TYPE (sym),
Pierre> &ref_type);
Pierre> +               }

Just use 'lookup_reference_type (SYMBOL_TYPE (sym))'.
That will handle the details for you.

Tom