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: Cell multi-arch symbol lookup broken (Re: [PATCH] solib_global_lookup: Fetch arch from objfile.)


"Ulrich Weigand" <uweigand@de.ibm.com> writes:
> Doug Evans wrote:
>
>> solib_global_lookup should be using the objfile's arch,
>> not fetching it from global state.
>> 
>> Regression tested on amd64-linux.
>> 
>> 2014-10-31  Doug Evans  <xdje42@gmail.com>
>> 
>> 	* objfiles.c (get_objfile_arch): Constify.
>> 	* objfiles.h (get_objfile_arch): Update prototype.
>> 	* solib.c (solib_global_lookup): Fetch arch from objfile,
>> 	not target_gdbarch.
>
> This also causes a regression in Cell multi-arch debugging.
>
> The problem is that solib_ops are really only installed for
> the main target_gdbarch.  As opposed to many other things,
> there is just a single solib_ops vector, intended to describe
> the dynamic loader in use on the inferior.  While it can make
> sense to have multiple loaders when debugging different
> inferiors, within a single inferior we support only one.
>
> The solib_global_lookup routine was intended to describe the
> operation of that (single) dynamic loader, so that GDB's symbol
> lookup can achieve the same result as the native dynamic loader
> would.  That's why we added that routine to the solib_ops.
>
> On Cell, the dynamic loader runs on the PowerPC side (which
> is was target_gdbarch () returns), and so we installed the
> solib_ops there.  The Cell implementation of solib_global_lookup
> then handles both PowerPC and SPU objects.
>
> However, after your patch, this implementation now never gets
> called for SPU objects any more, which means the special
> lookup rules are no longer implemented.
>
> Simply reverting this patch makes everything work for me again.
> Was there any specific reason why you made this change in the
> first place?  Does this fix any specific problem?

Interesting.
I wasn't aware of this treatment of gdbarch.
Sounds like another API cleanup is in order, it's pretty subtle.

So I get now that there is an inferior gdbarch that is potentially
distinct from whatever gdbarch one has at hand (e.g., an objfile's),
[that's the easy part]
and that certain API calls *have* to use the inferior gdbarch.
[that's the subtle part]

One question that comes to mind is why don't the other gdbarches
in the system, e.g., the spu's, delegate these calls to its "parent" gdbarch?
Any design that requires global state like this is suboptimal,
and I'd like to see gdb move away from it wherever possible.
[Here we have a gdbarch from the objfile, but we can't use it.]

Another thought that comes to mind is, if we don't want "child gdbarches"
to delegate to "parent gdbarches" (and I haven't decided myself), then
another way to go is to use different types.
E.g., inferior_gdbarch and gdbarch.
[I might name inferior_gdbarch differently, depending on what API calls
it contains, but this would be another way to avoid potential confusion
over what the correct gdbarch to use is in any particular situation.]

I'm ok with reverting this particular part of the patch,
though it'd be helpful to add a comment explaining
why things are the way they are.


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