* Re: [PATCH] Support DWARF tags for C++11 variadic templates
2023-02-04 16:23 [PATCH] Support DWARF tags for C++11 variadic templates Ed Catmur
@ 2023-02-22 17:00 ` Bruno Larsen
0 siblings, 0 replies; 2+ messages in thread
From: Bruno Larsen @ 2023-02-22 17:00 UTC (permalink / raw)
To: Ed Catmur, gdb-patches
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.
--
Cheers,
Bruno
> + 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. */
^ permalink raw reply [flat|nested] 2+ messages in thread