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

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

[RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute

[Pierre Muller]

  So, here is a new version of this patch.

  I will  try to improve reference type handling inside the
pascal specific sources later.

  Does that look OK?

Pierre

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

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

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 22:29:04 -0000
@@ -8662,6 +8662,15 @@ 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 = lookup_reference_type (SYMBOL_TYPE (sym));
+             SYMBOL_TYPE (sym) = ref_type;
+           }
+
          add_symbol_to_list (sym, cu->list_in_scope);
          break;
        case DW_TAG_unspecified_parameters:

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé : Thursday, May 20, 2010 12:09 AM
> À : Pierre Muller
> Cc : 'Joel Brobecker'; gdb-patches@sourceware.org; 'FPC Core Developer
> List'
> Objet : Re: [RFA] dwarf debug format: Support DW_AT_variable_parameter
> attribute
> 
> >>>>> "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

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

[Tom Tromey]

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

Pierre>   Does that look OK?

Pierre> 2010-05-20  Pierre Muller  <muller@ics.u-strasbg.fr>
Pierre> 	* dwarf2read.c (new_symbol): Handle DW_AT_variable_parameter
Pierre> 	attribute.

This is ok.

I think it would be improved by a test case, but that is up to you.

Tom

RE: [RFA-v2] 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 Tom Tromey
> Envoyé : Friday, May 21, 2010 7:20 PM
> À : Pierre Muller
> Cc : 'Joel Brobecker'; gdb-patches@sourceware.org; 'FPC Core Developer
> List'
> Objet : Re: [RFA-v2] dwarf debug format: Support
> DW_AT_variable_parameter attribute
> 
> >>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr>
> writes:
> 
> Pierre>   Does that look OK?
> 
> Pierre> 2010-05-20  Pierre Muller  <muller@ics.u-strasbg.fr>
> Pierre> 	* dwarf2read.c (new_symbol): Handle
> DW_AT_variable_parameter
> Pierre> 	attribute.
> 
> This is ok.

  Thanks, I committed the patch.

 
> I think it would be improved by a test case, but that is up to you.

  This test would be pascal specific anyhow,
I will try to write one later.

Pierre

[new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]

[Jan Kratochvil]

On Fri, 21 May 2010 22:45:35 +0200, Pierre Muller wrote:
>   Thanks, I committed the patch.

commit 570d5d01de35effada277a69552c0903a4928241
Author: Pierre Muller <muller@ics.u-strasbg.fr>
Date:   Fri May 21 20:45:18 2010 +0000

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


Please revert this patch.

From the DWARF-4 description of DW_AT_variable_parameter I do not think it
changes the data structures meaning in any way.  It just states which data can
be modified by whom.  I do not think this is useful for GDB in any way.


> > I think it would be improved by a test case, but that is up to you.
> 
>   This test would be pascal specific anyhow,

I believe the Pascal compiler should generate different debug info.  For the
`var' keyword you can either (a) use DW_TAG_reference_type and point
DW_AT_location at the pointer or (b) use DW_OP_deref (as iFort does) on the
passed pointer and point DW_AT_location at the value itself.
DW_AT_variable_parameter would be true only for the (a) choice.

1	  subroutine f (i)
2	  integer i
3	  i = 2
4	  end
5	  program varparam
6	  integer m
7	  m = 1
8	  call f (m)
9	  write(*,*) m
10	  end

for iFort 11.1 20100806 it regressed this code, GDB-7.1 displayed:

Breakpoint 1, f (i=1) at varparam.f90:3
3	  i = 2
(gdb) p i
$1 = 1

while GDB-7.2 displays:

Breakpoint 2, f (i=@0x1) at varparam.f90:3
3	  i = 2
(gdb) p i
$1 = (REF TO -> ( INTEGER(4) )) @0x1: <error reading variable>

(note that `0x1' GDB considers as an address here)



A new testcase for the iFort usage case is attached.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.

I will then check-in the testcase or you can or so.


Regards,
Jan


gdb/
2010-11-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Revert:
	2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
	* dwarf2read.c (new_symbol): Handle DW_AT_variable_parameter
	attribute.

