public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v3 0/1] Fortran entry and DW_TAG_entry_point
@ 2022-07-07  8:36 Nils-Christian Kempke
  2022-07-07  8:36 ` [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point Nils-Christian Kempke
  0 siblings, 1 reply; 4+ messages in thread
From: Nils-Christian Kempke @ 2022-07-07  8:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: tom, jinisusan.george, kevinb, Nils-Christian Kempke

Hi,

please find attached v3 of this series.

v1 can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-March/186900.html

v2 can be found here:
https://sourceware.org/pipermail/gdb-patches/2022-April/187853.html

Since v2 I adapted the series to Tom's comment here
https://sourceware.org/pipermail/gdb-patches/2022-May/189479.html

Changes since v2:

  * In dwarf2/index-write.c (write_cooked_index): I added DW_TAG_entry_point
  to the GDB_INDEX_SYMBOL_KIND_FUNCTION when writing the GDB index.

  * In dwarf2/cooked-index.h (cooked_index_entry::matches): I added the
  DW_TAG_entry_point to the FUNCTIONS_DOMAIN which enabled me to break
  on entry points when compiled in a different CU than the one
  containing main (as was pointed out by Tom).

  * In testsuite/gdb.dwarf2/dw2-entry-points.exp: I added a second CU to
  the test DWARF containing an entry point which I used to test the
  changes in cooked-index.h

Cheers!
Nils

Nils-Christian Kempke (1):
  dwarf, fortran: add support for DW_TAG_entry_point

 gdb/dwarf2/abbrev.c                           |   1 +
 gdb/dwarf2/cooked-index.h                     |   3 +-
 gdb/dwarf2/index-write.c                      |   3 +-
 gdb/dwarf2/read.c                             |  72 +++++-
 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c   |  39 ++++
 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp | 215 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/entry-point.exp     |  84 +++++++
 gdb/testsuite/gdb.fortran/entry-point.f90     |  67 ++++++
 8 files changed, 481 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
 create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
 create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90

-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point
  2022-07-07  8:36 [PATCH v3 0/1] Fortran entry and DW_TAG_entry_point Nils-Christian Kempke
@ 2022-07-07  8:36 ` Nils-Christian Kempke
  2022-07-11 13:02   ` Andrew Burgess
  0 siblings, 1 reply; 4+ messages in thread
From: Nils-Christian Kempke @ 2022-07-07  8:36 UTC (permalink / raw)
  To: gdb-patches
  Cc: tom, jinisusan.george, kevinb, Nils-Christian Kempke,
	Bernhard Heckel, Tim Wiederhake

Fortran provides additional entry points for subroutines and functions.
These entry points may use only a subset (or a different set) of the
parameters of the original subroutine.  The entry points may be described
via the DWARF tag DW_TAG_entry_point.

This commit adds support for parsing the DW_TAG_entry_point DWARF tag.
Currently, between ifx/ifort/gfortran, only ifort is actually emitting
this tag.  Both, ifx and gfortran use the DW_TAG_subprogram tag as
workaround/alternative.  Thus, this patch really only adds more ifort
support.  Even so, some of the attached tests still fail for ifort, due
to some wrong line info generated for the entry points in ifort.

After this patch it is possible to set a breakpoint in gdb with the
ifort compiled example at the entry points 'foo' and 'foobar', which was not
possible before.

As gcc and ifx do not emit the tag I also added a test to gdb.dwarf2
which uses some underlying c compiled code and adds some Fortran style DWARF
to it emitting the DW_TAG_entry_point.  Before this patch it was not
possible to actually define breakpoint at the entry point tags.

For gfortran there actually exists a bug on bugzilla, asking for the use
of DW_TAG_entry_point over DW_TAG_subprogram:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37134

This patch was originally posted here

https://sourceware.org/legacy-ml/gdb-patches/2017-07/msg00317.html

but its review/pinging got lost after a while.  I reworked it to fit the
current GDB.

Co-authored-by: Bernhard Heckel <bernhard.heckel@intel.com>
Co-authored-by: Tim Wiederhake  <tim.wiederhake@intel.com>
---
 gdb/dwarf2/abbrev.c                           |   1 +
 gdb/dwarf2/cooked-index.h                     |   3 +-
 gdb/dwarf2/index-write.c                      |   3 +-
 gdb/dwarf2/read.c                             |  72 +++++-
 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c   |  39 ++++
 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp | 215 ++++++++++++++++++
 gdb/testsuite/gdb.fortran/entry-point.exp     |  84 +++++++
 gdb/testsuite/gdb.fortran/entry-point.f90     |  67 ++++++
 8 files changed, 481 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
 create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
 create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
 create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90

diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
index 4ca27eaa7e0..f9f87203e3e 100644
--- a/gdb/dwarf2/abbrev.c
+++ b/gdb/dwarf2/abbrev.c
@@ -88,6 +88,7 @@ tag_interesting_for_index (dwarf_tag tag)
     case DW_TAG_base_type:
     case DW_TAG_class_type:
     case DW_TAG_constant:
+    case DW_TAG_entry_point:
     case DW_TAG_enumeration_type:
     case DW_TAG_enumerator:
     case DW_TAG_imported_declaration:
diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
index 439cbb19fa7..e63d3001d2a 100644
--- a/gdb/dwarf2/cooked-index.h
+++ b/gdb/dwarf2/cooked-index.h
@@ -113,7 +113,8 @@ struct cooked_index_entry : public allocate_on_obstack
 		|| tag == DW_TAG_constant
 		|| tag == DW_TAG_enumerator);
       case FUNCTIONS_DOMAIN:
-	return tag == DW_TAG_subprogram;
+	return (tag == DW_TAG_subprogram
+		|| tag == DW_TAG_entry_point);
       case TYPES_DOMAIN:
 	return tag_is_type (tag);
       case MODULES_DOMAIN:
diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
index c3aad8dd999..a819fd49032 100644
--- a/gdb/dwarf2/index-write.c
+++ b/gdb/dwarf2/index-write.c
@@ -1128,7 +1128,8 @@ write_cooked_index (cooked_index_vector *table,
       const char *name = entry->full_name (&symtab->m_string_obstack);
 
       gdb_index_symbol_kind kind;
-      if (entry->tag == DW_TAG_subprogram)
+      if (entry->tag == DW_TAG_subprogram
+	  || entry->tag == DW_TAG_entry_point)
 	kind = GDB_INDEX_SYMBOL_KIND_FUNCTION;
       else if (entry->tag == DW_TAG_variable
 	       || entry->tag == DW_TAG_constant
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 23fe5679cbd..64a255c6487 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -8607,6 +8607,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
 	  && die->parent->tag == DW_TAG_subprogram)
 	cu->processing_has_namespace_info = true;
       /* Fall through.  */
+    case DW_TAG_entry_point:
     case DW_TAG_inlined_subroutine:
       read_func_scope (die, cu);
       break;
@@ -8723,6 +8724,7 @@ die_needs_namespace (struct die_info *die, struct dwarf2_cu *cu)
     case DW_TAG_enumerator:
     case DW_TAG_subprogram:
     case DW_TAG_inlined_subroutine:
+    case DW_TAG_entry_point:
     case DW_TAG_member:
     case DW_TAG_imported_declaration:
       return 1;
@@ -12994,6 +12996,53 @@ dwarf2_ranges_read_low_addrs (unsigned offset, struct dwarf2_cu *cu,
     });
 }
 
+/* Determine the low and high pc of a DW_TAG_entry_point.  */
+
+static pc_bounds_kind
+dwarf2_get_pc_bounds_entry_point (die_info *die, CORE_ADDR *lowpc,
+				  CORE_ADDR *highpc, dwarf2_cu *cu)
+{
+  CORE_ADDR low = 0;
+  CORE_ADDR high = 0;
+
+  if (die->parent->tag != DW_TAG_subprogram)
+    {
+      complaint (_("DW_TAG_entry_point not embedded in DW_TAG_subprogram"));
+      return PC_BOUNDS_INVALID;
+    }
+
+  /* A DW_TAG_entry_point is embedded in an subprogram.  Therefore, we can use
+     the highpc from its enveloping subprogram and get the lowpc from
+     DWARF.  */
+  const enum pc_bounds_kind bounds_kind = dwarf2_get_pc_bounds (die->parent,
+								&low, &high,
+								cu, nullptr,
+								nullptr);
+  if (bounds_kind == PC_BOUNDS_INVALID || bounds_kind == PC_BOUNDS_NOT_PRESENT)
+    return PC_BOUNDS_INVALID;
+
+  attribute *attr_low = dwarf2_attr (die, DW_AT_low_pc, cu);
+  if (!attr_low)
+    {
+      complaint (_("DW_TAG_entry_point is missing DW_AT_low_pc"));
+      return PC_BOUNDS_INVALID;
+    }
+  low = attr_low->as_address ();
+  if (high <= low)
+    return PC_BOUNDS_INVALID;
+  if (low == 0 && !cu->per_objfile->per_bfd->has_section_at_zero)
+    return PC_BOUNDS_INVALID;
+
+  if (lowpc)
+    *lowpc = low;
+  if (highpc)
+    *highpc = high;
+
+  /* Return PC_BOUNDS_RANGES/PC_BOUNDS_HIGH_LOW depending on the parent
+     die's bounds kind.  */
+  return bounds_kind;
+}
+
 /* Get low and high pc attributes from a die.  See enum pc_bounds_kind
    definition for the return value.  *LOWPC and *HIGHPC are set iff
    neither PC_BOUNDS_NOT_PRESENT nor PC_BOUNDS_INVALID are returned.  */
@@ -13010,6 +13059,9 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
   CORE_ADDR high = 0;
   enum pc_bounds_kind ret;
 
+  if (die->tag == DW_TAG_entry_point)
+    return dwarf2_get_pc_bounds_entry_point (die, lowpc, highpc, cu);
+
   attr_high = dwarf2_attr (die, DW_AT_high_pc, cu);
   if (attr_high)
     {
@@ -20760,6 +20812,20 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	  sym->set_domain (LABEL_DOMAIN);
 	  add_symbol_to_list (sym, cu->list_in_scope);
 	  break;
+	case DW_TAG_entry_point:
+	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
+	     finish_block.  */
+	  sym->set_aclass_index (LOC_BLOCK);
+	  /* DW_TAG_entry_point provides an additional entry_point to an
+	     existing sub_program.  Therefore, we inherit the "external"
+	     attribute from the sub_program to which the entry_point
+	     belongs to.  */
+	  attr2 = dwarf2_attr (die->parent, DW_AT_external, cu);
+	  if (attr2 != nullptr && attr2->as_boolean ())
+	    list_to_add = cu->get_builder ()->get_global_symbols ();
+	  else
+	    list_to_add = cu->list_in_scope;
+	  break;
 	case DW_TAG_subprogram:
 	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
 	     finish_block.  */
@@ -21464,6 +21530,7 @@ read_type_die_1 (struct die_info *die, struct dwarf2_cu *cu)
     case DW_TAG_enumeration_type:
       this_type = read_enumeration_type (die, cu);
       break;
+    case DW_TAG_entry_point:
     case DW_TAG_subprogram:
     case DW_TAG_subroutine_type:
     case DW_TAG_inlined_subroutine:
@@ -21783,12 +21850,15 @@ determine_prefix (struct die_info *die, struct dwarf2_cu *cu)
 	return "";
       case DW_TAG_subprogram:
 	/* Nested subroutines in Fortran get a prefix with the name
-	   of the parent's subroutine.  */
+	   of the parent's subroutine.  Entry points are prefixed by the
+	   parent's namespace.  */
 	if (cu->per_cu->lang () == language_fortran)
 	  {
 	    if ((die->tag ==  DW_TAG_subprogram)
 		&& (dwarf2_name (parent, cu) != NULL))
 	      return dwarf2_name (parent, cu);
+	    else if (die->tag == DW_TAG_entry_point)
+	      return determine_prefix (parent, cu);
 	  }
 	return "";
       case DW_TAG_enumeration_type:
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
new file mode 100644
index 00000000000..e8c99d1f52e
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
@@ -0,0 +1,39 @@
+/* Copyright 2022 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/>.  */
+
+/* In the generated DWARF, we'll use the locations of foo_entry_label and
+   foobar_entry_label as the low_pc's of our entry point TAGs.  */
+
+int I = 0;
+int J = 0;
+int K = 0;
+
+__attribute__((noinline))
+void bar_helper () {
+  __asm__("bar_helper_label: .globl bar_helper_label");
+  I++;
+  J++;
+  __asm__("foo_entry_label: .globl foo_entry_label");
+  J++;
+  K++;
+  __asm__("foobar_entry_label: .globl foobar_entry_label");
+}
+
+int main() {
+  __asm__("main_label: .globl main_label");
+  bar_helper ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
new file mode 100644
index 00000000000..1b5ae52fd8a
--- /dev/null
+++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
@@ -0,0 +1,215 @@
+# Copyright 2022 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/>.
+
+# Test that the DW_TAG_entry_point is handled properly by GDB and that we can
+# set breakpoints on function entry points.
+
+load_lib dwarf.exp
+
+# This test can only be run on targets that support DWARF-2 and use
+# gas.
+if {![dwarf2_support]} {
+    return 0
+}
+
+standard_testfile .c -dw.S
+
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
+	return -1
+}
+
+set int_size [get_sizeof "int" 4]
+
+# Make some DWARF for the test.
+set asm_file [standard_output_file $srcfile2]
+Dwarf::assemble $asm_file {
+    global srcfile
+    declare_labels int_label int2_label
+
+    get_func_info main
+    get_func_info bar_helper
+
+    set prog_line 1
+    set bar_line 2
+    set foo_line 3
+    set foobar_line 4
+
+    set global_I [gdb_target_symbol I]
+    set global_J [gdb_target_symbol J]
+    set global_K [gdb_target_symbol K]
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_Fortran90}
+	    {name dw2-entry-points.f90}
+	    {comp_dir /tmp}
+	} {
+	    int_label: base_type {
+		{name "int"}
+		{byte_size 4 sdata}
+		{encoding @DW_ATE_signed}
+	    }
+	    subprogram {
+		{name prog}
+		{decl_file 1 data1}
+		{decl_line $prog_line data1}
+		{low_pc $main_start addr}
+		{high_pc "$main_start + $main_len" addr}
+		{external 1 flag}
+		{main_subprogram 1 flag}
+	    }
+	    subprogram {
+		{name bar}
+		{decl_file 1 data1}
+		{decl_line $bar_line data1}
+		{external 1 flag}
+		{low_pc $bar_helper_start addr}
+		{high_pc "$bar_helper_start + $bar_helper_len" addr}
+	    } {
+		formal_parameter {
+		    {name I}
+		    {type :$int_label}
+		    {location {addr $global_I} SPECIAL_expr}
+		}
+		formal_parameter {
+		    {name J}
+		    {type :$int_label}
+		    {location {addr $global_J} SPECIAL_expr}
+		}
+		entry_point {
+		    {name foo}
+		    {decl_file 1 data1}
+		    {decl_line $foo_line data1}
+		    {low_pc foo_entry_label addr}
+		} {
+		    formal_parameter {
+			{name J}
+			{type :$int_label}
+			{location {addr $global_J} SPECIAL_expr}
+		    }
+		    formal_parameter {
+			{name K}
+			{type :$int_label}
+			{location {addr $global_K} SPECIAL_expr}
+		    }
+		}
+		entry_point {
+			{name foobar}
+			{decl_file 1 data1}
+			{decl_line $foobar_line data1}
+			{low_pc foobar_entry_label addr}
+		    } {
+		    formal_parameter {
+			{name J}
+			{type :$int_label}
+			{location {addr $global_J} SPECIAL_expr}
+		    }
+		}
+	    }
+	}
+    }
+
+    cu {} {
+	compile_unit {
+	    {language @DW_LANG_Fortran90}
+	    {name dw2-entry-points-2.f90}
+	    {comp_dir /tmp}
+	} {
+	    int2_label: base_type {
+		{name "int"}
+		{byte_size 4 sdata}
+		{encoding @DW_ATE_signed}
+	    }
+	    subprogram {
+		{name barso}
+		{decl_file 1 data1}
+		{decl_line $bar_line data1}
+		{external 1 flag}
+		{low_pc $bar_helper_start addr}
+		{high_pc "$bar_helper_start + $bar_helper_len" addr}
+	    } {
+		formal_parameter {
+		    {name I}
+		    {type :$int2_label}
+		    {location {addr $global_I} SPECIAL_expr}
+		}
+		formal_parameter {
+		    {name J}
+		    {type :$int2_label}
+		    {location {addr $global_J} SPECIAL_expr}
+		}
+		entry_point {
+		    {name fooso}
+		    {decl_file 1 data1}
+		    {decl_line $foo_line data1}
+		    {low_pc foo_entry_label addr}
+		} {
+		    formal_parameter {
+			{name J}
+			{type :$int2_label}
+			{location {addr $global_J} SPECIAL_expr}
+		    }
+		    formal_parameter {
+			{name K}
+			{type :$int2_label}
+			{location {addr $global_K} SPECIAL_expr}
+		    }
+		}
+	    }
+	}
+    }
+}
+
+if {[prepare_for_testing "failed to prepare" ${testfile} \
+	 [list $srcfile $asm_file] {nodebug}]} {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+# Try whether we can set and hit breakpoints at the entry_points.
+gdb_breakpoint "foo"
+gdb_breakpoint "foobar"
+
+# Now hit the entry_point break point and check their call-stack.
+gdb_continue_to_breakpoint "foo"
+gdb_test "bt" [multi_line \
+		   "#0.*${hex} in foo \\(J=1, K=0\\).*" \
+		   "#1.*${hex} in prog \\(\\).*" \
+    ] "bt foo"
+
+gdb_continue_to_breakpoint "foobar"
+gdb_test "bt" [multi_line \
+		   "#0.*${hex} in foobar \\(J=2\\).*" \
+		   "#1.*${hex} in prog \\(\\).*" \
+    ] "bt foobar"
+
+# Now try whether we can also set breakpoints on entry_points from other CUs.
+
+clean_restart ${testfile}
+
+if ![runto_main] {
+    return -1
+}
+
+gdb_breakpoint "fooso"
+gdb_continue_to_breakpoint "foo_so"
+
+gdb_test "bt" [multi_line \
+		    "#0.*${hex} in foo \\(J=1, K=0\\).*" \
+		    "#1.*${hex} in prog \\(\\).*" \
+] "bt fooso"
diff --git a/gdb/testsuite/gdb.fortran/entry-point.exp b/gdb/testsuite/gdb.fortran/entry-point.exp
new file mode 100644
index 00000000000..679191f79f8
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/entry-point.exp
@@ -0,0 +1,84 @@
+# Copyright 2022 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/>.
+#
+# Test Fortran entry points for subroutines.
+
+if { [skip_fortran_tests] } { return -1 }
+
+standard_testfile .f90
+load_lib "fortran.exp"
+
+if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug f90}] } {
+    return -1
+}
+
+if { ![fortran_runto_main] } {
+    untested "could not run to main"
+    return -1
+}
+
+# Test if we can set a breakpoint via the entry-point name.
+set entry_point_name "foo"
+gdb_breakpoint $entry_point_name
+gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
+    ".*entry foo\\(J,K,L,I1\\).*"
+
+gdb_test "print j" "= 11" "print j, entered via $entry_point_name"
+gdb_test "print k" "= 22" "print k, entered via $entry_point_name"
+gdb_test "print l" "= 33" "print l, entered via $entry_point_name"
+gdb_test "print i1" "= 44" "print i1, entered via $entry_point_name"
+gdb_test "info args" \
+    [multi_line "j = 11" \
+		"k = 22" \
+		"l = 33" \
+		"i1 = 44"] \
+    "info args, entered via $entry_point_name"
+
+# Test if we can set a breakpoint via the function name.
+set entry_point_name "bar"
+gdb_breakpoint $entry_point_name
+gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
+    ".*subroutine bar\\(I,J,K,I1\\).*"
+
+gdb_test "print i" "= 444" "print i, entered via $entry_point_name"
+gdb_test "print j" "= 555" "print j, entered via $entry_point_name"
+gdb_test "print k" "= 666" "print k, entered via $entry_point_name"
+gdb_test "print i1" "= 777" "print i1, entered via $entry_point_name"
+
+# Test a second entry point.
+set entry_point_name "foobar"
+gdb_breakpoint $entry_point_name
+gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
+    ".* entry foobar\\(J\\).*"
+
+gdb_test "print j" "= 1" "print j, entered via $entry_point_name"
+gdb_test "info args" "j = 1" "info args, entered via $entry_point_name"
+
+# Test breaking at the entrypoint defined inside the module mod via its
+# scoped name.
+set entry_point_name "mod::mod_foo"
+
+# GCC moves subroutines with entry points out of the module scope into the
+# compile unit scope.
+if {[test_compiler_info {gcc-*}]} {
+    setup_xfail "gcc/105272" *-*-*
+}
+gdb_breakpoint $entry_point_name
+
+if {[test_compiler_info {gcc-*}]} {
+    setup_xfail "gcc/105272" *-*-*
+}
+gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
+    ".* entry mod_foo\\(\\).*"
diff --git a/gdb/testsuite/gdb.fortran/entry-point.f90 b/gdb/testsuite/gdb.fortran/entry-point.f90
new file mode 100644
index 00000000000..12a0557e787
--- /dev/null
+++ b/gdb/testsuite/gdb.fortran/entry-point.f90
@@ -0,0 +1,67 @@
+! Copyright 2022 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
+implicit none
+
+contains
+  subroutine mod_bar
+    integer :: I = 3
+
+    goto 100
+
+    entry mod_foo
+    I = 33
+
+100 print *, I
+  end subroutine mod_bar
+end module mod
+
+
+subroutine bar(I,J,K,I1)
+  integer :: I,J,K,L,I1
+  integer :: A
+  real :: C
+
+  A = 0
+  C = 0.0
+
+  A = I + K + I1
+  goto 300
+
+  entry foo(J,K,L,I1)
+  A = J + K + L + I1
+
+200 C = J
+  goto 300
+
+  entry foobar(J)
+  goto 200
+
+300 A = C + 1
+  C = J * 1.5
+
+  return
+end subroutine
+
+program TestEntryPoint
+  use mod
+
+  call foo(11,22,33,44)
+  call bar(444,555,666,777)
+  call foobar(1)
+
+  call mod_foo()
+end program TestEntryPoint
-- 
2.25.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point
  2022-07-07  8:36 ` [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point Nils-Christian Kempke
@ 2022-07-11 13:02   ` Andrew Burgess
  2022-07-13  9:38     ` Kempke, Nils-Christian
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Burgess @ 2022-07-11 13:02 UTC (permalink / raw)
  To: Nils-Christian Kempke, gdb-patches
  Cc: Bernhard Heckel, Tim Wiederhake, jinisusan.george, tom

Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
writes:

> Fortran provides additional entry points for subroutines and functions.
> These entry points may use only a subset (or a different set) of the
> parameters of the original subroutine.  The entry points may be described
> via the DWARF tag DW_TAG_entry_point.
>
> This commit adds support for parsing the DW_TAG_entry_point DWARF tag.
> Currently, between ifx/ifort/gfortran, only ifort is actually emitting
> this tag.  Both, ifx and gfortran use the DW_TAG_subprogram tag as
> workaround/alternative.  Thus, this patch really only adds more ifort
> support.  Even so, some of the attached tests still fail for ifort, due
> to some wrong line info generated for the entry points in ifort.
>
> After this patch it is possible to set a breakpoint in gdb with the
> ifort compiled example at the entry points 'foo' and 'foobar', which was not
> possible before.
>
> As gcc and ifx do not emit the tag I also added a test to gdb.dwarf2
> which uses some underlying c compiled code and adds some Fortran style DWARF
> to it emitting the DW_TAG_entry_point.  Before this patch it was not
> possible to actually define breakpoint at the entry point tags.
>
> For gfortran there actually exists a bug on bugzilla, asking for the use
> of DW_TAG_entry_point over DW_TAG_subprogram:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37134
>
> This patch was originally posted here
>
> https://sourceware.org/legacy-ml/gdb-patches/2017-07/msg00317.html
>
> but its review/pinging got lost after a while.  I reworked it to fit the
> current GDB.

Thanks for the revisions.  I took a look through as I have an interest
in Fortran support, and I had some pretty minor feedback.  I'm no expert
on the DWARF indexes and CU expansion in GDB, but that side of things
looks reasonable to me - at least I'd be happy to debug this code if
anything came up later.

Anyway, my feedback is inline below.

Thanks,
Andrew

>
> Co-authored-by: Bernhard Heckel <bernhard.heckel@intel.com>
> Co-authored-by: Tim Wiederhake  <tim.wiederhake@intel.com>
> ---
>  gdb/dwarf2/abbrev.c                           |   1 +
>  gdb/dwarf2/cooked-index.h                     |   3 +-
>  gdb/dwarf2/index-write.c                      |   3 +-
>  gdb/dwarf2/read.c                             |  72 +++++-
>  gdb/testsuite/gdb.dwarf2/dw2-entry-points.c   |  39 ++++
>  gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp | 215 ++++++++++++++++++
>  gdb/testsuite/gdb.fortran/entry-point.exp     |  84 +++++++
>  gdb/testsuite/gdb.fortran/entry-point.f90     |  67 ++++++
>  8 files changed, 481 insertions(+), 3 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
>  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
>  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90
>
> diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
> index 4ca27eaa7e0..f9f87203e3e 100644
> --- a/gdb/dwarf2/abbrev.c
> +++ b/gdb/dwarf2/abbrev.c
> @@ -88,6 +88,7 @@ tag_interesting_for_index (dwarf_tag tag)
>      case DW_TAG_base_type:
>      case DW_TAG_class_type:
>      case DW_TAG_constant:
> +    case DW_TAG_entry_point:
>      case DW_TAG_enumeration_type:
>      case DW_TAG_enumerator:
>      case DW_TAG_imported_declaration:
> diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
> index 439cbb19fa7..e63d3001d2a 100644
> --- a/gdb/dwarf2/cooked-index.h
> +++ b/gdb/dwarf2/cooked-index.h
> @@ -113,7 +113,8 @@ struct cooked_index_entry : public allocate_on_obstack
>  		|| tag == DW_TAG_constant
>  		|| tag == DW_TAG_enumerator);
>        case FUNCTIONS_DOMAIN:
> -	return tag == DW_TAG_subprogram;
> +	return (tag == DW_TAG_subprogram
> +		|| tag == DW_TAG_entry_point);
>        case TYPES_DOMAIN:
>  	return tag_is_type (tag);
>        case MODULES_DOMAIN:
> diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
> index c3aad8dd999..a819fd49032 100644
> --- a/gdb/dwarf2/index-write.c
> +++ b/gdb/dwarf2/index-write.c
> @@ -1128,7 +1128,8 @@ write_cooked_index (cooked_index_vector *table,
>        const char *name = entry->full_name (&symtab->m_string_obstack);
>  
>        gdb_index_symbol_kind kind;
> -      if (entry->tag == DW_TAG_subprogram)
> +      if (entry->tag == DW_TAG_subprogram
> +	  || entry->tag == DW_TAG_entry_point)
>  	kind = GDB_INDEX_SYMBOL_KIND_FUNCTION;
>        else if (entry->tag == DW_TAG_variable
>  	       || entry->tag == DW_TAG_constant
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index 23fe5679cbd..64a255c6487 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -8607,6 +8607,7 @@ process_die (struct die_info *die, struct dwarf2_cu *cu)
>  	  && die->parent->tag == DW_TAG_subprogram)
>  	cu->processing_has_namespace_info = true;
>        /* Fall through.  */
> +    case DW_TAG_entry_point:
>      case DW_TAG_inlined_subroutine:
>        read_func_scope (die, cu);
>        break;
> @@ -8723,6 +8724,7 @@ die_needs_namespace (struct die_info *die, struct dwarf2_cu *cu)
>      case DW_TAG_enumerator:
>      case DW_TAG_subprogram:
>      case DW_TAG_inlined_subroutine:
> +    case DW_TAG_entry_point:
>      case DW_TAG_member:
>      case DW_TAG_imported_declaration:
>        return 1;
> @@ -12994,6 +12996,53 @@ dwarf2_ranges_read_low_addrs (unsigned offset, struct dwarf2_cu *cu,
>      });
>  }
>  
> +/* Determine the low and high pc of a DW_TAG_entry_point.  */
> +
> +static pc_bounds_kind
> +dwarf2_get_pc_bounds_entry_point (die_info *die, CORE_ADDR *lowpc,
> +				  CORE_ADDR *highpc, dwarf2_cu *cu)
> +{
> +  CORE_ADDR low = 0;
> +  CORE_ADDR high = 0;
> +
> +  if (die->parent->tag != DW_TAG_subprogram)
> +    {
> +      complaint (_("DW_TAG_entry_point not embedded in DW_TAG_subprogram"));
> +      return PC_BOUNDS_INVALID;
> +    }
> +
> +  /* A DW_TAG_entry_point is embedded in an subprogram.  Therefore, we can use
> +     the highpc from its enveloping subprogram and get the lowpc from
> +     DWARF.  */
> +  const enum pc_bounds_kind bounds_kind = dwarf2_get_pc_bounds (die->parent,
> +								&low, &high,
> +								cu, nullptr,
> +								nullptr);
> +  if (bounds_kind == PC_BOUNDS_INVALID || bounds_kind == PC_BOUNDS_NOT_PRESENT)
> +    return PC_BOUNDS_INVALID;

Is there a reason why PC_BOUNDS_NOT_PRESENT is transformed to
PC_BOUNDS_INVALID here?  I guess I would have expected 'return
bounds_kind' instead.

> +
> +  attribute *attr_low = dwarf2_attr (die, DW_AT_low_pc, cu);
> +  if (!attr_low)
> +    {
> +      complaint (_("DW_TAG_entry_point is missing DW_AT_low_pc"));
> +      return PC_BOUNDS_INVALID;
> +    }
> +  low = attr_low->as_address ();
> +  if (high <= low)
> +    return PC_BOUNDS_INVALID;
> +  if (low == 0 && !cu->per_objfile->per_bfd->has_section_at_zero)
> +    return PC_BOUNDS_INVALID;
> +
> +  if (lowpc)
> +    *lowpc = low;
> +  if (highpc)
> +    *highpc = high;

These should be 'lowpc != nullptr' and 'highpc != nullptr'.  We don't
make use of automatic pointer to bool conversion in GDB.

Also, I'm suspicious of the conditional write to *lowpc.  There's only
one call to dwarf2_get_pc_bounds_entry_point, from dwarf2_get_pc_bounds,
and that function seems to write unconditionally to lowpc.  I'd be more
inclined to just add a 'gdb_assert (lowpc != nullptr)' and then
unconditional write to *lowpc, like dwarf2_get_pc_bounds does.

With that change done, I notice that everything after 'if (high <= low)'
is duplicated between dwarf2_get_pc_bounds_entry_point and
dwarf2_get_pc_bounds, so I wonder if there's any scope to have more code
sharing, I'm thinking something like this:

  static enum pc_bounds_kind
  dwarf2_get_pc_bounds ( .... )
  {
    ...
    enum pc_bounds_kind ret;
  
    if (die->tag == DW_TAG_entry_point)
      ret = dwarf2_get_pc_bounds_entry_point (die, lowpc, highpc, cu);
    else
      ret = dwarf2_get_pc_bounds_???? ( .... );
  
    /* partial_die_info::read has also the strict LOW < HIGH requirement.  */
    if (high <= low)
      return PC_BOUNDS_INVALID;
  
    ... etc ...
  }

That's a pretty iffy explanation, but hopefully you can see what I'm thinking.

> +
> +  /* Return PC_BOUNDS_RANGES/PC_BOUNDS_HIGH_LOW depending on the parent
> +     die's bounds kind.  */
> +  return bounds_kind;
> +}
> +
>  /* Get low and high pc attributes from a die.  See enum pc_bounds_kind
>     definition for the return value.  *LOWPC and *HIGHPC are set iff
>     neither PC_BOUNDS_NOT_PRESENT nor PC_BOUNDS_INVALID are returned.  */
> @@ -13010,6 +13059,9 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
>    CORE_ADDR high = 0;
>    enum pc_bounds_kind ret;
>  
> +  if (die->tag == DW_TAG_entry_point)
> +    return dwarf2_get_pc_bounds_entry_point (die, lowpc, highpc, cu);
> +
>    attr_high = dwarf2_attr (die, DW_AT_high_pc, cu);
>    if (attr_high)
>      {
> @@ -20760,6 +20812,20 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
>  	  sym->set_domain (LABEL_DOMAIN);
>  	  add_symbol_to_list (sym, cu->list_in_scope);
>  	  break;
> +	case DW_TAG_entry_point:
> +	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
> +	     finish_block.  */
> +	  sym->set_aclass_index (LOC_BLOCK);
> +	  /* DW_TAG_entry_point provides an additional entry_point to an
> +	     existing sub_program.  Therefore, we inherit the "external"
> +	     attribute from the sub_program to which the entry_point
> +	     belongs to.  */
> +	  attr2 = dwarf2_attr (die->parent, DW_AT_external, cu);
> +	  if (attr2 != nullptr && attr2->as_boolean ())
> +	    list_to_add = cu->get_builder ()->get_global_symbols ();
> +	  else
> +	    list_to_add = cu->list_in_scope;
> +	  break;
>  	case DW_TAG_subprogram:
>  	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
>  	     finish_block.  */
> @@ -21464,6 +21530,7 @@ read_type_die_1 (struct die_info *die, struct dwarf2_cu *cu)
>      case DW_TAG_enumeration_type:
>        this_type = read_enumeration_type (die, cu);
>        break;
> +    case DW_TAG_entry_point:
>      case DW_TAG_subprogram:
>      case DW_TAG_subroutine_type:
>      case DW_TAG_inlined_subroutine:
> @@ -21783,12 +21850,15 @@ determine_prefix (struct die_info *die, struct dwarf2_cu *cu)
>  	return "";
>        case DW_TAG_subprogram:
>  	/* Nested subroutines in Fortran get a prefix with the name
> -	   of the parent's subroutine.  */
> +	   of the parent's subroutine.  Entry points are prefixed by the
> +	   parent's namespace.  */
>  	if (cu->per_cu->lang () == language_fortran)
>  	  {
>  	    if ((die->tag ==  DW_TAG_subprogram)
>  		&& (dwarf2_name (parent, cu) != NULL))
>  	      return dwarf2_name (parent, cu);
> +	    else if (die->tag == DW_TAG_entry_point)
> +	      return determine_prefix (parent, cu);
>  	  }
>  	return "";
>        case DW_TAG_enumeration_type:
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> new file mode 100644
> index 00000000000..e8c99d1f52e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> @@ -0,0 +1,39 @@
> +/* Copyright 2022 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/>.  */
> +
> +/* In the generated DWARF, we'll use the locations of foo_entry_label and
> +   foobar_entry_label as the low_pc's of our entry point TAGs.  */
> +
> +int I = 0;
> +int J = 0;
> +int K = 0;
> +
> +__attribute__((noinline))
> +void bar_helper () {

I think GNU coding standard should be followed here, and throughout this
source, unless the test requires that standard to be broken.  So:

  void
  bar_helper (void)
  {

etc.

> +  __asm__("bar_helper_label: .globl bar_helper_label");

I notice you used '__asm__' while all the other tests seem to use just
'asm'.  Is there a reason for this difference?  Also there should be a
space before the '('.

> +  I++;
> +  J++;
> +  __asm__("foo_entry_label: .globl foo_entry_label");
> +  J++;
> +  K++;
> +  __asm__("foobar_entry_label: .globl foobar_entry_label");
> +}
> +
> +int main() {
> +  __asm__("main_label: .globl main_label");
> +  bar_helper ();
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> new file mode 100644
> index 00000000000..1b5ae52fd8a
> --- /dev/null
> +++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> @@ -0,0 +1,215 @@
> +# Copyright 2022 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/>.
> +
> +# Test that the DW_TAG_entry_point is handled properly by GDB and that we can
> +# set breakpoints on function entry points.
> +
> +load_lib dwarf.exp
> +
> +# This test can only be run on targets that support DWARF-2 and use
> +# gas.
> +if {![dwarf2_support]} {
> +    return 0
> +}
> +
> +standard_testfile .c -dw.S
> +
> +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> +	return -1
> +}
> +
> +set int_size [get_sizeof "int" 4]

I don't think 'int_size' is used, but maybe it should be?  Right now,
I/J/K are defined as 4 bytes, but declared as 'int'.  You could maybe
declard I/J/K as 'int32_t' and stick with the DWARF hard-coded to 4, or
maybe make use of $int_size to adjust the DWARF for I/J/K...

> +
> +# Make some DWARF for the test.
> +set asm_file [standard_output_file $srcfile2]
> +Dwarf::assemble $asm_file {
> +    global srcfile
> +    declare_labels int_label int2_label
> +
> +    get_func_info main
> +    get_func_info bar_helper
> +
> +    set prog_line 1
> +    set bar_line 2
> +    set foo_line 3
> +    set foobar_line 4
> +
> +    set global_I [gdb_target_symbol I]
> +    set global_J [gdb_target_symbol J]
> +    set global_K [gdb_target_symbol K]
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_Fortran90}
> +	    {name dw2-entry-points.f90}
> +	    {comp_dir /tmp}
> +	} {
> +	    int_label: base_type {
> +		{name "int"}
> +		{byte_size 4 sdata}
> +		{encoding @DW_ATE_signed}
> +	    }
> +	    subprogram {
> +		{name prog}
> +		{decl_file 1 data1}
> +		{decl_line $prog_line data1}
> +		{low_pc $main_start addr}
> +		{high_pc "$main_start + $main_len" addr}
> +		{external 1 flag}
> +		{main_subprogram 1 flag}
> +	    }
> +	    subprogram {
> +		{name bar}
> +		{decl_file 1 data1}
> +		{decl_line $bar_line data1}
> +		{external 1 flag}
> +		{low_pc $bar_helper_start addr}
> +		{high_pc "$bar_helper_start + $bar_helper_len" addr}
> +	    } {
> +		formal_parameter {
> +		    {name I}
> +		    {type :$int_label}
> +		    {location {addr $global_I} SPECIAL_expr}
> +		}
> +		formal_parameter {
> +		    {name J}
> +		    {type :$int_label}
> +		    {location {addr $global_J} SPECIAL_expr}
> +		}
> +		entry_point {
> +		    {name foo}
> +		    {decl_file 1 data1}
> +		    {decl_line $foo_line data1}
> +		    {low_pc foo_entry_label addr}
> +		} {
> +		    formal_parameter {
> +			{name J}
> +			{type :$int_label}
> +			{location {addr $global_J} SPECIAL_expr}
> +		    }
> +		    formal_parameter {
> +			{name K}
> +			{type :$int_label}
> +			{location {addr $global_K} SPECIAL_expr}
> +		    }
> +		}
> +		entry_point {
> +			{name foobar}
> +			{decl_file 1 data1}
> +			{decl_line $foobar_line data1}
> +			{low_pc foobar_entry_label addr}
> +		    } {
> +		    formal_parameter {
> +			{name J}
> +			{type :$int_label}
> +			{location {addr $global_J} SPECIAL_expr}
> +		    }
> +		}
> +	    }
> +	}
> +    }
> +
> +    cu {} {
> +	compile_unit {
> +	    {language @DW_LANG_Fortran90}
> +	    {name dw2-entry-points-2.f90}
> +	    {comp_dir /tmp}
> +	} {
> +	    int2_label: base_type {
> +		{name "int"}
> +		{byte_size 4 sdata}
> +		{encoding @DW_ATE_signed}
> +	    }
> +	    subprogram {
> +		{name barso}
> +		{decl_file 1 data1}
> +		{decl_line $bar_line data1}
> +		{external 1 flag}
> +		{low_pc $bar_helper_start addr}
> +		{high_pc "$bar_helper_start + $bar_helper_len" addr}
> +	    } {
> +		formal_parameter {
> +		    {name I}
> +		    {type :$int2_label}
> +		    {location {addr $global_I} SPECIAL_expr}
> +		}
> +		formal_parameter {
> +		    {name J}
> +		    {type :$int2_label}
> +		    {location {addr $global_J} SPECIAL_expr}
> +		}
> +		entry_point {
> +		    {name fooso}
> +		    {decl_file 1 data1}
> +		    {decl_line $foo_line data1}
> +		    {low_pc foo_entry_label addr}
> +		} {
> +		    formal_parameter {
> +			{name J}
> +			{type :$int2_label}
> +			{location {addr $global_J} SPECIAL_expr}
> +		    }
> +		    formal_parameter {
> +			{name K}
> +			{type :$int2_label}
> +			{location {addr $global_K} SPECIAL_expr}
> +		    }
> +		}
> +	    }
> +	}
> +    }
> +}
> +
> +if {[prepare_for_testing "failed to prepare" ${testfile} \
> +	 [list $srcfile $asm_file] {nodebug}]} {
> +    return -1
> +}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +# Try whether we can set and hit breakpoints at the entry_points.
> +gdb_breakpoint "foo"
> +gdb_breakpoint "foobar"
> +
> +# Now hit the entry_point break point and check their call-stack.
> +gdb_continue_to_breakpoint "foo"
> +gdb_test "bt" [multi_line \
> +		   "#0.*${hex} in foo \\(J=1, K=0\\).*" \
> +		   "#1.*${hex} in prog \\(\\).*" \
> +    ] "bt foo"
> +
> +gdb_continue_to_breakpoint "foobar"
> +gdb_test "bt" [multi_line \
> +		   "#0.*${hex} in foobar \\(J=2\\).*" \
> +		   "#1.*${hex} in prog \\(\\).*" \
> +    ] "bt foobar"
> +
> +# Now try whether we can also set breakpoints on entry_points from other CUs.
> +
> +clean_restart ${testfile}
> +
> +if ![runto_main] {
> +    return -1
> +}
> +
> +gdb_breakpoint "fooso"
> +gdb_continue_to_breakpoint "foo_so"
> +
> +gdb_test "bt" [multi_line \
> +		    "#0.*${hex} in foo \\(J=1, K=0\\).*" \
> +		    "#1.*${hex} in prog \\(\\).*" \
> +] "bt fooso"
> diff --git a/gdb/testsuite/gdb.fortran/entry-point.exp b/gdb/testsuite/gdb.fortran/entry-point.exp
> new file mode 100644
> index 00000000000..679191f79f8
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/entry-point.exp
> @@ -0,0 +1,84 @@
> +# Copyright 2022 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/>.
> +#
> +# Test Fortran entry points for subroutines.
> +
> +if { [skip_fortran_tests] } { return -1 }
> +
> +standard_testfile .f90
> +load_lib "fortran.exp"
> +
> +if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug f90}] } {
> +    return -1
> +}
> +
> +if { ![fortran_runto_main] } {
> +    untested "could not run to main"
> +    return -1
> +}
> +
> +# Test if we can set a breakpoint via the entry-point name.
> +set entry_point_name "foo"
> +gdb_breakpoint $entry_point_name
> +gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
> +    ".*entry foo\\(J,K,L,I1\\).*"
> +
> +gdb_test "print j" "= 11" "print j, entered via $entry_point_name"
> +gdb_test "print k" "= 22" "print k, entered via $entry_point_name"
> +gdb_test "print l" "= 33" "print l, entered via $entry_point_name"
> +gdb_test "print i1" "= 44" "print i1, entered via $entry_point_name"
> +gdb_test "info args" \
> +    [multi_line "j = 11" \
> +		"k = 22" \
> +		"l = 33" \
> +		"i1 = 44"] \
> +    "info args, entered via $entry_point_name"
> +
> +# Test if we can set a breakpoint via the function name.
> +set entry_point_name "bar"
> +gdb_breakpoint $entry_point_name
> +gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
> +    ".*subroutine bar\\(I,J,K,I1\\).*"
> +
> +gdb_test "print i" "= 444" "print i, entered via $entry_point_name"
> +gdb_test "print j" "= 555" "print j, entered via $entry_point_name"
> +gdb_test "print k" "= 666" "print k, entered via $entry_point_name"
> +gdb_test "print i1" "= 777" "print i1, entered via $entry_point_name"
> +
> +# Test a second entry point.
> +set entry_point_name "foobar"
> +gdb_breakpoint $entry_point_name
> +gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
> +    ".* entry foobar\\(J\\).*"
> +
> +gdb_test "print j" "= 1" "print j, entered via $entry_point_name"
> +gdb_test "info args" "j = 1" "info args, entered via $entry_point_name"
> +
> +# Test breaking at the entrypoint defined inside the module mod via its
> +# scoped name.
> +set entry_point_name "mod::mod_foo"
> +
> +# GCC moves subroutines with entry points out of the module scope into the
> +# compile unit scope.
> +if {[test_compiler_info {gcc-*}]} {
> +    setup_xfail "gcc/105272" *-*-*
> +}
> +gdb_breakpoint $entry_point_name
> +
> +if {[test_compiler_info {gcc-*}]} {
> +    setup_xfail "gcc/105272" *-*-*
> +}
> +gdb_continue_to_breakpoint "continue to breakpoint: $entry_point_name" \
> +    ".* entry mod_foo\\(\\).*"
> diff --git a/gdb/testsuite/gdb.fortran/entry-point.f90 b/gdb/testsuite/gdb.fortran/entry-point.f90
> new file mode 100644
> index 00000000000..12a0557e787
> --- /dev/null
> +++ b/gdb/testsuite/gdb.fortran/entry-point.f90
> @@ -0,0 +1,67 @@
> +! Copyright 2022 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
> +implicit none
> +
> +contains
> +  subroutine mod_bar
> +    integer :: I = 3
> +
> +    goto 100
> +
> +    entry mod_foo
> +    I = 33
> +
> +100 print *, I
> +  end subroutine mod_bar
> +end module mod
> +
> +
> +subroutine bar(I,J,K,I1)
> +  integer :: I,J,K,L,I1
> +  integer :: A
> +  real :: C
> +
> +  A = 0
> +  C = 0.0
> +
> +  A = I + K + I1
> +  goto 300
> +
> +  entry foo(J,K,L,I1)
> +  A = J + K + L + I1
> +
> +200 C = J
> +  goto 300
> +
> +  entry foobar(J)
> +  goto 200
> +
> +300 A = C + 1
> +  C = J * 1.5
> +
> +  return
> +end subroutine
> +
> +program TestEntryPoint
> +  use mod
> +
> +  call foo(11,22,33,44)
> +  call bar(444,555,666,777)
> +  call foobar(1)
> +
> +  call mod_foo()
> +end program TestEntryPoint
> -- 
> 2.25.1
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point
  2022-07-11 13:02   ` Andrew Burgess
@ 2022-07-13  9:38     ` Kempke, Nils-Christian
  0 siblings, 0 replies; 4+ messages in thread
