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/30/2018 11:45 PM, Joel Brobecker wrote:
> > Is there any reason why the gdbarch structure, which won't be freed
> > until the corresponding architecture is, needs to have a lifetime that
> > matches the objfiles?
> 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.

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

> > > I happen to have hit the same issue as you, but from an Ada expression,
> > > and sent it a fix not long ago:
> > > https://www.sourceware.org/ml/gdb-patches/2018-01/msg00240.html
> > >
> > > Does it fix your problem too?
> > Yes, it does fix my problem of gdb asserting on the "set {char[]}$pc="hi""
> > command, as
> > reported in the PR,
> Good!
>
> > but still asserts on a slightly modified "set {unsigned char[]}$pc="hi"
> > command.
> It's should be something fairly similar. Can you track down which type
> it is which is arch-owned, and where it comes from? I have a feeling
> that there is a simple fix similar to mine to be made that would
> fix that.

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.

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.

> I can help taking a look, but I'm a little tied up this week...

I'd really appreciate it if you can take a look at your convenience. It 
doesn't
have to be this week. Thanks.