From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 2C1F23858D33 for ; Wed, 22 Feb 2023 17:00:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2C1F23858D33 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677085208; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6FXRNSZs+N/ogd4IWIWIwa4bpsleMG+soMICV7Oby4w=; b=JhcZajFDAPJ2Sl+8rBCCBYx2XVtG48ZlqeYPPLTLMUmyha35cRlto5+odZRTNF9Jmp4awI M6PTwfxuWGX67lK3zRnM/dGMW8q7FGghxZ9YAWYvTSll7RK2Tjso2SOrlJEiiG/rAhULIq hjKqlrkhWe0sLqcDVj5G1UUL2x7ABAw= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-645-1nVr0i2wOKG1ItIJ7GE5Rw-1; Wed, 22 Feb 2023 12:00:05 -0500 X-MC-Unique: 1nVr0i2wOKG1ItIJ7GE5Rw-1 Received: by mail-wr1-f71.google.com with SMTP id 4-20020a5d47a4000000b002c5699ff08aso1755015wrb.9 for ; Wed, 22 Feb 2023 09:00:04 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6FXRNSZs+N/ogd4IWIWIwa4bpsleMG+soMICV7Oby4w=; b=0+xywl9NJy7R4vB3vO7u/Lo1RyimcwsoADLNn2GAZf0ps08gmGoZ99lZd6tdWYLK4j HF0cCIZJoqYDchyntc+tJ4CqsFtFq5dmd1WAOOMoQIOBHn3Pdvj0T9lL+EZxKH6oHmn0 qA5C8umvUBpL8K4bz3L5iJnspimzyk+Sw1DGu9Nw+FxkO0SWln3qbMYiX3jlMg6WHLl4 dEJbefMofZ5GNAFx3O3ldSiPJ6si6sq5V4eYPYk9S2T3I15LHUyO8kSTUDBDwzoNuoiq 8mIIUh1EGybas8QLT0T4GSKGuuZi1D/NEDzaMek/9WaoLb062M9CC7lMIaVq2LQ1GU7o ZS5A== X-Gm-Message-State: AO0yUKXMxXgT67GhTLTTdYGOEPlkfoaZNPIYQqgSsSXPNSHxl9pmW4Go YJ0h1x5lI2Lv1Tfdqyz0E+SEH06Q3+YecCXtQYcQ55CKuPD1Z4+oxzTbyhX8IcJHfistoCOGNjs IDBPLRNhy8l95ozrwowXq39yUnxA= X-Received: by 2002:a5d:4570:0:b0:2c5:52fc:ed1a with SMTP id a16-20020a5d4570000000b002c552fced1amr6748883wrc.55.1677085203377; Wed, 22 Feb 2023 09:00:03 -0800 (PST) X-Google-Smtp-Source: AK7set9ZnE82rYaWQ0oQU3zRhGXxiBdsqKe0WcJZirbyHD3E1JcURdWy6nOwVB3cli2lSyiDA/ElQw== X-Received: by 2002:a5d:4570:0:b0:2c5:52fc:ed1a with SMTP id a16-20020a5d4570000000b002c552fced1amr6748865wrc.55.1677085202998; Wed, 22 Feb 2023 09:00:02 -0800 (PST) Received: from [10.43.2.105] (nat-pool-brq-t.redhat.com. [213.175.37.10]) by smtp.gmail.com with ESMTPSA id x8-20020adfdd88000000b002c58ca558b6sm7327677wrl.88.2023.02.22.09.00.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Feb 2023 09:00:02 -0800 (PST) Message-ID: Date: Wed, 22 Feb 2023 18:00:01 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [PATCH] Support DWARF tags for C++11 variadic templates To: Ed Catmur , gdb-patches@sourceware.org References: <20230204162321.2640-1-ed@catmur.uk> From: Bruno Larsen In-Reply-To: <20230204162321.2640-1-ed@catmur.uk> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,KAM_STOCKGEN,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 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. */