[patch] Flat DW_TAG_module support (PR fortran/9806)

[patch] Flat DW_TAG_module support (PR fortran/9806)

[Jan Kratochvil]

Hi,

gfortran-4.4 may look as having an regression - GCC PR debug/39073 - as it now
supports Fortran modules "namespaces" by wrapping the DIEs to DW_TAG_module.

GDB currently ignores DW_TAG_module and so the DIEs get completely lost.

This patch is not the real Fortran module support - it only merges the
namespaces as flat - but it fixes the "regression" of gfortran-4.4.

No GDB testsuite regressions on x86_64-unknown-linux-gnu.


Thanks,
Jan


gdb/
2009-02-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR fortran/9806
	* dwarf2read.c (scan_partial_symbols <DW_TAG_module)
	(process_die <DW_TAG_module>): New.

gdb/testsuite/
2009-02-07  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR fortran/9806
	* gdb.fortran/module.exp, gdb.fortran/module.f90: New.

--- gdb/dwarf2read.c	3 Jan 2009 05:57:51 -0000	1.292
+++ gdb/dwarf2read.c	7 Feb 2009 13:57:26 -0000
@@ -1851,6 +1851,11 @@ scan_partial_symbols (struct partial_die
 	    case DW_TAG_namespace:
 	      add_partial_namespace (pdi, lowpc, highpc, cu);
 	      break;
+	    case DW_TAG_module:
+	      /* FIXME: Support the separate Fortran module namespaces.  */
+	      if (pdi->has_children)
+		scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
+	      break;
 	    default:
 	      break;
 	    }
@@ -2822,6 +2827,19 @@ process_die (struct die_info *die, struc
       processing_has_namespace_info = 1;
       read_namespace (die, cu);
       break;
+    case DW_TAG_module:
+      /* FIXME: Support the separate Fortran module namespaces.  */
+      if (die->child != NULL)
+	{
+	  struct die_info *child_die = die->child;
+	  
+	  while (child_die && child_die->tag)
+	    {
+	      process_die (child_die, cu);
+	      child_die = sibling_die (child_die);
+	    }
+	}
+      break;
     case DW_TAG_imported_declaration:
     case DW_TAG_imported_module:
       /* FIXME: carlton/2002-10-16: Eventually, we should use the
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.fortran/module.exp	7 Feb 2009 13:57:29 -0000
@@ -0,0 +1,35 @@
+# Copyright 2009 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/>.
+
+set testfile "module"
+set srcfile ${testfile}.f90
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug f77 quiet}] != "" } {
+    untested "Couldn't compile ${srcfile}"
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto MAIN__] then {
+    perror "couldn't run to breakpoint MAIN__"
+    continue
+}
+
+gdb_test "print i" "\\$\[0-9\]+ = 42"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.fortran/module.f90	7 Feb 2009 13:57:29 -0000
@@ -0,0 +1,22 @@
+! Copyright 2009 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/>.
+
+module mod
+        integer :: i = 42
+end module mod
+
+        use mod
+        print *, i
+end

Re: [patch] Flat DW_TAG_module support (PR fortran/9806)

[Joel Brobecker]

> 2009-02-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	PR fortran/9806
> 	* dwarf2read.c (scan_partial_symbols <DW_TAG_module)
> 	(process_die <DW_TAG_module>): New.

Just a few minor requests...

> gdb/testsuite/
> 2009-02-07  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	PR fortran/9806
> 	* gdb.fortran/module.exp, gdb.fortran/module.f90: New.

This part looks OK to me, although I do have one suggestion.

> +	    case DW_TAG_module:
> +	      /* FIXME: Support the separate Fortran module namespaces.  */
> +	      if (pdi->has_children)
> +		scan_partial_symbols (pdi->die_child, lowpc, highpc, cu);
> +	      break;

To be consistent with the current approach, would you mind putting
this code into its own routine (add_partial_module), and then call
this routine from here. I know it's a little bit overkill given
the current limited implementation, but it'll set things up nicely
for when we do want to implement modules support.

> +    case DW_TAG_module:
> +      /* FIXME: Support the separate Fortran module namespaces.  */
> +      if (die->child != NULL)
> +	{
> +	  struct die_info *child_die = die->child;
> +	  
> +	  while (child_die && child_die->tag)
> +	    {
> +	      process_die (child_die, cu);
> +	      child_die = sibling_die (child_die);
> +	    }
> +	}

Same here: Can we put this in its own subprogram (read_module)?

> +gdb_test "print i" "\\$\[0-9\]+ = 42"

Here, your expected output is not incorrect. But you can also drop
the complex regexp that matches the "$NUM" part of the output if
you like:

    gdb_test "print i" " = 42"

This is a common practice in our testsuite.

-- 
Joel

Re: [patch] Flat DW_TAG_module support (PR fortran/9806)

[Jan Kratochvil]

On Wed, 11 Feb 2009 20:17:59 +0100, Joel Brobecker wrote:
> To be consistent with the current approach, would you mind putting
> this code into its own routine (add_partial_module), and then call
> this routine from here. I know it's a little bit overkill given
> the current limited implementation, but it'll set things up nicely
> for when we do want to implement modules support.

Done, IMO no need for so many reasons for moving a code.


> Same here: Can we put this in its own subprogram (read_module)?

Done.


> > +gdb_test "print i" "\\$\[0-9\]+ = 42"
...
>     gdb_test "print i" " = 42"
> 
> This is a common practice in our testsuite.

Done; the other way is also common in the CVS. ;-)


