[v2,2/3] GDB: Bring back the handling of DW_CC_program

Message ID alpine.DEB.2.20.2303281941270.15477@tpp.orcam.me.uk
State Superseded
Headers
Series Fixes and improvements for the starting function |

Commit Message

Maciej W. Rozycki March 28, 2023, 6:48 p.m. UTC
  Fix a functional regression and restore the handling of DW_CC_program 
code of DW_AT_calling_convention attribute for determining the name of 
the starting function of the program where the DW_AT_main_subprogram 
attribute has not been provided, such as with Fortran code compiled with 
GCC versions 4.5.4 and below, or where DWARF version 3 or below has been 
requested.  Without it "main" is considered the starting function.  Cf. 
GCC PR fortran/43414.

Original code was removed with commit 6209cde4ddb8 ("Delete DWARF 
psymtab code"), and then an update to complement commit 81873cc81eff 
("[gdb/symtab] Support DW_AT_main_subprogram with -readnow.") has also 
been included here.
---
NB this was discovered with and is covered by the test case included with 
3/3; it has been verified to trigger with GCC 4.4.7 and will likely also 
with other configurations.

No change from v1.
---
 gdb/dwarf2/read.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)
  

Comments

Tom Tromey March 28, 2023, 7:26 p.m. UTC | #1
>>>>> "Maciej" == Maciej W Rozycki <macro@embecosm.com> writes:

Maciej> Original code was removed with commit 6209cde4ddb8 ("Delete DWARF 
Maciej> psymtab code"), and then an update to complement commit 81873cc81eff 
Maciej> ("[gdb/symtab] Support DW_AT_main_subprogram with -readnow.") has also 
Maciej> been included here.

Sorry about that.

Maciej> NB this was discovered with and is covered by the test case included with 
Maciej> 3/3; it has been verified to trigger with GCC 4.4.7 and will likely also 
Maciej> with other configurations.

Maciej> +/* Return TRUE if the given DIE is the program's "main".  */
Maciej> +
Maciej> +static bool
Maciej> +dwarf2_func_is_main_p (struct die_info *die, struct dwarf2_cu *cu)
Maciej> +{
Maciej> +  if (dwarf2_flag_true_p (die, DW_AT_main_subprogram, cu))
Maciej> +    return true;
Maciej> +  struct attribute *attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
Maciej> +  return (attr != nullptr
Maciej> +	  && attr->constant_value (DW_CC_normal) == DW_CC_program);

Should the DW_AT_calling_convention thing only apply to Fortran CUs?  I
have only ever heard of this approach being used by Fortran compilers.

Maciej> +	case DW_AT_calling_convention:
Maciej> +	  if (attr.constant_value (DW_CC_normal) == DW_CC_program)
Maciej> +	    *flags |= IS_MAIN;

Both the spots doing this check could probably use a comment explaining
why.  Maybe someday we'll get to remove this again.

Tom
  
Maciej W. Rozycki March 29, 2023, 5:54 p.m. UTC | #2
On Tue, 28 Mar 2023, Tom Tromey wrote:

> Maciej> +/* Return TRUE if the given DIE is the program's "main".  */
> Maciej> +
> Maciej> +static bool
> Maciej> +dwarf2_func_is_main_p (struct die_info *die, struct dwarf2_cu *cu)
> Maciej> +{
> Maciej> +  if (dwarf2_flag_true_p (die, DW_AT_main_subprogram, cu))
> Maciej> +    return true;
> Maciej> +  struct attribute *attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
> Maciej> +  return (attr != nullptr
> Maciej> +	  && attr->constant_value (DW_CC_normal) == DW_CC_program);
> 
> Should the DW_AT_calling_convention thing only apply to Fortran CUs?  I
> have only ever heard of this approach being used by Fortran compilers.

 Well, DWARFv4 has this:

'If the semantics of the language of the compilation unit containing the 
subroutine entry distinguishes between ordinary subroutines and 
subroutines that can serve as the "main program," that is, subroutines 
that cannot be called directly according to the ordinary calling 
conventions, then the debugging information entry for such a subroutine 
may have a calling convention attribute whose value is the constant 
DW_CC_program.'

as a part of the standard, and then:

'The DW_CC_program value is intended to support Fortran main programs 
which in some implementations may not be callable or which must be invoked 
in a special way.  It is not intended as a way of finding the entry 
address for the program.'

as an additional explanation (in italics, therefore outside the standard 
proper), so while it does mention Fortran, there's nothing specific to 
that language about the attribute value.

 Similarly for DW_AT_main_subprogram:

'11. A DW_AT_main_subprogram attribute, which is a flag whose presence 
     indicates that the compilation unit contains a subprogram that has 
     been identified as the starting function of the program.  If more 
     than one compilation unit contains this flag, any one of them may
     contain the starting function.'

again clarified thus:

'    Fortran has a PROGRAM statement which is used to specify and provide 
     a user-specified name for the main subroutine of a program.  C uses 
     the name "main" to identify the main subprogram of a program.  Some 
     other languages provide similar or other means to identify the main 
     subprogram of a program.'

 It's not clear to me why the DW_AT_main_subprogram has been added, but 
offhand I guess it is just because DW_AT_calling_convention carried two 
pieces of information at once, while there may exist languages where the 
starting function of the program does follow the ordinary calling
conventions and may therefore be called directly, yet we still want to 
be able to identify it.

 GCC sources have this note:

      /* DWARF 2 doesn't provide a way to identify a program's source-level
	entry point.  DW_AT_calling_convention attributes are only meant
	to describe functions' calling conventions.  However, lacking a
	better way to signal the Fortran main program, we used this for
	a long time, following existing custom.  Now, DWARF 4 has
	DW_AT_main_subprogram, which we add below, but some tools still
	rely on the old way, which we thus keep.  */

but I don't find it convincing as written; I find the DWARF wording and 
the name itself of DW_CC_program both indicating the attribute value does 
identify the starting function of the program even if in conjunction with 
a different calling convention.

 Then GCC does emit DW_CC_program for Fortran only, but so it does 
DW_AT_main_subprogram.  I can't speak for other compilers.

 So I think let's keep the arrangement generic i.e. universally enabled 
and let the producers decide how to shape debug information.

> Maciej> +	case DW_AT_calling_convention:
> Maciej> +	  if (attr.constant_value (DW_CC_normal) == DW_CC_program)
> Maciej> +	    *flags |= IS_MAIN;
> 
> Both the spots doing this check could probably use a comment explaining
> why.  Maybe someday we'll get to remove this again.

 It's not clear to me why we'd ever want to selectively remove this piece,
but I guess we can mention how the DWARF standard evolved giving the two 
ways to annotate the starting function of the program.  I'll post v3 with 
such an update.

 Thank you for your input.

  Maciej
  

Patch

Index: src/gdb/dwarf2/read.c
===================================================================
--- src.orig/gdb/dwarf2/read.c
+++ src/gdb/dwarf2/read.c
@@ -9994,6 +9994,18 @@  inherit_abstract_dies (struct die_info *
     compute_delayed_physnames (origin_cu);
 }
 
+/* Return TRUE if the given DIE is the program's "main".  */
+
+static bool
+dwarf2_func_is_main_p (struct die_info *die, struct dwarf2_cu *cu)
+{
+  if (dwarf2_flag_true_p (die, DW_AT_main_subprogram, cu))
+    return true;
+  struct attribute *attr = dwarf2_attr (die, DW_AT_calling_convention, cu);
+  return (attr != nullptr
+	  && attr->constant_value (DW_CC_normal) == DW_CC_program);
+}
+
 static void
 read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 {
@@ -10084,7 +10096,7 @@  read_func_scope (struct die_info *die, s
   newobj = cu->get_builder ()->push_context (0, lowpc);
   newobj->name = new_symbol (die, read_type_die (die, cu), cu, templ_func);
 
-  if (dwarf2_flag_true_p (die, DW_AT_main_subprogram, cu))
+  if (dwarf2_func_is_main_p (die, cu))
     set_objfile_main_name (objfile, newobj->name->linkage_name (),
 			   cu->lang ());
 
@@ -16141,6 +16153,11 @@  cooked_indexer::scan_attributes (dwarf2_
 	    *flags |= IS_MAIN;
 	  break;
 
+	case DW_AT_calling_convention:
+	  if (attr.constant_value (DW_CC_normal) == DW_CC_program)
+	    *flags |= IS_MAIN;
+	  break;
+
 	case DW_AT_declaration:
 	  is_declaration = attr.as_boolean ();
 	  break;