From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22553 invoked by alias); 8 Feb 2015 14:48:24 -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 22541 invoked by uid 89); 8 Feb 2015 14:48:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.6 required=5.0 tests=AWL,BAYES_00,MSGID_MULTIPLE_AT autolearn=no version=3.3.2 X-HELO: mailhost.u-strasbg.fr Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.222.212) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 08 Feb 2015 14:48:21 +0000 Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antispam (Postfix) with ESMTP id 956F11404AE; Sun, 8 Feb 2015 15:48:18 +0100 (CET) Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id 850D1140BE6; Sun, 8 Feb 2015 15:48:18 +0100 (CET) Received: from lmr.u-strasbg.fr (lmr2.u-strasbg.fr [172.30.21.2]) by mr2.u-strasbg.fr (Postfix) with ESMTP id 64FF61404AE; Sun, 8 Feb 2015 15:48:16 +0100 (CET) Received: from lmr.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id 33DF3E2; Sun, 8 Feb 2015 15:48:16 +0100 (CET) Received: from E6510Muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (Authenticated sender: mullerp) by lmr2.u-strasbg.fr (Postfix) with ESMTPSA id 90C3FDB; Sun, 8 Feb 2015 15:48:13 +0100 (CET) From: "Pierre Muller" To: "'Doug Evans'" Cc: References: <54d3aa6b.a23d460a.0d37.0908SMTPIN_ADDED_BROKEN@mx.google.com> In-Reply-To: Subject: RE: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE Date: Sun, 08 Feb 2015 14:48:00 -0000 Message-ID: <000001d043ae$43fd0410$cbf70c30$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-SW-Source: 2015-02/txt/msg00181.txt.bz2 Hi, > -----Message d'origine----- > De : gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Doug Evans > Envoy=C3=A9 : dimanche 8 f=C3=A9vrier 2015 00:05 > =C3=80 : Pierre Muller > Cc : gdb-patches@sourceware.org > Objet : Re: [PATCH 4/5] Remove struct > main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE >=20 > "Pierre Muller" writes: > > Hi all, > > > > > >> -----Message d'origine----- > >> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches- > >> owner@sourceware.org] De la part de Doug Evans > >> Envoy=C3=A9 : lundi 26 janvier 2015 01:07 > >> =C3=80 : gdb-patches@sourceware.org; gaius@glam.ac.uk > >> Objet : [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: > >> TYPE_SPECIFIC_SELF_TYPE > >> > >> Hi. > >> > >> 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. > >> > >> 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. > >> > >> In order to catch errant uses I've added accessor functions > >> that do some checking. > >> > >> One can no longer assign to TYPE_SELF_TYPE like this: > >> > >> TYPE_SELF_TYPE (foo) =3D bar; > >> > >> One instead has to do: > >> > >> set_type_self_type (foo, bar); > >> > >> But I've left reading of the type to the macro: > >> > >> bar =3D TYPE_SELF_TYPE (foo); > >> > >> I could add SET_TYPE_SELF_TYPE as a wrapper on set_type_self_type > >> if you prefer that. > >> > >> 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++; > >> } > >> > >> - /* 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); > >> > >> + /* 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 > > I saw when trying to test my pascal patch that used stabs debugging > > information. > > > > internal_type_self_type generates an internal error > > 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 > here. > > */ > > 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; > > } >=20 > Thanks for the testcase! I found the culprit. >=20 > I added some logging to allocate_stub_method, and running the entire > testsuite with stabs (-gstabs+) does not exercise this function. Bleah. >=20 > 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? Yes,=20 I also tried with g++ and never got to the same internal_error. I have a minimal pascal source, and extracted from it a subset of the stabs indormation. I have put both at the end of this email. =20 > 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. I agree with you that this patch seems much secure, as it allows to use TYPE_SELF_TYPE as a test as before. =20 > 2015-02-07 Doug Evans >=20 > * gdbtypes.c (internal_type_self_type): If TYPE_SPECIFIC_FIELD > hasn't > been initialized yet, return NULL. >=20 > 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_TYPE); > 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_TYE (type)->type_specific.func_stuff- > >self_type; > default: Simple pascal source: muller@gcc45:~/pas/trunk/fpcsrc/ide$ cat test-obj.pp type tobj =3D object constructor create; end; constructor tobj.create; begin end; var t : tobj; begin t.create; end. Compiled with free pascal using ppc386 -al -gs test-obj.pp the option -al generates an intermediate assembler file called test-obj.s that is converted into an object file using GNU assembler for i386 cpu. I reduced the generated file, keeping only=20 the global stabs information. muller@gcc45:~/pas/trunk/fpcsrc/ide$ cat test-min.s .file "test-obj.pp" # Begin asmlist al_begin .section .text .globl DEBUGSTART_P$PROGRAM .type DEBUGSTART_P$PROGRAM,@object DEBUGSTART_P$PROGRAM: .stabs "/home/muller/pas/trunk/fpcsrc/ide/",100,0,0,.Lf3 .stabs "test-obj.pp",100,0,0,.Lf3 .Lf3: # End asmlist al_begin # Begin asmlist al_stabs .section .data .globl DEBUGINFO_P$PROGRAM .type DEBUGINFO_P$PROGRAM,@object DEBUGINFO_P$PROGRAM: # Defs - Begin unit SYSTEM has index 1 .stabs "void:t2=3D2",128,0,0,0 .stabs "LONGBOOL:t3=3D-23;",128,0,0,0 .stabs "POINTER:t4=3D*2",128,0,0,0 # Defs - End unit SYSTEM has index 1 # Defs - Begin unit FPINTRES has index 2 # Defs - End unit FPINTRES has index 2 # Defs - Begin unit SI_PRC has index 2 # Defs - End unit SI_PRC has index 2 # Defs - Begin Staticsymtable .stabs "LONGINT:t5=3Dr5;-2147483648;2147483647;",128,0,0,0 .stabs "CHAR:t6=3D-20;",128,0,0,0 .stabs "BYTE:t7=3Dr7;0;255;",128,0,0,0 .stabs "SHORTSTRING:Tt8=3Ds256length:7,0,8;st:ar7;1;255;6,8,2040;;"= ,128,0,0,0 .stabs ":t9=3D*8",128,0,0,0 .stabs ":t10=3Dar5;0;0;4",128,0,0,0 .stabs "__vtbl_ptr_type:Tt11=3Ds2;",128,0,0,0 .stabs "pvmt:t12=3D*11",128,0,0,0 .stabs "vtblarray:t13=3Dar5;0;1;12",128,0,0,0 .stabs ":Tt1=3Ds4$vf1:13,0;CREATE::14=3D##3;:__ct__4TOBJ7POINTER;2A= .;;~%1;",128,0,0,0 .stabs "vmt_PROGRAMTOBJ:S11",38,0,0,VMT_P$PROGRAM_TOBJ # Defs - End Staticsymtable # Syms - Begin Staticsymtable .stabs "TOBJ:Tt1",128,0,3,0 .stabs "T:S1",40,0,13,U_P$PROGRAM_T # Syms - End Staticsymtable # End asmlist al_stabs # Begin asmlist al_procedures It seems that the trouble is really coming from=20 .stabs ":Tt1=3Ds4$vf1:13,0;CREATE::14=3D##3;:__ct__4TOBJ7POINTER;2A= .;;~%1;",128,0,0,0 especially the first "implicit parameter of the constructor called create, because TOBJ type is not yet known (and thus is a stub). The named type TOVJ is defined later in: .stabs "TOBJ:Tt1",128,0,3,0 In a g++ equivalent source, the parameters of the functions are given via d= bx types so that no stub type is generated, and thus also no internal error. I do believe that the stabs generated by Free Pascal is correct and should still be accepted. Anyhow, your patch does solve the issue and should be committed. Pierre Muller