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: "Ulrich Weigand" <uweigand at de dot ibm dot com>
- To: cole945 at gmail dot com (Wei-cheng Wang)
- Cc: palves at redhat dot com, gdb-patches at sourceware dot org
- Date: Tue, 17 Mar 2015 14:47:47 +0100 (CET)
- Subject: Re: [PATCH 1/2] Fast tracepoint for powerpc64le
- Authentication-results: sourceware.org; auth=none
Wei-cheng Wang wrote:
> On 2015/2/28 ä¸?å?? 03:52, Ulrich Weigand wrote:
> > The tspeed.exp file already has:
> > # Typically we need a little extra time for this test.
> > set timeout 180
> > Is that still not enough?
>
> It should include the time spent in trying different loop counts,
> so it would be 11 + 22 + 45 + 90 + 180 = at least 348 seconds in my environment.
> (for 10000, 20000, 40000, 80000, 160000 iterations respectively)
> If I set timeout to 360, the case will pass.
I guess that's OK with me. Or else we could reduce the number of passes ...
> >> * tfind.exp: One of the tracepoint is inserted at
> >> `*gdb_recursion_test'. It's not hit because local-entry is called
> >> instead. The 18 FAILs are off-by-one error.
> > This test case seem a bit more complicated, we may need to split it
> > in two parts; one that uses a normal "trace gdb_recursion_test"
> > without the "*", and possibly a second one that specifically tests
> > that "trace *func" works, using a source file that makes sure to
> > call func via a function pointers (as in step-bt.c).
>
> How about simply change the code to this? It wouldn't hurt other cases.
> And all the failed cases in tfind.exp now pass.
That should be OK.
> > This is odd, I don't see the point of this either. Of course, as the
> > comment says, much of this stuff will break anyway if gdbserver is
> > compiled differently than the inferior (e.g. a 64-bit gdbserver
> > debugging a 32-bit inferior), because it assumes the structure layout
> > is identical. However, if we do have a 32-bit gdbserver, then I don't
> > see why it shouldn't be possible to debug a 32-bit inferior, just
> > because CORE_ADDR is a 64-bit type ...
>
> For example, CORE_ADDR ptr = 0x11223344, a 32-bit address,
> and sizeof (void *) = 4-byte
>
> |------------ 64-bit CORE_ADDR ---------|
> MSB LSB
> | 00 | 00 | 00 | 00 | 11 | 22 | 33 | 44 |
> Low High Address
> |-- 32-bit(void*) --|
> &ptr,4 means these zeros are written to inferior.
>
> static int
> write_inferior_data_ptr (CORE_ADDR where, CORE_ADDR ptr)
> {
> return write_inferior_memory (where,
> (unsigned char *) &ptr, sizeof (void *));
> ^^^^ ^^^^^^^^^^^^^^^
> }
>
> CORE_ADDR is declared as "unsigned long long" for gdbserver
> (in common/gdb/common-types.h)
I understood why this is failing with the code as is, I just didn't
understand why the code is that way today :-) Given Pedro's comment,
I think we should simply remove that function.
> > 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 great to me!
OK, let's do it that way.
Bye,
Ulrich
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com