This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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




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.

Sorry, I just don't see how it's possible to have an array type where part of the type is objfile-owned and part of it arch-owned. When an array type is copied, the space allocation depends on whether or not the element type is defined in the program. If it is defined, space for the index type, range type, and array type itself are all allocated from that object file. Otherwise, they are all allocated from the 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!

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.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]