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

[Joel Brobecker <brobecker@adacore.com>]

> > 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.

> > 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!

> 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.

-- 
Joel