From: Kempke, Nils-Christian @ 2022-07-13  9:38 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches; +Cc: jinisusan.george, tom

Hi Andrew,

Thanks for the feedback! I inlined my comments and will send a v4 soon.

Cheers,
Nils

> -----Original Message-----
> From: Andrew Burgess <aburgess@redhat.com>
> Sent: Monday, July 11, 2022 3:03 PM
> To: Kempke, Nils-Christian <nils-christian.kempke@intel.com>; gdb-
> patches@sourceware.org
> Cc: Bernhard Heckel <bernhard.heckel@intel.com>; Tim Wiederhake
> <tim.wiederhake@intel.com>; jinisusan.george@amd.com;
> tom@tromey.com
> Subject: Re: [PATCH v3 1/1] dwarf, fortran: add support for
> DW_TAG_entry_point
> 
> Nils-Christian Kempke via Gdb-patches <gdb-patches@sourceware.org>
> writes:
> 
> > Fortran provides additional entry points for subroutines and functions.
> > These entry points may use only a subset (or a different set) of the
> > parameters of the original subroutine.  The entry points may be described
> > via the DWARF tag DW_TAG_entry_point.
> >
> > This commit adds support for parsing the DW_TAG_entry_point DWARF
> tag.
> > Currently, between ifx/ifort/gfortran, only ifort is actually emitting
> > this tag.  Both, ifx and gfortran use the DW_TAG_subprogram tag as
> > workaround/alternative.  Thus, this patch really only adds more ifort
> > support.  Even so, some of the attached tests still fail for ifort, due
> > to some wrong line info generated for the entry points in ifort.
> >
> > After this patch it is possible to set a breakpoint in gdb with the
> > ifort compiled example at the entry points 'foo' and 'foobar', which was not
> > possible before.
> >
> > As gcc and ifx do not emit the tag I also added a test to gdb.dwarf2
> > which uses some underlying c compiled code and adds some Fortran style
> DWARF
> > to it emitting the DW_TAG_entry_point.  Before this patch it was not
> > possible to actually define breakpoint at the entry point tags.
> >
> > For gfortran there actually exists a bug on bugzilla, asking for the use
> > of DW_TAG_entry_point over DW_TAG_subprogram:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37134
> >
> > This patch was originally posted here
> >
> > https://sourceware.org/legacy-ml/gdb-patches/2017-07/msg00317.html
> >
> > but its review/pinging got lost after a while.  I reworked it to fit the
> > current GDB.
> 
> Thanks for the revisions.  I took a look through as I have an interest
> in Fortran support, and I had some pretty minor feedback.  I'm no expert
> on the DWARF indexes and CU expansion in GDB, but that side of things
> looks reasonable to me - at least I'd be happy to debug this code if
> anything came up later.
> 
> Anyway, my feedback is inline below.
> 
> Thanks,
> Andrew
> 
> >
> > Co-authored-by: Bernhard Heckel <bernhard.heckel@intel.com>
> > Co-authored-by: Tim Wiederhake  <tim.wiederhake@intel.com>
> > ---
> >  gdb/dwarf2/abbrev.c                           |   1 +
> >  gdb/dwarf2/cooked-index.h                     |   3 +-
> >  gdb/dwarf2/index-write.c                      |   3 +-
> >  gdb/dwarf2/read.c                             |  72 +++++-
> >  gdb/testsuite/gdb.dwarf2/dw2-entry-points.c   |  39 ++++
> >  gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp | 215
> ++++++++++++++++++
> >  gdb/testsuite/gdb.fortran/entry-point.exp     |  84 +++++++
> >  gdb/testsuite/gdb.fortran/entry-point.f90     |  67 ++++++
> >  8 files changed, 481 insertions(+), 3 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> >  create mode 100644 gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> >  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.exp
> >  create mode 100644 gdb/testsuite/gdb.fortran/entry-point.f90
> >
> > diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c
> > index 4ca27eaa7e0..f9f87203e3e 100644
> > --- a/gdb/dwarf2/abbrev.c
> > +++ b/gdb/dwarf2/abbrev.c
> > @@ -88,6 +88,7 @@ tag_interesting_for_index (dwarf_tag tag)
> >      case DW_TAG_base_type:
> >      case DW_TAG_class_type:
> >      case DW_TAG_constant:
> > +    case DW_TAG_entry_point:
> >      case DW_TAG_enumeration_type:
> >      case DW_TAG_enumerator:
> >      case DW_TAG_imported_declaration:
> > diff --git a/gdb/dwarf2/cooked-index.h b/gdb/dwarf2/cooked-index.h
> > index 439cbb19fa7..e63d3001d2a 100644
> > --- a/gdb/dwarf2/cooked-index.h
> > +++ b/gdb/dwarf2/cooked-index.h
> > @@ -113,7 +113,8 @@ struct cooked_index_entry : public
> allocate_on_obstack
> >  		|| tag == DW_TAG_constant
> >  		|| tag == DW_TAG_enumerator);
> >        case FUNCTIONS_DOMAIN:
> > -	return tag == DW_TAG_subprogram;
> > +	return (tag == DW_TAG_subprogram
> > +		|| tag == DW_TAG_entry_point);
> >        case TYPES_DOMAIN:
> >  	return tag_is_type (tag);
> >        case MODULES_DOMAIN:
> > diff --git a/gdb/dwarf2/index-write.c b/gdb/dwarf2/index-write.c
> > index c3aad8dd999..a819fd49032 100644
> > --- a/gdb/dwarf2/index-write.c
> > +++ b/gdb/dwarf2/index-write.c
> > @@ -1128,7 +1128,8 @@ write_cooked_index (cooked_index_vector
> *table,
> >        const char *name = entry->full_name (&symtab->m_string_obstack);
> >
> >        gdb_index_symbol_kind kind;
> > -      if (entry->tag == DW_TAG_subprogram)
> > +      if (entry->tag == DW_TAG_subprogram
> > +	  || entry->tag == DW_TAG_entry_point)
> >  	kind = GDB_INDEX_SYMBOL_KIND_FUNCTION;
> >        else if (entry->tag == DW_TAG_variable
> >  	       || entry->tag == DW_TAG_constant
> > diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> > index 23fe5679cbd..64a255c6487 100644
> > --- a/gdb/dwarf2/read.c
> > +++ b/gdb/dwarf2/read.c
> > @@ -8607,6 +8607,7 @@ process_die (struct die_info *die, struct
> dwarf2_cu *cu)
> >  	  && die->parent->tag == DW_TAG_subprogram)
> >  	cu->processing_has_namespace_info = true;
> >        /* Fall through.  */
> > +    case DW_TAG_entry_point:
> >      case DW_TAG_inlined_subroutine:
> >        read_func_scope (die, cu);
> >        break;
> > @@ -8723,6 +8724,7 @@ die_needs_namespace (struct die_info *die,
> struct dwarf2_cu *cu)
> >      case DW_TAG_enumerator:
> >      case DW_TAG_subprogram:
> >      case DW_TAG_inlined_subroutine:
> > +    case DW_TAG_entry_point:
> >      case DW_TAG_member:
> >      case DW_TAG_imported_declaration:
> >        return 1;
> > @@ -12994,6 +12996,53 @@ dwarf2_ranges_read_low_addrs (unsigned
> offset, struct dwarf2_cu *cu,
> >      });
> >  }
> >
> > +/* Determine the low and high pc of a DW_TAG_entry_point.  */
> > +
> > +static pc_bounds_kind
> > +dwarf2_get_pc_bounds_entry_point (die_info *die, CORE_ADDR *lowpc,
> > +				  CORE_ADDR *highpc, dwarf2_cu *cu)
> > +{
> > +  CORE_ADDR low = 0;
> > +  CORE_ADDR high = 0;
> > +
> > +  if (die->parent->tag != DW_TAG_subprogram)
> > +    {
> > +      complaint (_("DW_TAG_entry_point not embedded in
> DW_TAG_subprogram"));
> > +      return PC_BOUNDS_INVALID;
> > +    }
> > +
> > +  /* A DW_TAG_entry_point is embedded in an subprogram.  Therefore,
> we can use
> > +     the highpc from its enveloping subprogram and get the lowpc from
> > +     DWARF.  */
> > +  const enum pc_bounds_kind bounds_kind = dwarf2_get_pc_bounds
> (die->parent,
> > +								&low, &high,
> > +								cu, nullptr,
> > +								nullptr);
> > +  if (bounds_kind == PC_BOUNDS_INVALID || bounds_kind ==
> PC_BOUNDS_NOT_PRESENT)
> > +    return PC_BOUNDS_INVALID;
> 
> Is there a reason why PC_BOUNDS_NOT_PRESENT is transformed to
> PC_BOUNDS_INVALID here?  I guess I would have expected 'return
> bounds_kind' instead.
> 

No, this was a mistake in v2 I think when Jini made me aware of a problem
at that same spot in the first place.  The should be correct in the new patch
series (as an result of the outsourcing as well).

> > +
> > +  attribute *attr_low = dwarf2_attr (die, DW_AT_low_pc, cu);
> > +  if (!attr_low)
> > +    {
> > +      complaint (_("DW_TAG_entry_point is missing DW_AT_low_pc"));
> > +      return PC_BOUNDS_INVALID;
> > +    }
> > +  low = attr_low->as_address ();
> > +  if (high <= low)
> > +    return PC_BOUNDS_INVALID;
> > +  if (low == 0 && !cu->per_objfile->per_bfd->has_section_at_zero)
> > +    return PC_BOUNDS_INVALID;
> > +
> > +  if (lowpc)
> > +    *lowpc = low;
> > +  if (highpc)
> > +    *highpc = high;
> 
> These should be 'lowpc != nullptr' and 'highpc != nullptr'.  We don't
> make use of automatic pointer to bool conversion in GDB.

Ah, I stole this from the dwarf2_get_pc_bounds - yes.  I corrected it in v4
for what remains of dwarf2_get_pc_bounds.

> 
> Also, I'm suspicious of the conditional write to *lowpc.  There's only
> one call to dwarf2_get_pc_bounds_entry_point, from
> dwarf2_get_pc_bounds,
> and that function seems to write unconditionally to lowpc.  I'd be more
> inclined to just add a 'gdb_assert (lowpc != nullptr)' and then
> unconditional write to *lowpc, like dwarf2_get_pc_bounds does.

The function dwarf2_get_pc_bounds should probably also have this assert.
In v4 I added it.

> 
> With that change done, I notice that everything after 'if (high <= low)'
> is duplicated between dwarf2_get_pc_bounds_entry_point and
> dwarf2_get_pc_bounds, so I wonder if there's any scope to have more code
> sharing, I'm thinking something like this:
> 
>   static enum pc_bounds_kind
>   dwarf2_get_pc_bounds ( .... )
>   {
>     ...
>     enum pc_bounds_kind ret;
> 
>     if (die->tag == DW_TAG_entry_point)
>       ret = dwarf2_get_pc_bounds_entry_point (die, lowpc, highpc, cu);
>     else
>       ret = dwarf2_get_pc_bounds_???? ( .... );
> 
>     /* partial_die_info::read has also the strict LOW < HIGH requirement.  */
>     if (high <= low)
>       return PC_BOUNDS_INVALID;
> 
>     ... etc ...
>   }
> 
> That's a pretty iffy explanation, but hopefully you can see what I'm thinking.

Yes, so all of this is correct - I outsourced the get_pc_bounds code now into
a function I called dwarf_get_pc_bounds_ranges_or_highlow_pc for now.  I do
not really know a nicer name - this one at least seems to describe a bit what
the function does - it get the pc bounds using ranges or high/low pc attributes.

I now also added an assert 'gdb_assert (lowpc != nullptr);' as you indicated for
dwarf2_get_pc_bounds_entry_point to dwarf2_get_pc_bounds - this has a bit
more impact on GDB in general but should be fine.

I found that these changes made the patch a bit convoluted and so I split this
patch into 3 now - one oursourcing the code in get_pc_bounds, one adding
the assert to get_pc_bounds and one introducing the DW_TAG_entry_point.

> > +
> > +  /* Return PC_BOUNDS_RANGES/PC_BOUNDS_HIGH_LOW depending
> on the parent
> > +     die's bounds kind.  */
> > +  return bounds_kind;
> > +}
> > +
> >  /* Get low and high pc attributes from a die.  See enum pc_bounds_kind
> >     definition for the return value.  *LOWPC and *HIGHPC are set iff
> >     neither PC_BOUNDS_NOT_PRESENT nor PC_BOUNDS_INVALID are
> returned.  */
> > @@ -13010,6 +13059,9 @@ dwarf2_get_pc_bounds (struct die_info *die,
> CORE_ADDR *lowpc,
> >    CORE_ADDR high = 0;
> >    enum pc_bounds_kind ret;
> >
> > +  if (die->tag == DW_TAG_entry_point)
> > +    return dwarf2_get_pc_bounds_entry_point (die, lowpc, highpc, cu);
> > +
> >    attr_high = dwarf2_attr (die, DW_AT_high_pc, cu);
> >    if (attr_high)
> >      {
> > @@ -20760,6 +20812,20 @@ new_symbol (struct die_info *die, struct type
> *type, struct dwarf2_cu *cu,
> >  	  sym->set_domain (LABEL_DOMAIN);
> >  	  add_symbol_to_list (sym, cu->list_in_scope);
> >  	  break;
> > +	case DW_TAG_entry_point:
> > +	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
> > +	     finish_block.  */
> > +	  sym->set_aclass_index (LOC_BLOCK);
> > +	  /* DW_TAG_entry_point provides an additional entry_point to an
> > +	     existing sub_program.  Therefore, we inherit the "external"
> > +	     attribute from the sub_program to which the entry_point
> > +	     belongs to.  */
> > +	  attr2 = dwarf2_attr (die->parent, DW_AT_external, cu);
> > +	  if (attr2 != nullptr && attr2->as_boolean ())
> > +	    list_to_add = cu->get_builder ()->get_global_symbols ();
> > +	  else
> > +	    list_to_add = cu->list_in_scope;
> > +	  break;
> >  	case DW_TAG_subprogram:
> >  	  /* SYMBOL_BLOCK_VALUE (sym) will be filled in later by
> >  	     finish_block.  */
> > @@ -21464,6 +21530,7 @@ read_type_die_1 (struct die_info *die, struct
> dwarf2_cu *cu)
> >      case DW_TAG_enumeration_type:
> >        this_type = read_enumeration_type (die, cu);
> >        break;
> > +    case DW_TAG_entry_point:
> >      case DW_TAG_subprogram:
> >      case DW_TAG_subroutine_type:
> >      case DW_TAG_inlined_subroutine:
> > @@ -21783,12 +21850,15 @@ determine_prefix (struct die_info *die,
> struct dwarf2_cu *cu)
> >  	return "";
> >        case DW_TAG_subprogram:
> >  	/* Nested subroutines in Fortran get a prefix with the name
> > -	   of the parent's subroutine.  */
> > +	   of the parent's subroutine.  Entry points are prefixed by the
> > +	   parent's namespace.  */
> >  	if (cu->per_cu->lang () == language_fortran)
> >  	  {
> >  	    if ((die->tag ==  DW_TAG_subprogram)
> >  		&& (dwarf2_name (parent, cu) != NULL))
> >  	      return dwarf2_name (parent, cu);
> > +	    else if (die->tag == DW_TAG_entry_point)
> > +	      return determine_prefix (parent, cu);
> >  	  }
> >  	return "";
> >        case DW_TAG_enumeration_type:
> > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> > new file mode 100644
> > index 00000000000..e8c99d1f52e
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.c
> > @@ -0,0 +1,39 @@
> > +/* Copyright 2022 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/>.
> */
> > +
> > +/* In the generated DWARF, we'll use the locations of foo_entry_label
> and
> > +   foobar_entry_label as the low_pc's of our entry point TAGs.  */
> > +
> > +int I = 0;
> > +int J = 0;
> > +int K = 0;
> > +
> > +__attribute__((noinline))
> > +void bar_helper () {
> 
> I think GNU coding standard should be followed here, and throughout this
> source, unless the test requires that standard to be broken.  So:
> 
>   void
>   bar_helper (void)
>   {
> 
> etc.

Yes, indeed .. Fixed in v4.

> 
> > +  __asm__("bar_helper_label: .globl bar_helper_label");
> 
> I notice you used '__asm__' while all the other tests seem to use just
> 'asm'.  Is there a reason for this difference?  Also there should be a
> space before the '('.

Actually, I think both exists in the tests - there are some that use '__asm__'
and some that use 'asm'.  When writing the test I actually thought that
'__asm__' was used more often but I think I was be wrong here.

For gcc it seems that asm is the standard way and __asm__ an alternative when
compiling with -ansi or -std:

https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html

While the gcc documentation says that __asm__ might not work with other C
compilers, I've not found this to be the case with icx/icc/clang.  For Intel icx/icc
I think the __asm__ option is preferred as it holds some safeguard when
also using Microsoft style asm (which is not the case here obviously).  Actually,
here https://en.cppreference.com/w/c/language/asm it says that clang and gcc
prefer __asm__ over asm when using options like -std=c++11.

I don't really know which one to prefer - as said, there exist both within the
testsuite.

I for now changed this to be 'asm'.
I also added the spaces.

> 
> > +  I++;
> > +  J++;
> > +  __asm__("foo_entry_label: .globl foo_entry_label");
> > +  J++;
> > +  K++;
> > +  __asm__("foobar_entry_label: .globl foobar_entry_label");
> > +}
> > +
> > +int main() {
> > +  __asm__("main_label: .globl main_label");
> > +  bar_helper ();
> > +
> > +  return 0;
> > +}
> > diff --git a/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> > new file mode 100644
> > index 00000000000..1b5ae52fd8a
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.dwarf2/dw2-entry-points.exp
> > @@ -0,0 +1,215 @@
> > +# Copyright 2022 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/>.
> > +
> > +# Test that the DW_TAG_entry_point is handled properly by GDB and that
> we can
> > +# set breakpoints on function entry points.
> > +
> > +load_lib dwarf.exp
> > +
> > +# This test can only be run on targets that support DWARF-2 and use
> > +# gas.
> > +if {![dwarf2_support]} {
> > +    return 0
> > +}
> > +
> > +standard_testfile .c -dw.S
> > +
> > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } {
> > +	return -1
> > +}
> > +
> > +set int_size [get_sizeof "int" 4]
> 
> I don't think 'int_size' is used, but maybe it should be?  Right now,
> I/J/K are defined as 4 bytes, but declared as 'int'.  You could maybe
> declard I/J/K as 'int32_t' and stick with the DWARF hard-coded to 4, or
> maybe make use of $int_size to adjust the DWARF for I/J/K...
> 

Yes, int_size was intended to be the size of the I/J/K - I changed the
two "int" types to be of size $int_size.

> > +
> > +# Make some DWARF for the test.
> > +set asm_file [standard_output_file $srcfile2]
> > +Dwarf::assemble $asm_file {
> > +    global srcfile
> > +    declare_labels int_label int2_label
> > +
> > +    get_func_info main
> > +    get_func_info bar_helper
> > +
> > +    set prog_line 1
> > +    set bar_line 2
> > +    set foo_line 3
> > +    set foobar_line 4
> > +
> > +    set global_I [gdb_target_symbol I]
> > +    set global_J [gdb_target_symbol J]
> > +    set global_K [gdb_target_symbol K]
> > +
> > +    cu {} {
> > +	compile_unit {
> > +	    {language @DW_LANG_Fortran90}
> > +	    {name dw2-entry-points.f90}
> > +	    {comp_dir /tmp}
> > +	} {
> > +	    int_label: base_type {
> > +		{name "int"}
> > +		{byte_size 4 sdata}
> > +		{encoding @DW_ATE_signed}
> > +	    }
> > +	    subprogram {
> > +		{name prog}
> > +		{decl_file 1 data1}
> > +		{decl_line $prog_line data1}
> > +		{low_pc $main_start addr}
> > +		{high_pc "$main_start + $main_len" addr}
> > +		{external 1 flag}
> > +		{main_subprogram 1 flag}
> > +	    }
> > +	    subprogram {
> > +		{name bar}
> > +		{decl_file 1 data1}
> > +		{decl_line $bar_line data1}
> > +		{external 1 flag}
> > +		{low_pc $bar_helper_start addr}
> > +		{high_pc "$bar_helper_start + $bar_helper_len" addr}
> > +	    } {
> > +		formal_parameter {
> > +		    {name I}
> > +		    {type :$int_label}
> > +		    {location {addr $global_I} SPECIAL_expr}
> > +		}
> > +		formal_parameter {
> > +		    {name J}
> > +		    {type :$int_label}
> > +		    {location {addr $global_J} SPECIAL_expr}
> > +		}
> > +		entry_point {
> > +		    {name foo}
> > +		    {decl_file 1 data1}
> > +		    {decl_line $foo_line data1}
> > +		    {low_pc foo_entry_label addr}
> > +		} {
> > +		    formal_parameter {
> > +			{name J}
> > +			{type :$int_label}
> > +			{location {addr $global_J} SPECIAL_expr}
> > +		    }
> > +		    formal_parameter {
> > +			{name K}
> > +			{type :$int_label}
> > +			{location {addr $global_K} SPECIAL_expr}
> > +		    }
> > +		}
> > +		entry_point {
> > +			{name foobar}
> > +			{decl_file 1 data1}
> > +			{decl_line $foobar_line data1}
> > +			{low_pc foobar_entry_label addr}
> > +		    } {
> > +		    formal_parameter {
> > +			{name J}
> > +			{type :$int_label}
> > +			{location {addr $global_J} SPECIAL_expr}
> > +		    }
> > +		}
> > +	    }
> > +	}
> > +    }
> > +
> > +    cu {} {
> > +	compile_unit {
> > +	    {language @DW_LANG_Fortran90}
> > +	    {name dw2-entry-points-2.f90}
> > +	    {comp_dir /tmp}
> > +	} {
> > +	    int2_label: base_type {
> > +		{name "int"}
> > +		{byte_size 4 sdata}
> > +		{encoding @DW_ATE_signed}
> > +	    }
> > +	    subprogram {
> > +		{name barso}
> > +		{decl_file 1 data1}
> > +		{decl_line $bar_line data1}
> > +		{external 1 flag}
> > +		{low_pc $bar_helper_start addr}
> > +		{high_pc "$bar_helper_start + $bar_helper_len" addr}
> > +	    } {
> > +		formal_parameter {
> > +		    {name I}
> > +		    {type :$int2_label}
> > +		    {location {addr $global_I} SPECIAL_expr}
> > +		}
> > +		formal_parameter {
> > +		    {name J}
> > +		    {type :$int2_label}
> > +		    {location {addr $global_J} SPECIAL_expr}
> > +		}
> > +		entry_point {
> > +		    {name fooso}
> > +		    {decl_file 1 data1}
> > +		    {decl_line $foo_line data1}
> > +		    {low_pc foo_entry_label addr}
> > +		} {
> > +		    formal_parameter {
> > +			{name J}
> > +			{type :$int2_label}
> > +			{location {addr $global_J} SPECIAL_expr}
> > +		    }
> > +		    formal_parameter {
> > +			{name K}
> > +			{type :$int2_label}
> > +			{location {addr $global_K} SPECIAL_expr}
> > +		    }
> > +		}
> > +	    }
> > +	}
> > +    }
> > +}
> > +
> > +if {[prepare_for_testing "failed to prepare" ${testfile} \
> > +	 [list $srcfile $asm_file] {nodebug}]} {
> > +    return -1
> > +}
> > +
> > +if ![runto_main] {
> > +    return -1
> > +}
> > +
> > +# Try whether we can set and hit breakpoints at the entry_points.
> > +gdb_breakpoint "foo"
> > +gdb_breakpoint "foobar"
> > +
> > +# Now hit the entry_point break point and check their call-stack.
> > +gdb_continue_to_breakpoint "foo"
> > +gdb_test "bt" [multi_line \
> > +		   "#0.*${hex} in foo \\(J=1, K=0\\).*" \
> > +		   "#1.*${hex} in prog \\(\\).*" \
> > +    ] "bt foo"
> > +
> > +gdb_continue_to_breakpoint "foobar"
> > +gdb_test "bt" [multi_line \
> > +		   "#0.*${hex} in foobar \\(J=2\\).*" \
> > +		   "#1.*${hex} in prog \\(\\).*" \
> > +    ] "bt foobar"
> > +
> > +# Now try whether we can also set breakpoints on entry_points from
> other CUs.
> > +
> > +clean_restart ${testfile}
> > +
> > +if ![runto_main] {
> > +    return -1
> > +}
> > +
> > +gdb_breakpoint "fooso"
> > +gdb_continue_to_breakpoint "foo_so"
> > +
> > +gdb_test "bt" [multi_line \
> > +		    "#0.*${hex} in foo \\(J=1, K=0\\).*" \
> > +		    "#1.*${hex} in prog \\(\\).*" \
> > +] "bt fooso"
> > diff --git a/gdb/testsuite/gdb.fortran/entry-point.exp
> b/gdb/testsuite/gdb.fortran/entry-point.exp
> > new file mode 100644
> > index 00000000000..679191f79f8
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.fortran/entry-point.exp
> > @@ -0,0 +1,84 @@
> > +# Copyright 2022 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/>.
> > +#
> > +# Test Fortran entry points for subroutines.
> > +
> > +if { [skip_fortran_tests] } { return -1 }
> > +
> > +standard_testfile .f90
> > +load_lib "fortran.exp"
> > +
> > +if { [prepare_for_testing $testfile.exp $testfile $srcfile {debug f90}] } {
> > +    return -1
> > +}
> > +
> > +if { ![fortran_runto_main] } {
> > +    untested "could not run to main"
> > +    return -1
> > +}
> > +
> > +# Test if we can set a breakpoint via the entry-point name.
> > +set entry_point_name "foo"
> > +gdb_breakpoint $entry_point_name
> > +gdb_continue_to_breakpoint "continue to breakpoint:
> $entry_point_name" \
> > +    ".*entry foo\\(J,K,L,I1\\).*"
> > +
> > +gdb_test "print j" "= 11" "print j, entered via $entry_point_name"
> > +gdb_test "print k" "= 22" "print k, entered via $entry_point_name"
> > +gdb_test "print l" "= 33" "print l, entered via $entry_point_name"
> > +gdb_test "print i1" "= 44" "print i1, entered via $entry_point_name"
> > +gdb_test "info args" \
> > +    [multi_line "j = 11" \
> > +		"k = 22" \
> > +		"l = 33" \
> > +		"i1 = 44"] \
> > +    "info args, entered via $entry_point_name"
> > +
> > +# Test if we can set a breakpoint via the function name.
> > +set entry_point_name "bar"
> > +gdb_breakpoint $entry_point_name
> > +gdb_continue_to_breakpoint "continue to breakpoint:
> $entry_point_name" \
> > +    ".*subroutine bar\\(I,J,K,I1\\).*"
> > +
> > +gdb_test "print i" "= 444" "print i, entered via $entry_point_name"
> > +gdb_test "print j" "= 555" "print j, entered via $entry_point_name"
> > +gdb_test "print k" "= 666" "print k, entered via $entry_point_name"
> > +gdb_test "print i1" "= 777" "print i1, entered via $entry_point_name"
> > +
> > +# Test a second entry point.
> > +set entry_point_name "foobar"
> > +gdb_breakpoint $entry_point_name
> > +gdb_continue_to_breakpoint "continue to breakpoint:
> $entry_point_name" \
> > +    ".* entry foobar\\(J\\).*"
> > +
> > +gdb_test "print j" "= 1" "print j, entered via $entry_point_name"
> > +gdb_test "info args" "j = 1" "info args, entered via $entry_point_name"
> > +
> > +# Test breaking at the entrypoint defined inside the module mod via its
> > +# scoped name.
> > +set entry_point_name "mod::mod_foo"
> > +
> > +# GCC moves subroutines with entry points out of the module scope into
> the
> > +# compile unit scope.
> > +if {[test_compiler_info {gcc-*}]} {
> > +    setup_xfail "gcc/105272" *-*-*
> > +}
> > +gdb_breakpoint $entry_point_name
> > +
> > +if {[test_compiler_info {gcc-*}]} {
> > +    setup_xfail "gcc/105272" *-*-*
> > +}
> > +gdb_continue_to_breakpoint "continue to breakpoint:
> $entry_point_name" \
> > +    ".* entry mod_foo\\(\\).*"
> > diff --git a/gdb/testsuite/gdb.fortran/entry-point.f90
> b/gdb/testsuite/gdb.fortran/entry-point.f90
> > new file mode 100644
> > index 00000000000..12a0557e787
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.fortran/entry-point.f90
> > @@ -0,0 +1,67 @@
> > +! Copyright 2022 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
> > +implicit none
> > +
> > +contains
> > +  subroutine mod_bar
> > +    integer :: I = 3
> > +
> > +    goto 100
> > +
> > +    entry mod_foo
> > +    I = 33
> > +
> > +100 print *, I
> > +  end subroutine mod_bar
> > +end module mod
> > +
> > +
> > +subroutine bar(I,J,K,I1)
> > +  integer :: I,J,K,L,I1
> > +  integer :: A
> > +  real :: C
> > +
> > +  A = 0
> > +  C = 0.0
> > +
> > +  A = I + K + I1
> > +  goto 300
> > +
> > +  entry foo(J,K,L,I1)
> > +  A = J + K + L + I1
> > +
> > +200 C = J
> > +  goto 300
> > +
> > +  entry foobar(J)
> > +  goto 200
> > +
> > +300 A = C + 1
> > +  C = J * 1.5
> > +
> > +  return
> > +end subroutine
> > +
> > +program TestEntryPoint
> > +  use mod
> > +
> > +  call foo(11,22,33,44)
> > +  call bar(444,555,666,777)
> > +  call foobar(1)
> > +
> > +  call mod_foo()
> > +end program TestEntryPoint
> > --
> > 2.25.1
> >
> > Intel Deutschland GmbH
> > Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
> > Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
> > Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva
> > Chairperson of the Supervisory Board: Nicole Lau
> > Registered Office: Munich
> > Commercial Register: Amtsgericht Muenchen HRB 186928

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-13  9:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07  8:36 [PATCH v3 0/1] Fortran entry and DW_TAG_entry_point Nils-Christian Kempke
2022-07-07  8:36 ` [PATCH v3 1/1] dwarf, fortran: add support for DW_TAG_entry_point Nils-Christian Kempke
2022-07-11 13:02   ` Andrew Burgess
2022-07-13  9:38     ` Kempke, Nils-Christian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).