gdb/testsuite/
2010-11-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dw2-ifort-parameter-debug.S: New file.
	* dw2-ifort-parameter.c: New file.
	* dw2-ifort-parameter.exp: New file.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10801,14 +10801,6 @@ new_symbol_full (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    {
 	      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 = lookup_reference_type (SYMBOL_TYPE (sym));
-	      SYMBOL_TYPE (sym) = ref_type;
-	    }
 
 	  list_to_add = cu->list_in_scope;
 	  break;
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
@@ -0,0 +1,122 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev: DW_TAG_compile_unit */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	1				/* DW_AT_language (C) */
+	.4byte	func				/* DW_AT_low_pc */
+	.4byte	main				/* DW_AT_high_pc */
+
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram */
+	.byte		1			/* DW_AT_external */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		func			/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.uleb128	3			/* Abbrev: DW_TAG_formal_parameter */
+	.ascii		"param\0"		/* DW_AT_name */
+	.byte		1			/* DW_AT_variable_parameter */
+	.4byte		.Ltype_int - .Lcu1_begin	/* DW_AT_type */
+	.byte		2f - 1f			/* DW_AT_location */
+1:	.byte		3			/*   DW_OP_addr */
+	.4byte		ptr			/*   <addr> */
+	.byte		0x6			/*   DW_OP_deref */
+2:
+
+	.byte		0			/* End of children of func */
+
+.Ltype_int:
+	.uleb128	4			/* Abbrev: DW_TAG_base_type */
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		1			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x05			/* DW_TAG_formal_parameter */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x4b			/* DW_AT_variable_parameter */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x2			/* DW_AT_location */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int value = 0xdeadf00d;
+int *ptr = &value;
+
+void
+func (void)
+{
+}
+
+int
+main (void)
+{
+  func ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.exp
@@ -0,0 +1,40 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Verify DW_AT_variable_parameter does not modify the data structures meaning
+# for GDB, as being used by iFort.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0
+}
+
+set testfile "dw2-ifort-parameter"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} [list ${testfile}.c ${testfile}-debug.S] {nodebug}] } {
+    return -1
+}
+
+if ![runto func] {
+    return -1
+}
+
+gdb_test "p/x param" " = 0xdeadf00d"

RE: [Core] [new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]

[Pierre Free Pascal]

  Rereading the dwarf4 specifications,
I agree with Jan that I misinterpreted 
this TAG and that it is only an information tag,
that could possibly be used as a warning notice
that we might modify caller variables.

  Please apply this patch,
I will try to correct Free Pascal code generation
to cope for this issue.

Pierre Muller
Pascal language maintainer of GDB
and core team member of Free Pascal Compiler.


-----Message d'origine-----
De : core-bounces@freepascal.org [mailto:core-bounces@freepascal.org] De la
part de Jan Kratochvil
Envoyé : jeudi 4 novembre 2010 06:32
À : Pierre Muller
Cc : tromey@redhat.com; 'FPC Core Developer List';
gdb-patches@sourceware.org; 'Joel Brobecker'
Objet : [Core] [new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2]
dwarf debug format: Support DW_AT_variable_parameter attribute]

On Fri, 21 May 2010 22:45:35 +0200, Pierre Muller wrote:
>   Thanks, I committed the patch.

commit 570d5d01de35effada277a69552c0903a4928241
Author: Pierre Muller <muller@ics.u-strasbg.fr>
Date:   Fri May 21 20:45:18 2010 +0000

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


Please revert this patch.

From the DWARF-4 description of DW_AT_variable_parameter I do not think it
changes the data structures meaning in any way.  It just states which data
can
be modified by whom.  I do not think this is useful for GDB in any way.


> > I think it would be improved by a test case, but that is up to you.
> 
>   This test would be pascal specific anyhow,

I believe the Pascal compiler should generate different debug info.  For the
`var' keyword you can either (a) use DW_TAG_reference_type and point
DW_AT_location at the pointer or (b) use DW_OP_deref (as iFort does) on the
passed pointer and point DW_AT_location at the value itself.
DW_AT_variable_parameter would be true only for the (a) choice.

1	  subroutine f (i)
2	  integer i
3	  i = 2
4	  end
5	  program varparam
6	  integer m
7	  m = 1
8	  call f (m)
9	  write(*,*) m
10	  end

for iFort 11.1 20100806 it regressed this code, GDB-7.1 displayed:

Breakpoint 1, f (i=1) at varparam.f90:3
3	  i = 2
(gdb) p i
$1 = 1

while GDB-7.2 displays:

Breakpoint 2, f (i=@0x1) at varparam.f90:3
3	  i = 2
(gdb) p i
$1 = (REF TO -> ( INTEGER(4) )) @0x1: <error reading variable>

(note that `0x1' GDB considers as an address here)



A new testcase for the iFort usage case is attached.

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.

I will then check-in the testcase or you can or so.


Regards,
Jan


gdb/
2010-11-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Revert:
	2010-05-21  Pierre Muller  <muller@ics.u-strasbg.fr>
	* dwarf2read.c (new_symbol): Handle DW_AT_variable_parameter
	attribute.

