Re: [PATCH] cg: fix offset for > 8 bit bitfields in dt_cg_ctf_offsetof()

Nick Alcock
2025-08-26T17:40:41Z

On 26 Aug 2025, Alan Maguire verbalised:

> The tcp provider uses dt_cg_tramp_get_member() to retrieve the
> offset of the sk_protocol field in struct sock. However it
> returns the wrong value on UEK6 since it is an 8-bit bitfield.
> From pahole we see:
>
> unsigned int __sk_flags_offset[0]; /* 560 0 */
> unsigned int sk_padding:1; /* 560: 0 4 */
> unsigned int sk_kern_sock:1; /* 560: 1 4 */
> unsigned int sk_no_check_tx:1; /* 560: 2 4 */
> unsigned int sk_no_check_rx:1; /* 560: 3 4 */
> unsigned int sk_userlocks:4; /* 560: 4 4 */
> unsigned int sk_protocol:8; /* 560: 8 4 */
>
> In other words it is really at offset 561 but because we just
> lookup the member offset and not the member type offset we get the
> wrong value for the sk_protoocol.

Excellent description! It's annoying as hell that BTF still has this
redundant representation :( DTrace literally always got this wrong,
right back to the year dot.

> The fix is to look up the member _type_ offset and add it to the
> bit offset we get for the member itself. With this in place the
> state-change probes fire, but the local tcp tests still fail due
> to separate issues with the tcp:::accept-established probe.

This is certainly necessary (and your code looks good), but we also need
to ensure that the load is suitably shifted and masked, particularly for
bitfields that cross machine words. DTrace never used to do this
either... e.g. the size returned by dt_cg_ldsize() assumes that the
entire bitfield can be read in one load op, which depending on its
offset may not be true.

It looks like dt_cg_field_get might get this *mostly* right (once you
delete the #if 0'ed stuff -- but I'd be suspicious until I'd tested it
on a big-endian platform), but even that assumes it only needs to do one
load. As soon as a bitfield crosses machine words, I suspect we'll only
get the first half of it :( we might need to do a second load to fetch
the second half of the bitfield, then a shift and | to combine the two,
and DTrace has never had any code for that.