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

[Eugene Loh <eugene.loh@oracle.com>]

Reviewed-by: Eugene Loh <eugene.loh@oracle.com>

On 9/4/25 05:08, 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.
>
> Signed-off-by: Alan Maguire <alan.maguire@oracle.com>
>
> Changes since v1:
>
> - added comment reflecting need to handle more complex bitfield values
>    in future (Nick)
> ---
>   libdtrace/dt_cg.c | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index cd9e7f4e..e378cf96 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,17 @@ 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 bit offset which means we need
> +	 * to adjust the reported byte offset.  Currently there is no need
> +	 * to bitshift values as they always both fall on 8-bit boundaries
> +	 * and are sizes which are multiples of 8 bits, but in future if
> +	 * more complex bitfield retrieval is required, changes such as those
> +	 * in dt_cg_field_get() will be needed to support that.
> +	 */
> +	if (ctf_type_encoding(ctfp, ctm.ctm_type, &cte) != CTF_ERR)
> +		offset += cte.cte_offset;
>   
>   	if (sizep || ldopp) {
>   		uint_t	ldop;
> @@ -1982,7 +1995,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)