Assuming it got approved by your mail so going to check it in.


Thanks,
Jan



gdb/
2009-02-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR fortran/9806
	* dwarf2read.c (process_die <DW_TAG_module>, read_module)
	(scan_partial_symbols <DW_TAG_module>, add_partial_module): New.

gdb/testsuite/
2009-02-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	PR fortran/9806
	* gdb.fortran/module.exp, gdb.fortran/module.f90: New.

--- gdb/dwarf2read.c	9 Feb 2009 00:07:53 -0000	1.293
+++ gdb/dwarf2read.c	11 Feb 2009 20:09:44 -0000
@@ -774,6 +774,10 @@ static void add_partial_namespace (struc
 				   CORE_ADDR *lowpc, CORE_ADDR *highpc,
 				   int need_pc, struct dwarf2_cu *cu);
 
+static void add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
+				CORE_ADDR *highpc, int need_pc,
+				struct dwarf2_cu *cu);
+
 static void add_partial_enumeration (struct partial_die_info *enum_pdi,
 				     struct dwarf2_cu *cu);
 
@@ -956,6 +960,8 @@ static void read_common_block (struct di
 
 static void read_namespace (struct die_info *die, struct dwarf2_cu *);
 
+static void read_module (struct die_info *die, struct dwarf2_cu *cu);
+
 static const char *namespace_name (struct die_info *die,
 				   int *is_anonymous, struct dwarf2_cu *);
 
@@ -1857,6 +1863,9 @@ scan_partial_symbols (struct partial_die
 	    case DW_TAG_namespace:
 	      add_partial_namespace (pdi, lowpc, highpc, need_pc, cu);
 	      break;
+	    case DW_TAG_module:
+	      add_partial_module (pdi, lowpc, highpc, need_pc, cu);
+	      break;
 	    default:
 	      break;
 	    }
@@ -2176,6 +2185,20 @@ add_partial_namespace (struct partial_di
     scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
 }
 
+/* Read a partial die corresponding to a Fortran module.  */
+
+static void
+add_partial_module (struct partial_die_info *pdi, CORE_ADDR *lowpc,
+		    CORE_ADDR *highpc, int need_pc, struct dwarf2_cu *cu)
+{
+  /* Now scan partial symbols in that module.
+
+     FIXME: Support the separate Fortran module namespaces.  */
+
+  if (pdi->has_children)
+    scan_partial_symbols (pdi->die_child, lowpc, highpc, need_pc, cu);
+}
+
 /* Read a partial die corresponding to a subprogram and create a partial
    symbol for that subprogram.  When the CU language allows it, this
    routine also defines a partial symbol for each nested subprogram
@@ -2839,6 +2862,9 @@ process_die (struct die_info *die, struc
       processing_has_namespace_info = 1;
       read_namespace (die, cu);
       break;
+    case DW_TAG_module:
+      read_module (die, cu);
+      break;
     case DW_TAG_imported_declaration:
     case DW_TAG_imported_module:
       /* FIXME: carlton/2002-10-16: Eventually, we should use the
@@ -4748,6 +4774,22 @@ read_namespace (struct die_info *die, st
     }
 }
 
+/* Read a Fortran module.  */
+
+static void
+read_module (struct die_info *die, struct dwarf2_cu *cu)
+{
+  struct die_info *child_die = die->child;
+
+  /* FIXME: Support the separate Fortran module namespaces.  */
+
+  while (child_die && child_die->tag)
+    {
+      process_die (child_die, cu);
+      child_die = sibling_die (child_die);
+    }
+}
+
 /* Return the name of the namespace represented by DIE.  Set
    *IS_ANONYMOUS to tell whether or not the namespace is an anonymous
    namespace.  */
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.fortran/module.exp	11 Feb 2009 20:09:49 -0000
@@ -0,0 +1,35 @@
+# Copyright 2009 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/>.
+
+set testfile "module"
+set srcfile ${testfile}.f90
+set binfile ${objdir}/${subdir}/${testfile}
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug f77 quiet}] != "" } {
+    untested "Couldn't compile ${srcfile}"
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto MAIN__] then {
+    perror "couldn't run to breakpoint MAIN__"
+    continue
+}
+
+gdb_test "print i" " = 42"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.fortran/module.f90	11 Feb 2009 20:09:49 -0000
@@ -0,0 +1,22 @@
+! Copyright 2009 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/>.
+
+module mod
+        integer :: i = 42
+end module mod
+
+        use mod
+        print *, i
+end