From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 78787 invoked by alias); 4 Nov 2019 20:42:14 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 78778 invoked by uid 89); 4 Nov 2019 20:42:14 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=dies, settled X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Nov 2019 20:42:03 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572900122; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=N+9uVtStn3ik0nHHLTGIlj3NFi4n6UAlX6xbrfPXRBg=; b=aP6v2UES7WOSt5qcR98Y1ElUm86j9PHhVdZ12+jWjXVboI94de/j179VagDlpDgATO0+oi oEtheflB7OcecXf3oy/052e0RQiIQZK8EP81ZvnB+ASTlmSxp9nb/TJK3+XEBSLzzHL3os 9eLTvKGzo6/MAFwLfshYahnKMiGqcBQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-410-3T20vjFYOv-nDsZXBCtZMA-1; Mon, 04 Nov 2019 15:41:52 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5EDA6477; Mon, 4 Nov 2019 20:41:51 +0000 (UTC) Received: from f29-4.lan (ovpn-116-78.phx2.redhat.com [10.3.116.78]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 35500600C4; Mon, 4 Nov 2019 20:41:51 +0000 (UTC) Date: Mon, 04 Nov 2019 20:42:00 -0000 From: Kevin Buettner To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Fix BZ 25065 - Ensure that physnames are computed for inherited DIEs Message-ID: <20191104134150.0c090b38@f29-4.lan> In-Reply-To: <20191022152307.37dea0d1@f29-4.lan> References: <20191014001842.27413-1-kevinb@redhat.com> <20191014001842.27413-2-kevinb@redhat.com> <8c073683bc0b0a8e7918fdfb346cbb03@polymtl.ca> <20191015092743.7037f47c@f29-4.lan> <20191017180851.7db69958@f29-4.lan> <8b301205-e8f6-ead5-0484-eacfb82e5b7a@polymtl.ca> <20191022152307.37dea0d1@f29-4.lan> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00103.txt.bz2 Ping. On Tue, 22 Oct 2019 15:23:07 -0700 Kevin Buettner wrote: > On Fri, 18 Oct 2019 11:07:31 -0400 > Simon Marchi wrote: >=20 > > > I looked over your patch; it looks reasonable to me. If we go that > > > route, I like the idea of introducing a dwarf2_cu_processing_context > > > struct. (But see below for some later misgivings that I have/had.)= =20=20=20=20 > >=20 > > Ok, I can try to make something cleaner, but I don't know when it > > would be ready, and I wouldn't want that to block the GDB 9.1 branch > > creation (or release) for that. Would you like to still push your > > patch (or a perhaps updated version of it) so that we have the fix > > in GDB 9.1?=20=20 >=20 > [...] >=20 > > > There may be other concerns too; I'm certain that I didn't look at all > > > of the ways that CU is used in dwarf2_physname and its callees.=20=20= =20=20 > >=20 > > I don't think it's humanly possible to manually check all the > > possible branches this code can take. I say, let's do a quick pass > > to check for the obvious (like what you found above), but otherwise > > I'm fine with this patch, it already makes things better than they > > are now.=20=20 >=20 > My testing shows that the patch below still fixes the problem while > also avoiding the poential problems of passing a CU to > compute_delayed_physnames() which is different from the CU of the > methods for which want to compute physnames. >=20 > I think that this patch is safer than the one I originally proposed > and is, therefore, a better short term solution. >=20 > What do you think? >=20 > - - - >=20 > Fix BZ 25065 - Ensure that physnames are computed for inherited DIEs >=20 > This is a fix for BZ 25065. >=20 > GDB segfaults when running either gdb.cp/subtypes.exp or > gdb.cp/local.exp in conjunction with using the -flto compiler/linker > flag. >=20 > A much simpler program, which was used to help create the test for > this fix, is: >=20 > -- doit.cc -- > int main() > { > class Foo { > public: > int doit () > { > return 0; > } > }; >=20 > Foo foo; >=20 > return foo.doit (); > } > -- end doit.cc -- >=20 > gcc -o doit -flto -g doit.cc > gdb -q doit > Reading symbols from doit... > (gdb) ptype main::Foo > type =3D class Foo { > Segmentation fault (core dumped) >=20 > The segfault occurs due to a NULL physname in > c_type_print_base_struct_union in c-typeprint.c. Specifically, > calling is_constructor_name() eventually causes the SIGSEGV is this > code in c-typeprint.c: >=20 > const char *physname =3D TYPE_FN_FIELD_PHYSNAME (f, j); > int is_full_physname_constructor =3D > TYPE_FN_FIELD_CONSTRUCTOR (f, j) > || is_constructor_name (physname) > || is_destructor_name (physname) > || method_name[0] =3D=3D '~'; >=20 > However, looking at compute_delayed_physnames(), we see that > the TYPE_FN_FIELD_PHYSNAME field should never be NULL. This > field will be set to "" for NULL physnames: >=20 > physname =3D dwarf2_physname (mi.name, mi.die, cu); > TYPE_FN_FIELD_PHYSNAME (fn_flp->fn_fields, mi.index) > =3D physname ? physname : ""; >=20 > For this particular case, it turns out that compute_delayed_physnames > wasn't being called, which left TYPE_FN_FIELD_PHYSNAME set to the NULL > value that it started with when that data structure was allocated. >=20 > The place to fix it, I think, is towards the end of > inherit_abstract_dies(). >=20 > My first attempt at fix caused the origin CU's method_list (which is > simply the list of methods whose physnames still need to be computed) > to be added to the CU which is doing the inheriting. One drawback > with this approach is that compute_delayed_physnames is (eventually) > called with a CU that's different than the CU in which the methods > were found. It's not clear whether this will cause problems or not. >=20 > A safer approach, which is what I ultimately settled on, is to call > compute_delayed_physnames() from inherit_abstract_dies(). One > potential drawback is that all needed types might not be known at that > point. However, in my testing, I haven't seen a problem along these > lines. >=20 > gdb/ChangeLog: >=20 > * dwarf2read.c (inherit_abstract_dies): Ensure that delayed > physnames are computed for inherited DIEs. >=20=20=20=20=20 > Change-Id: I6c6ffe96b301a9daab9f653956b89e3a33fa9445 > --- > gdb/dwarf2read.c | 3 +++ > 1 file changed, 3 insertions(+) >=20 > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c > index ee9df34ed2..976153640a 100644 > --- a/gdb/dwarf2read.c > +++ b/gdb/dwarf2read.c > @@ -13666,6 +13666,9 @@ inherit_abstract_dies (struct die_info *die, stru= ct dwarf2_cu *cu) > origin_child_die =3D sibling_die (origin_child_die); > } > origin_cu->list_in_scope =3D origin_previous_list_in_scope; > + > + if (cu !=3D origin_cu) > + compute_delayed_physnames (origin_cu); > } >=20=20 > static void >=20