This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/2] Fast tracepoint for powerpc64le
- From: Pedro Alves <palves at redhat dot com>
- To: Ulrich Weigand <uweigand at de dot ibm dot com>, Wei-cheng Wang <cole945 at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 04 Mar 2015 17:13:37 +0000
- Subject: Re: [PATCH 1/2] Fast tracepoint for powerpc64le
- Authentication-results: sourceware.org; auth=none
- References: <201502271952 dot t1RJqq7X018591 at d03av02 dot boulder dot ibm dot com>
On 02/27/2015 07:52 PM, Ulrich Weigand wrote:
>> The main reason why PowerPC64 big-endian doesn't work is
>> calling convention (function descriptors) issue.
>> When installing a tracepoint in inferior memory, gdbserver
>> asks the address of "gdb_collect" (and etc.) using qSymbol packet,
>> and it generate a sequence of instructions to calling that address.
>> However, gdb-client "return the start of code instead of
>> any data function descriptor."
>> See commenting in remote_check_symbols/remote.c,
>> https://sourceware.org/ml/gdb-patches/2007-06/msg00389.html
>> and gen_call() in this patch.
>> In order for powerpc64be to work, qSymbol packet should be
>> extend for function descriptors.
> So I guess there's two ways to fix this. One would be to change
> gdbserver to work more like GDB here. This would involve removing
> the descriptor->code address conversion in remote.c, and instead
> performing the conversion in gdbserver's thread_db_enable_reporting.
> Now, there is no gdbarch_convert_from_func_ptr_addr in gdbserver,
> so a similar mechanism would have to be invented there. (I guess
> this would mean a new target hook.) Fortunately, the only platform
> that uses function descriptors *and* supports libthread_db debugging
> in gdbserver is ppc64-linux, so we'd only have to add that new
> mechanim on this platform.
Note sure about this one, ppc64_convert_from_func_ptr_addr wants to
get at the bfd/binary's unrelocated sections. We'd have to teach
gdbserver to read the binary.
>
> This has the advantage that qSymbol could now be used to lookup
> function symbols and get the descriptor address as expected.
> On the other hand, this would mean an incompatible change in the
> remote protocol: if you used a new GDB together with an old
> gdbserver (or vice versa), thread debugging would stop working.
> However, I guess that could be fixed by having gdbserver request
> the new behavior from GDB by specifying a feature code. With old
> GDBs gdbserver would have to skip the descriptor->code conversion.
>
>
> The second alternative would be to extend qSymbol to support
> returning two different types of addresses for function symbols:
> the symbol value (i.e. function pointer value, i.e. descriptor
> on PPC64), and a code address suitable to set a breakpoint on
> function entry. This could be either by having gdbserver
> request one or the other via an additional flag on the qSymbol
> request, or else by GDB simply always returning both values
> in two fields. Again, this would be an incompatible protocol
> change that would need to be guarded by a qFeature check.
>
> In this case, gdbserver would use the "normal" symbol values
> for most purposes (e.g. tracepoint routine lookup), but would
> use the "code address" values to return from ps_pglobal_lookup.
> With old GDBs, gdbserver would disable fast tracepoint support
> on powerpc64.
>
> If we do that, then for consistency it might also be useful
> to pass code addresses to ps_pglobal_lookup in GDB itself.
> In addition, the "code address" could be changed to skip
> the local entry point prolog on powerpc64le to ensure that
> the breakpoint is set correctly. (This does not matter in
> practice since __nptl_create_event has no local entry point,
> but it would seem more fully correct in general.)
>
>
> Overall, the second alternative seems a bit better to me,
> but I'd certainly appreciate feedback on this ...
I inclined for the second alternative as well.
(Note for testing: __nptl_create_event will only be used
on old kernels without PTRACE_EVENT_CLONE, unless you hack the
code to force usage.)
>
>
>> For powerpc32 to work, some data structure/function in tracepoint.c
>> need to be fixed. For example,
>>
>> * write_inferior_data_ptr should be fixed for big-endian.
>> If sizeof (CORE_ADDR) is larger than sizeof (void*), zeros are written.
>> BTW, I thnink write_inferior_data_pointer provides the same functionality
>> without this issue. I'm not sure why write_inferior_data_ptr is needed?
>
> This is odd, I don't see the point of this either.
Yeah, probably I just missed merging them fully while cleaning up the
initial contribution... I agree we should just use
write_inferior_data_pointer.
>> * Data structure layout between gdbserver and IPA is not consistent.
>>
>> There are two versions of tracepoint_action one for gdbserver,
>> and antoher for inferior (IPA side).
>>
...
> Ugh. That's a strange construct, and extremely dependent on alignment
> rules (as you noticed). I'm not really sure what the best way to fix
> this would be. My preference right now would be get rid of "ops" on
> the gdbserver side too, and just switch on "type" in the two places
> where the ops->send and ops->download routines are called right now.
>
> This makes the data structures the same on gdbserver and IPA, which
> simplifies downloading quite a bit. (Also, it keeps the data structure
> identical in IPA, which should avoid compatibility issues between
> versions.)
That sounds fine.
Thanks,
Pedro Alves