This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE
- From: Doug Evans <xdje42 at gmail dot com>
- To: "Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: <gdb-patches at sourceware dot org>
- Date: Sat, 07 Feb 2015 15:05:12 -0800
- Subject: Re: [PATCH 4/5] Remove struct main_type.vptr_{fieldno,basetype}: TYPE_SPECIFIC_SELF_TYPE
- Authentication-results: sourceware.org; auth=none
- References: <m3twze8kxc dot fsf at sspiff dot org> <54d3aa6b dot a23d460a dot 0d37 dot 0908SMTPIN_ADDED_BROKEN at mx dot google dot com>
"Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr> 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ÃÂ: lundi 26 janvier 2015 01:07
>> ÃÂ: 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) = bar;
>>
>> One instead has to do:
>>
>> set_type_self_type (foo, bar);
>>
>> But I've left reading of the type to the macro:
>>
>> bar = 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) == TYPE_CODE_FUNC)
>> + TYPE_CODE (new_sublist->fn_field.type) = TYPE_CODE_METHOD;
>> + else
>> + gdb_assert (TYPE_CODE (new_sublist->fn_field.type)
>> + == 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) = type;
>> + set_type_self_type (new_sublist->fn_field.type, type);
>> new_sublist->fn_field.is_stub = 1;
>> }
>> +
>> new_sublist->fn_field.physname = savestring (*pp, p - *pp);
>> *pp = 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) != 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) ==
> TYPE_SPECIFIC_NONE)
> set_type_self_type (new_sublist->fn_field.type, type);
> new_sublist->fn_field.is_stub = 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 <xdje42@gmail.com>
* 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) == TYPE_SPECIFIC_NONE)
+ return NULL;
gdb_assert (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_SELF_TYPE);
return TYPE_MAIN_TYPE (type)->type_specific.self_type;
case TYPE_CODE_METHOD:
+ if (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_NONE)
+ return NULL;
gdb_assert (TYPE_SPECIFIC_FIELD (type) == TYPE_SPECIFIC_FUNC);
return TYPE_MAIN_TYPE (type)->type_specific.func_stuff->self_type;
default: