From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28134 invoked by alias); 7 Feb 2015 23:06:03 -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 28122 invoked by uid 89); 7 Feb 2015 23:06:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pd0-f175.google.com Received: from mail-pd0-f175.google.com (HELO mail-pd0-f175.google.com) (209.85.192.175) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 07 Feb 2015 23:06:00 +0000 Received: by pdbnh10 with SMTP id nh10so19936097pdb.12 for ; Sat, 07 Feb 2015 15:05:59 -0800 (PST) X-Received: by 10.66.102.2 with SMTP id fk2mr16511658pab.149.1423350359122; Sat, 07 Feb 2015 15:05:59 -0800 (PST) Received: from seba.sebabeach.org.gmail.com (173-13-178-53-sfba.hfc.comcastbusiness.net. [173.13.178.53]) by mx.google.com with ESMTPSA id qc8sm11959057pdb.86.2015.02.07.15.05.57 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 07 Feb 2015 15:05:58 -0800 (PST) From: Doug Evans To: "Pierre Muller" Cc: Subject: Re: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE References: <54d3aa6b.a23d460a.0d37.0908SMTPIN_ADDED_BROKEN@mx.google.com> Date: Sat, 07 Feb 2015 23:06:00 -0000 In-Reply-To: <54d3aa6b.a23d460a.0d37.0908SMTPIN_ADDED_BROKEN@mx.google.com> (Pierre Muller's message of "Thu, 5 Feb 2015 18:37:00 +0100") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2015-02/txt/msg00176.txt.bz2 "Pierre Muller" writes: > Hi all, > > >> -----Message d'origine----- >> De=C2=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- >> owner@sourceware.org] De la part de Doug Evans >> Envoy=C3=A9=C2=A0: lundi 26 janvier 2015 01:07 >> =C3=80=C2=A0: gdb-patches@sourceware.org; gaius@glam.ac.uk >> Objet=C2=A0: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: >> TYPE_SPECIFIC_SELF_TYPE >>=20 >> Hi. >>=20 >> This patch moves TYPE_SELF_TYPE into new field type_specific.self_type >> for MEMBERPTR,METHODPTR types, and into type_specific.func_stuff >> for METHODs, and then updates everything to use that. >> TYPE_CODE_METHOD could share some things with TYPE_CODE_FUNC >> (e.g. TYPE_NO_RETURN) and it seemed simplest to keep them together. >>=20 >> Moving TYPE_SELF_TYPE into type_specific.func_stuff for >> TYPE_CODE_METHOD >> is also nice because when we allocate space for function types we >> assume >> they're TYPE_CODE_FUNCs. If TYPE_CODE_METHODs don't need or use that >> space then that space would be wasted, and cleaning that up would >> involve >> more invasive changes. >>=20 >> In order to catch errant uses I've added accessor functions >> that do some checking. >>=20 >> One can no longer assign to TYPE_SELF_TYPE like this: >>=20 >> TYPE_SELF_TYPE (foo) =3D bar; >>=20 >> One instead has to do: >>=20 >> set_type_self_type (foo, bar); >>=20 >> But I've left reading of the type to the macro: >>=20 >> bar =3D TYPE_SELF_TYPE (foo); >>=20 >> I could add SET_TYPE_SELF_TYPE as a wrapper on set_type_self_type >> if you prefer that. >>=20 >> In order to discourage bypassing the TYPE_SELF_TYPE macro >> I've named the underlying function that implements it > .... >> * stabsread.c (read_member_functions): Mark methods with >> TYPE_CODE_METHOD, not TYPE_CODE_FUNC. Update setting of >> TYPE_SELF_TYPE. > ..... >> diff --git a/gdb/stabsread.c b/gdb/stabsread.c >> index 1f46f75..423c442 100644 >> --- a/gdb/stabsread.c >> +++ b/gdb/stabsread.c >> @@ -2376,14 +2376,21 @@ read_member_functions (struct field_info *fip, >> char **pp, struct type *type, >> p++; >> } >>=20 >> - /* If this is just a stub, then we don't have the real name >> here. */ >> + /* These are methods, not functions. */ >> + if (TYPE_CODE (new_sublist->fn_field.type) =3D=3D TYPE_CODE_FUNC) >> + TYPE_CODE (new_sublist->fn_field.type) =3D TYPE_CODE_METHOD; >> + else >> + gdb_assert (TYPE_CODE (new_sublist->fn_field.type) >> + =3D=3D TYPE_CODE_METHOD); >>=20 >> + /* If this is just a stub, then we don't have the real name >> here. */ >> if (TYPE_STUB (new_sublist->fn_field.type)) >> { >> if (!TYPE_SELF_TYPE (new_sublist->fn_field.type)) > I suspect this is the part that generates the failure=20 > I saw when trying to test my pascal patch that used stabs debugging > information. >=20=20=20=20 > internal_type_self_type generates an internal error=20=20=20 > it does not simply return NULL... > >> - TYPE_SELF_TYPE (new_sublist->fn_field.type) =3D type; >> + set_type_self_type (new_sublist->fn_field.type, type); >> new_sublist->fn_field.is_stub =3D 1; >> } >> + >> new_sublist->fn_field.physname =3D savestring (*pp, p - *pp); >> *pp =3D p + 1; > > The patch below removes the internal error, > but I am not sure it is the correct fix... > Maybe set_type_self_type should be called unconditionally. > > Likewise, the line: > valops.c:2547: gdb_assert (TYPE_SELF_TYPE (fns_ptr[0].type) !=3D NULL); > is not compatible with your new internal_type_self_type as this > new function never returns NULL.... > > > Pierre Muller > > $ git diff > diff --git a/gdb/stabsread.c b/gdb/stabsread.c > index 2a160c5..392fdb2 100644 > --- a/gdb/stabsread.c > +++ b/gdb/stabsread.c > @@ -2386,7 +2386,7 @@ read_member_functions (struct field_info *fip, char > **pp, struct type *type, > /* If this is just a stub, then we don't have the real name her= e. > */ > if (TYPE_STUB (new_sublist->fn_field.type)) > { > - if (!TYPE_SELF_TYPE (new_sublist->fn_field.type)) > + if (TYPE_SPECIFIC_FIELD (new_sublist->fn_field.type) =3D=3D > TYPE_SPECIFIC_NONE) > set_type_self_type (new_sublist->fn_field.type, type); > new_sublist->fn_field.is_stub =3D 1; > } Thanks for the testcase! I found the culprit. I added some logging to allocate_stub_method, and running the entire testsuite with stabs (-gstabs+) does not exercise this function. Bleah. I don't know stabs well enough to want to spend time coming up with a testcase. Is it possible for you to write one from your test? Here's a patch. You're patch is on the right track, TYPE_SPECIFIC_FIELD can legitimately be TYPE_SPECIFIC_NONE if the field hasn't been initialized yet. I like the patch below as it's more general. 2015-02-07 Doug Evans * gdbtypes.c (internal_type_self_type): If TYPE_SPECIFIC_FIELD hasn't been initialized yet, return NULL. diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 140fc6f..44483d4 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -1198,9 +1198,13 @@ internal_type_self_type (struct type *type) { case TYPE_CODE_METHODPTR: case TYPE_CODE_MEMBERPTR: + if (TYPE_SPECIFIC_FIELD (type) =3D=3D TYPE_SPECIFIC_NONE) + return NULL; gdb_assert (TYPE_SPECIFIC_FIELD (type) =3D=3D TYPE_SPECIFIC_SELF_TYP= E); return TYPE_MAIN_TYPE (type)->type_specific.self_type; case TYPE_CODE_METHOD: + if (TYPE_SPECIFIC_FIELD (type) =3D=3D TYPE_SPECIFIC_NONE) + return NULL; gdb_assert (TYPE_SPECIFIC_FIELD (type) =3D=3D TYPE_SPECIFIC_FUNC); return TYPE_MAIN_TYPE (type)->type_specific.func_stuff->self_type; default: