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] gdb.trace: Remove struct tracepoint_action_ops.


On 25/01/16 12:53, Pedro Alves wrote:
On 01/23/2016 07:31 PM, Marcin KoÅcielnicki wrote:
The struct tracepoint_action has an ops field, pointing to
a tracepoint_action_ops structure, containing send and download ops.
However, this field is only present when compiled in gdbserver, and not
when compiled in IPA.  When gdbserver is downloading tracepoint actions
to IPA, it skips offsetof(struct tracepoint_action, type) bytes from
its struct tracepoint_action, to get to the part that corresponds to
IPA's struct tracepoint_action.

Unfortunately, this fails badly on ILP32 platforms where alignof(long long)
== 8.  Consider struct collect_memory_action layout in gdbserver:

0-3: base.ops
4: base.type
8-15: addr
16-23: len
24-27: basereg
sizeof == 32

and its layout in IPA:

0: base.type
8-15: addr
16-23: len
24-27: basereg
sizeof == 32

When gdbserver tries to download it to IPA, it skips the first 4 bytes
(base.ops), figuring the rest will match what IPA expects - which is
not true, since addr is aligned to 8 bytes and will be at a different
relative position to base.type.

The problem went unnoticed on the currently supported platforms, since
aarch64 and x86_64 have ops aligned to 8 bytes, and i386 has only 4-byte
alignment for long long.

There are a few possible ways around this problem.  I decided on removing
ops altogether, since they can be easily inlined in their (only) places
of use - in fact allowing us share the code between 'L' and 'R'.  Any
approach where struct tracepoint_action is different between IPA and
gdbserver is just asking for trouble.

Found on s390.  Tested on x86_64, s390, s390x.

Hmm, this is essentially the same as:

  https://sourceware.org/ml/gdb-patches/2015-03/msg00995.html

Right?

Seems that other patch inlines things a bit less though, which offhand
looks preferable.  WDYT?

Not sure what happened to that series.  I thought most of it (if not all)
had been approved already.

Thanks,
Pedro Alves


Huh, I didn't know about that patch series. Good to know, since I was going to try doing ppc tracepoints next, and had no idea that has already been attempted. What happened to that patchset/author? Kind of strange to abandon mostly-finished code when there's a $3k bounty on it.

The other patch looks fine too, I have no preference here.

Marcin KoÅcielnicki


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