Re: [PATCH PR gdb/20057] Internal error on trying to set {char[]}$pc="string"

[Wei-min Pan <weimin dot pan at oracle dot com>]

On 1/31/2018 11:59 PM, Joel Brobecker wrote:
> > > Unfortunately, I only have vague answers for you. I know it's not
> > > as satisfactory as a firm one, but I haven't had time to investigate
> > > further.
> > >
> > > My feeling is that it's (intuitively) a bad idea to start mixing
> > > and matching the ownership type for a give type chain. It just
> > > muddies the waters, and makes memory management more complex.
> > Given there are functions such as arch_integer_type(),
> > arch_character_type(),
> > and arch_float_type() that can be used to add types to an arch, it doesn't
> > seem terribly wrong to add a type which is not associated with any objfile
> > to gdbarch? Also a type can actually exist in both an arch and an objfile.
> I am not sure we understand each other. For me, what seems wrong
> is the fact that we have an array type where part of the type is
> objfile-owned, and part of it arch-owned.
>
> Creating arch-owned type is fine, as long as the entire type is
> arch-owned.

I see, thanks for the clarification. But it doesn't seem to be the case
when we have a declaration like "unsigned char p[] = "abc";". The array
type and its element type will split between an objfile and an arch.

> > > Parallel to that, there is another obstacle if you want to enhance
> > > copy_type to handle arch-owned types, as the current implementation
> > > explicitly assumes that the type is objfile-owned, and therefore
> > > references its objfile's obstack:
> > >
> > >     if (TYPE_DYN_PROP_LIST (type) != NULL)
> > >       TYPE_DYN_PROP_LIST (new_type)
> > >         = copy_dynamic_prop_list (&TYPE_OBJFILE (type) -> objfile_obstack,
> > >                                   TYPE_DYN_PROP_LIST (type));
> > Good point. The following statement
> >
> >    if (TYPE_DYN_PROP_LIST (type) != NULL)
> >
> > needs to be changed to:
> >
> >    if (TYPE_DYN_PROP_LIST (type) != NULL && TYPE_OBJFILE_OWNED(type))
> That would be wrong, because the resulting type would be missing
> that dynamic property list, which means the resulting type would
> be a complete copy of the original type. It's not so simple!

What needs to be done then is to pick the correct obstack, from either
objfile_obstack or gdbarch->obstack, when copying the dynamic property list?

> > Your fix in lookup_array_range_type() takes care the case where
> > "element_type" was owned by an objfile but still creates an arch-owned
> > index type if it was not.
> That is correct, and it is not a problem as long as the entire type
> is consistent.
>
> > Here is the test case that comes with the PR:
> >
> > % cat x.c
> > char p[] = "hello";
> >
> > int main()
> > {
> >    return ((int)(p[0]));
> > }
> >
> > Please note that the test case declares base type "char" which has an
> > associated objfile and is picked up by lookup_symbol_aux() when
> > command "set {char[]}$pc="hi" is parsed and eventually is passed as
> > the element type argument to lookup_array_range_type(). Using any
> > other type, such as "unsigned char", in that gdb command results in
> > the element type that is picked up from gdbarch and has no associated
> > objfile.
> That is exactly the problem. At the point where it decides to use
> an arch-owned type, it should check the type it is for, and whether
> it is arch or objfile owned, and then create the type from there.
> If my intuition is right, my patch should be a good example of what
> needs to be done.

Since the type to be copied needs to be objfile-owned in copy_type(), 
will it still trigger
the assertion if the complete type was created and owned by an arch?

Thanks.