Support DWARF tags for C++11 variadic templates
Commit Message
This patch adds support for DW_TAG_GNU_formal_parameter_pack and
DW_TAG_GNU_template_parameter_pack, added to DWARF in March 2009 for
C++11 variadic templates[1].
They are not currently emitted thanks to a typo[2] but the fix is
trivial[3] and has been repeatedly submitted[4] to gcc; I'm not sure
what else I can do to get it accepted; regardless, anyone building their
own compiler can still make use of this.
This implementation synthesizes 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,
meaning that any debugger scripts will continue to work.
1. http://wiki.dwarfstd.org/index.php?title=C%2B%2B0x:_Variadic_templates
2. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70536
3. https://github.com/ecatmur/gcc/pull/5
4. https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609659.html
PR. https://sourceware.org/bugzilla/show_bug.cgi?id=17272
---
gdb/dwarf2/read.c | 71 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 67 insertions(+), 4 deletions(-)
Comments
On 04/02/2023 17:23, Ed Catmur wrote:
> This patch adds support for DW_TAG_GNU_formal_parameter_pack and
> DW_TAG_GNU_template_parameter_pack, added to DWARF in March 2009 for
> C++11 variadic templates[1].
>
> They are not currently emitted thanks to a typo[2] but the fix is
> trivial[3] and has been repeatedly submitted[4] to gcc; I'm not sure
> what else I can do to get it accepted; regardless, anyone building their
> own compiler can still make use of this.
>
> This implementation synthesizes 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,
> meaning that any debugger scripts will continue to work.
>
> 1. http://wiki.dwarfstd.org/index.php?title=C%2B%2B0x:_Variadic_templates
> 2. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70536
> 3. https://github.com/ecatmur/gcc/pull/5
> 4. https://gcc.gnu.org/pipermail/gcc-patches/2023-January/609659.html
> PR. https://sourceware.org/bugzilla/show_bug.cgi?id=17272
> ---
Hello Ed,
Thanks for working on this!
It would be nice if you could provide a testcase so we can validate the
solution even if we don't have an up-to-date compiler. There is a
convenient way to generate custom dwarf, using the "dwarf assembler" in
the GDB testsuite, but I don't think it supports
DW_TAG_GNU_formal_parameter_pack, so it might be easier to make a custom
assembly with it.
The patch also needs rebasing for proper regression testing, and I have
some style comments inlined.
> gdb/dwarf2/read.c | 71 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
> index ee4e7c1530..7591be5764 100644
> --- a/gdb/dwarf2/read.c
> +++ b/gdb/dwarf2/read.c
> @@ -12103,7 +12103,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;
> @@ -12152,6 +12153,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)
When testing a pointer for null, you should always be explicit, using
nullptr. Also, this like needs to be wrapped
> + {
> + struct symbol *arg = new_symbol (pack_die, NULL, cu);
> +
> + if (arg != NULL)
Same here, should be (arg != nullptr)
> + 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)
ditto
> + process_die (pack_die, cu);
> + }
> else
> process_die (child_die, cu);
> child_die = child_die->sibling;
> @@ -16639,6 +16657,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;
> + }
Shouldn't this block also set ftype->has_varargs to true?
Also, this could fail if there could be DW_TAG_formal_parameters after
the GNU version, which is mentioend at the end of the dwarf wiki you
linked. I think there should be a depth variable that knows if we should
go back up to the parent die once we reach the final sibling.
And there is a FIXME comment right before this loop that mentions that
GDB can't handle vararg functions that can probably be updated with this
patch.
> else if (child_die->tag == DW_TAG_unspecified_parameters)
> ftype->set_has_varargs (true);
>
> @@ -16714,6 +16737,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;
> + }
Same here for being able to return to parent DIE.
> child_die = child_die->sibling;
> }
> }
> @@ -20847,13 +20875,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)
explicit nullptr check again and wrapping to:
if (die->tag == DW_TAG_formal_parameter
&& die->parent == nullpre
&& 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 ())
> @@ -21073,6 +21104,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.
> @@ -22081,8 +22114,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
> @@ -22111,9 +22147,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
attr == nullptr && diw->parent != nullptr
> + && (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;
When GDB finds a template parameter that doesn't have a name, it instead
returns <unnamed N> to help users understand what's going on (see:
https://sourceware.org/pipermail/gdb-patches/2022-August/191528.html).
If this behavior is changed for GNU tags, it will look like a bug to
users, it would be better to continue calling unnamed_template_tag_name
in this exit.
@@ -12103,7 +12103,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;
@@ -12152,6 +12153,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;
@@ -16639,6 +16657,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);
@@ -16714,6 +16737,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;
}
}
@@ -20847,13 +20875,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 ())
@@ -21073,6 +21104,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.
@@ -22081,8 +22114,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
@@ -22111,9 +22147,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. */