diff mbox series

Support DW_TAG_GNU_formal_parameter_pack and DW_TAG_GNU_template_parameter_pack.

Message ID 20230115224242.29922-1-ed@catmur.uk
State New
Headers
Series Support DW_TAG_GNU_formal_parameter_pack and DW_TAG_GNU_template_parameter_pack. |

Commit Message

Ed Catmur Jan. 15, 2023, 10:42 p.m. UTC 
  https://sourceware.org/bugzilla/show_bug.cgi?id=17272

Synthesize type and parameter names as T#n, p#n e.g. Args#1, args#1.

This is a pretty simple approach but it seems to work OK and is compatible with the old style type and parameter names emitted by old versions of gcc and when it's writing stabs+ format.
---
 gdb/dwarf2/read.c | 71 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 67 insertions(+), 4 deletions(-)

Comments

Alexandra Petlanova Hajkova Jan. 23, 2023, 10 a.m. UTC | #1 
On Sun, Jan 15, 2023 at 11:43 PM Ed Catmur <ed@catmur.uk> wrote:

> https://sourceware.org/bugzilla/show_bug.cgi?id=17272
>
> Synthesize type and parameter names as T#n, p#n e.g. Args#1, args#1.
>
> This is a pretty simple approach but it seems to work OK and is compatible
> with the old style type and parameter names emitted by old versions of gcc
> and when it's writing stabs+ format.
> ---
>
>
Great patch but maybe your commit message could be slightly  more
informative.
Ed Catmur Feb. 4, 2023, 4:32 p.m. UTC | #2 
On Tue, 17 Jan 2023, at 16:41, Alexandra Petlanova Hajkova wrote:
> 
> 
> On Sun, Jan 15, 2023 at 11:43 PM Ed Catmur <ed@catmur.uk> wrote:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=17272
>> 
>> Synthesize type and parameter names as T#n, p#n e.g. Args#1, args#1.
>> 
>> This is a pretty simple approach but it seems to work OK and is compatible with the old style type and parameter names emitted by old versions of gcc and when it's writing stabs+ format.
>> ---
> 
> Great patch but maybe your commit message could be slightly  more informative.

Thanks, resent with a better description: https://sourceware.org/pipermail/gdb-patches/2023-February/196618.html
Alexandra Petlanova Hajkova Feb. 7, 2023, 5:55 a.m. UTC | #3 
On Sat, Feb 4, 2023 at 5:33 PM Ed Catmur <ed@catmur.uk> wrote:

> On Tue, 17 Jan 2023, at 16:41, Alexandra Petlanova Hajkova wrote:
> >
> >
> > On Sun, Jan 15, 2023 at 11:43 PM Ed Catmur <ed@catmur.uk> wrote:
> >> https://sourceware.org/bugzilla/show_bug.cgi?id=17272
> >>
> >> Synthesize type and parameter names as T#n, p#n e.g. Args#1, args#1.
> >>
> >> This is a pretty simple approach but it seems to work OK and is
> compatible with the old style type and parameter names emitted by old
> versions of gcc and when it's writing stabs+ format.
> >> ---
> >
> > Great patch but maybe your commit message could be slightly  more
> informative.
>
> Thanks, resent with a better description:
> https://sourceware.org/pipermail/gdb-patches/2023-February/196618.html


Good to go from my point of view.
Tom Tromey May 30, 2023, 5:28 p.m. UTC | #4 
>>>>> "Ed" == Ed Catmur <ed@catmur.uk> writes:

Hi.  I'm sorry about the long delay on this review.
Unfortunately in gdb you have to send pings for patches.

Ed> https://sourceware.org/bugzilla/show_bug.cgi?id=17272
Ed> Synthesize type and parameter names as T#n, p#n e.g. Args#1, args#1.

Ed> This is a pretty simple approach but it seems to work OK and is
Ed> compatible with the old style type and parameter names emitted by
Ed> old versions of gcc and when it's writing stabs+ format.

Does stabs even handle template parameter packs?

Also are synthesized names really necessary?  If they aren't required in
DWARF perhaps they can just be nullptr.

Ed> -      attr = dwarf2_attr (die,
Ed> +      struct die_info *line_file_die = die;
Ed> +      if (die->tag == DW_TAG_formal_parameter && die->parent && die->parent->tag == DW_TAG_GNU_formal_parameter_pack)

This line looks too long.  There are other minor formatting issues as
well, for example in the GNU style there is usually a space before an
open paren.

Ed> +	  size = snprintf(NULL, 0, "%s#%d", parent_name, ordinal) + 1;
Ed> +	  name = ((char *) obstack_alloc (&cu->per_objfile->per_bfd->obstack, size));
Ed> +	  snprintf(name, size, "%s#%d", parent_name, ordinal);

gdb doesn't generally use snprintf.  There are other options, like
printing to a string and then copying it.

A patch like this needs a test case.

Also I'm curious if this works ok with the Python API.

Tom
diff mbox series

Patch

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 851852404eb..cee9b7c4e02 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -12112,7 +12112,8 @@  read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
   for (child_die = die->child; child_die; child_die = child_die->sibling)
     {
       if (child_die->tag == DW_TAG_template_type_param
-	  || child_die->tag == DW_TAG_template_value_param)
+	  || child_die->tag == DW_TAG_template_value_param
+	  || child_die->tag == DW_TAG_GNU_template_parameter_pack)
 	{
 	  templ_func = new (&objfile->objfile_obstack) template_symbol;
 	  templ_func->subclass = SYMBOL_TEMPLATE;
@@ -12161,6 +12162,23 @@  read_func_scope (struct die_info *die, struct dwarf2_cu *cu)
 	      if (arg != NULL)
 		template_args.push_back (arg);
 	    }
+	  else if (child_die->tag == DW_TAG_GNU_template_parameter_pack)
+	    {
+	      struct die_info *pack_die;
+	      for (pack_die = child_die->child; pack_die; pack_die = pack_die->sibling)
+		{
+		  struct symbol *arg = new_symbol (pack_die, NULL, cu);
+
+		  if (arg != NULL)
+		    template_args.push_back (arg);
+		}
+	    }
+	  else if (child_die->tag == DW_TAG_GNU_formal_parameter_pack)
+	    {
+	      struct die_info *pack_die;
+	      for (pack_die = child_die->child; pack_die; pack_die = pack_die->sibling)
+		process_die (pack_die, cu);
+	    }
 	  else
 	    process_die (child_die, cu);
 	  child_die = child_die->sibling;
@@ -16648,6 +16666,11 @@  read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
 	{
 	  if (child_die->tag == DW_TAG_formal_parameter)
 	    nparams++;
+	  else if (child_die->tag == DW_TAG_GNU_formal_parameter_pack)
+	    {
+	      child_die = child_die->child;
+	      continue;
+	    }
 	  else if (child_die->tag == DW_TAG_unspecified_parameters)
 	    ftype->set_has_varargs (true);
 
@@ -16723,6 +16746,11 @@  read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu)
 	      ftype->field (iparams).set_type (arg_type);
 	      iparams++;
 	    }
+	  else if (child_die->tag == DW_TAG_GNU_formal_parameter_pack)
+	    {
+	      child_die = child_die->child;
+	      continue;
+	    }
 	  child_die = child_die->sibling;
 	}
     }
@@ -20844,13 +20872,16 @@  new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	sym->set_type (type);
       else
 	sym->set_type (die_type (die, cu));
-      attr = dwarf2_attr (die,
+      struct die_info *line_file_die = die;
+      if (die->tag == DW_TAG_formal_parameter && die->parent && die->parent->tag == DW_TAG_GNU_formal_parameter_pack)
+	line_file_die = die->parent;
+      attr = dwarf2_attr (line_file_die,
 			  inlined_func ? DW_AT_call_line : DW_AT_decl_line,
 			  cu);
       if (attr != nullptr)
 	sym->set_line (attr->constant_value (0));
 
-      attr = dwarf2_attr (die,
+      attr = dwarf2_attr (line_file_die,
 			  inlined_func ? DW_AT_call_file : DW_AT_decl_file,
 			  cu);
       if (attr != nullptr && attr->is_nonnegative ())
@@ -21070,6 +21101,8 @@  new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    list_to_add = cu->list_in_scope;
 	  }
 	  break;
+	case DW_TAG_GNU_formal_parameter_pack:
+	  break;
 	case DW_TAG_unspecified_parameters:
 	  /* From varargs functions; gdb doesn't seem to have any
 	     interest in this information, so just ignore it for now.
@@ -22078,8 +22111,11 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
   if (attr_name == nullptr
       && die->tag != DW_TAG_namespace
       && die->tag != DW_TAG_class_type
+      && die->tag != DW_TAG_formal_parameter
       && die->tag != DW_TAG_interface_type
       && die->tag != DW_TAG_structure_type
+      && die->tag != DW_TAG_template_type_param
+      && die->tag != DW_TAG_template_value_param
       && die->tag != DW_TAG_namelist
       && die->tag != DW_TAG_union_type
       && die->tag != DW_TAG_template_type_param
@@ -22108,9 +22144,36 @@  dwarf2_name (struct die_info *die, struct dwarf2_cu *cu)
 	return attr_name;
       return CP_ANONYMOUS_NAMESPACE_STR;
 
-    /* DWARF does not actually require template tags to have a name.  */
+    case DW_TAG_formal_parameter:
     case DW_TAG_template_type_param:
     case DW_TAG_template_value_param:
+      if (!attr
+	  && die->parent
+	  && (die->parent->tag == DW_TAG_GNU_formal_parameter_pack
+	      || die->parent->tag == DW_TAG_GNU_template_parameter_pack))
+	{
+	  const char *parent_name;
+	  int ordinal = 0;
+	  struct die_info *child_die;
+	  size_t size;
+	  char *name;
+	  parent_name = dwarf2_name(die->parent, cu);
+	  if (!parent_name)
+	    return NULL;
+	  for (child_die = die->parent->child; child_die != die; child_die = child_die->sibling)
+	    ++ordinal;
+	  size = snprintf(NULL, 0, "%s#%d", parent_name, ordinal) + 1;
+	  name = ((char *) obstack_alloc (&cu->per_objfile->per_bfd->obstack, size));
+	  snprintf(name, size, "%s#%d", parent_name, ordinal);
+	  return name;
+	}
+      if (die->tag == DW_TAG_formal_parameter)
+	{
+	  if (!attr || attr_name == NULL)
+	    return NULL;
+	  break;
+	}
+      /* DWARF does not actually require template tags to have a name.  */
       if (attr_name == nullptr)
 	return unnamed_template_tag_name (die, cu);
       /* FALLTHROUGH.  */