gdb/testsuite/
2010-11-04  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dw2-ifort-parameter-debug.S: New file.
	* dw2-ifort-parameter.c: New file.
	* dw2-ifort-parameter.exp: New file.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10801,14 +10801,6 @@ new_symbol_full (struct die_info *die, struct type
*type, struct dwarf2_cu *cu,
 	    {
 	      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 = lookup_reference_type (SYMBOL_TYPE (sym));
-	      SYMBOL_TYPE (sym) = ref_type;
-	    }
 
 	  list_to_add = cu->list_in_scope;
 	  break;
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter-debug.S
@@ -0,0 +1,122 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
*/
+
+	.section .debug_info
+.Lcu1_begin:
+	/* CU header */
+	.4byte	.Lcu1_end - .Lcu1_start		/* Length of Compilation
Unit */
+.Lcu1_start:
+	.2byte	2				/* DWARF Version */
+	.4byte	.Labbrev1_begin			/* Offset into abbrev
section */
+	.byte	4				/* Pointer size */
+
+	/* CU die */
+	.uleb128 1				/* Abbrev:
DW_TAG_compile_unit */
+	.ascii	"file1.txt\0"			/* DW_AT_name */
+	.ascii	"GNU C 3.3.3\0"			/* DW_AT_producer */
+	.byte	1				/* DW_AT_language (C) */
+	.4byte	func				/* DW_AT_low_pc */
+	.4byte	main				/* DW_AT_high_pc */
+
+	.uleb128	2			/* Abbrev: DW_TAG_subprogram
*/
+	.byte		1			/* DW_AT_external */
+	.ascii		"func\0"		/* DW_AT_name */
+	.4byte		func			/* DW_AT_low_pc */
+	.4byte		main			/* DW_AT_high_pc */
+
+	.uleb128	3			/* Abbrev:
DW_TAG_formal_parameter */
+	.ascii		"param\0"		/* DW_AT_name */
+	.byte		1			/* DW_AT_variable_parameter
*/
+	.4byte		.Ltype_int - .Lcu1_begin	/* DW_AT_type */
+	.byte		2f - 1f			/* DW_AT_location */
+1:	.byte		3			/*   DW_OP_addr */
+	.4byte		ptr			/*   <addr> */
+	.byte		0x6			/*   DW_OP_deref */
+2:
+
+	.byte		0			/* End of children of func
*/
+
+.Ltype_int:
+	.uleb128	4			/* Abbrev: DW_TAG_base_type
*/
+	.ascii		"int\0"			/* DW_AT_name */
+	.byte		4			/* DW_AT_byte_size */
+	.byte		5			/* DW_AT_encoding */
+
+	.byte		0			/* End of children of CU */
+.Lcu1_end:
+
+/* Abbrev table */
+	.section .debug_abbrev
+.Labbrev1_begin:
+	.uleb128	1			/* Abbrev code */
+	.uleb128	0x11			/* DW_TAG_compile_unit */
+	.byte		1			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x25			/* DW_AT_producer */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x13			/* DW_AT_language */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	2			/* Abbrev code */
+	.uleb128	0x2e			/* DW_TAG_subprogram */
+	.byte		1			/* has_children */
+	.uleb128	0x3f			/* DW_AT_external */
+	.uleb128	0xc			/* DW_FORM_flag */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x11			/* DW_AT_low_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.uleb128	0x12			/* DW_AT_high_pc */
+	.uleb128	0x1			/* DW_FORM_addr */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	3			/* Abbrev code */
+	.uleb128	0x05			/* DW_TAG_formal_parameter
*/
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0x4b			/* DW_AT_variable_parameter
*/
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x49			/* DW_AT_type */
+	.uleb128	0x13			/* DW_FORM_ref4 */
+	.uleb128	0x2			/* DW_AT_location */
+	.uleb128	0xa			/* DW_FORM_block1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.uleb128	4			/* Abbrev code */
+	.uleb128	0x24			/* DW_TAG_base_type */
+	.byte		0			/* has_children */
+	.uleb128	0x3			/* DW_AT_name */
+	.uleb128	0x8			/* DW_FORM_string */
+	.uleb128	0xb			/* DW_AT_byte_size */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.uleb128	0x3e			/* DW_AT_encoding */
+	.uleb128	0xb			/* DW_FORM_data1 */
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
+
+	.byte		0x0			/* Terminator */
+	.byte		0x0			/* Terminator */
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.c
@@ -0,0 +1,31 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
*/
+
+int value = 0xdeadf00d;
+int *ptr = &value;
+
+void
+func (void)
+{
+}
+
+int
+main (void)
+{
+  func ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-ifort-parameter.exp
@@ -0,0 +1,40 @@
+# Copyright 2010 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Verify DW_AT_variable_parameter does not modify the data structures
meaning
+# for GDB, as being used by iFort.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    return 0
+}
+
+set testfile "dw2-ifort-parameter"
+
+if { [prepare_for_testing ${testfile}.exp ${testfile} [list ${testfile}.c
${testfile}-debug.S] {nodebug}] } {
+    return -1
+}
+
+if ![runto func] {
+    return -1
+}
+
+gdb_test "p/x param" " = 0xdeadf00d"
_______________________________________________
core site list
core@freepascal.org
http://idefix.wisa.be/mailman/listinfo/core

Re: [Core] [new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]

[Joel Brobecker]

>   Please apply this patch,
> I will try to correct Free Pascal code generation
> to cope for this issue.

Should it be applied on 7.2 as well?

-- 
Joel

RE: [Core] [new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]

[Pierre Free Pascal]

  Yes, if it affect badly fortran debugging,
it should probably be applied to 7.2 branch 
as well.

Pierre

> -----Message d'origine-----
> De : Joel Brobecker [mailto:brobecker@adacore.com]
> Envoyé : jeudi 4 novembre 2010 19:01
> À : Pierre Free Pascal
> Cc : 'FPC Core Developer List'; gdb-patches@sourceware.org;
> tromey@redhat.com
> Objet : Re: [Core] [new testcase] Regression 7.1->7.2 for iFort [Re:
> [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter
> attribute]
> 
> >   Please apply this patch,
> > I will try to correct Free Pascal code generation
> > to cope for this issue.
> 
> Should it be applied on 7.2 as well?
> 
> --
> Joel

Re: [Core] [new testcase] Regression 7.1->7.2 for iFort [Re: [RFA-v2] dwarf debug format: Support DW_AT_variable_parameter attribute]

[Jan Kratochvil]

On Thu, 04 Nov 2010 09:46:22 +0100, Pierre Free Pascal wrote:
>   Rereading the dwarf4 specifications,
> I agree with Jan that I misinterpreted 
> this TAG and that it is only an information tag,
> that could possibly be used as a warning notice
> that we might modify caller variables.
> 
>   Please apply this patch,
> I will try to correct Free Pascal code generation
> to cope for this issue.

Checked in:
	http://sourceware.org/ml/gdb-cvs/2011-09/msg00003.html
	http://sourceware.org/ml/gdb-cvs/2011-09/msg00004.html

Somehow it got forgotten and referenced now by:
	Question about DW_AT_variable_parameter in dwarf2read.c
	http://sourceware.org/ml/gdb/2011-08/msg00138.html


Jan

[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: