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

Eugene Loh
2025-09-02T18:05:16Z

So far as I can tell:

*) https://oss.oracle.com/pipermail/dtrace-devel/2025-August/006562.html
Nick commented on the patch

*) https://oss.oracle.com/pipermail/dtrace-devel/2025-August/006573.html
Alan responded to Nick

Since this patch is related to observed test failures, can we complete
this patch review?  E.g., perhaps Nick can respond to Alan's last
message, maybe even agreeing with Alan's proposal simply to add a
comment that we don't need bitshifts yet.  If so, Alan can post a v2 of
the patch and then Nick or I could add our "Reviewed-by"?

On 8/26/25 04:12, Alan Maguire wrote:
> 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.
>
> This in turn causes tcp state-change probes (and in-progress UDP
> probes) to not fire since we verify that sk_protocol == IPPROTO_TCP.
>
> 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 issue is not seen on more recent kernels because sk_protocol
> becomes a __u16 as the number of protocols exceeds 256.
>
> Reported-by: Eugene Loh <eugene.loh@oracle.com>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
> ---
> libdtrace/dt_cg.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cd9e7f4e..7abb5bc6 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1959,6 +1959,8 @@ dt_cg_ctf_offsetof(const char *structname, const char *membername,
> dtrace_typeinfo_t sym;
> ctf_file_t *ctfp;
> ctf_membinfo_t ctm;
> + ctf_encoding_t cte;
> + int offset;
>
> if (dtrace_lookup_by_type(yypcb->pcb_hdl, DTRACE_OBJ_EVERY, structname,
> &sym))
> @@ -1973,6 +1975,13 @@ dt_cg_ctf_offsetof(const char *structname, const char *membername,
>
> longjmp(yypcb->pcb_jmpbuf, EDT_NOCTF);
> }
> + offset = ctm.ctm_offset;
> +
> + /* a bitfield may have an additional > 8 bit offset which means we need
> + * to adjust the reported byte offset.
> + */
> + if (ctf_type_encoding(ctfp, ctm.ctm_type, &cte) != CTF_ERR)
> + offset += cte.cte_offset;
>
> if (sizep || ldopp) {
> uint_t ldop;
> @@ -1982,7 +1991,7 @@ dt_cg_ctf_offsetof(const char *structname, const char *membername,
> *ldopp = ldop;
> }
>
> - return (ctm.ctm_offset / NBBY);
> + return (offset / NBBY);
> }
> static void
> dt_cg_act_breakpoint(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind)