[DTrace-devel] [PATCH 4/8] Add support for strrchr() subroutine
Kris Van Hees
kris.van.hees at oracle.com
Thu Oct 14 21:06:37 PDT 2021
On Wed, Sep 29, 2021 at 11:13:37AM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> The algorithm is different from the one used for strchr() due to
> how the BPF verifier handles decrementing and incrementing loops
> differently.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
... and added to my staging dev branch. I did add the .type and .size
markers to the strchr.S file.
> ---
> bpf/Build | 1 +
> bpf/strrchr.S | 109 +++++++++++++++++++++++++++++++
> libdtrace/dt_cg.c | 53 ++++++++++++++-
> libdtrace/dt_dlibs.c | 1 +
> test/unittest/funcs/tst.strchr.d | 1 -
> 5 files changed, 163 insertions(+), 2 deletions(-)
> create mode 100644 bpf/strrchr.S
>
> diff --git a/bpf/Build b/bpf/Build
> index 1aa9dd62..6e9ffa20 100644
> --- a/bpf/Build
> +++ b/bpf/Build
> @@ -29,6 +29,7 @@ bpf_dlib_SOURCES = \
> strchr.S \
> strcmp.S \
> strjoin.S \
> + strrchr.S \
> substr.S \
> strlen.c
>
> diff --git a/bpf/strrchr.S b/bpf/strrchr.S
> new file mode 100644
> index 00000000..72463f26
> --- /dev/null
> +++ b/bpf/strrchr.S
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
> + */
> +
> +#define DT_STRLEN_BYTES 2
> +
> +#define BPF_FUNC_probe_read 4
> +#define BPF_FUNC_probe_read_str 45
> +
> +/*
> + * uint64_t dt_strrchr(char *src, uint64_t c, char *dst) {
> + *
> + * c <<= 56;
> + *
> + * [%fp-8]=src
> + * [%fp-16]=c
> + * [%fp-24]=dst
> + *
> + * r6 = bpf_probe_read_str(dst, STRSZ, src + DT_STRLEN_BYTES);
> + * r6--;
> + *
> + * r8 = r6;
> + * Lloop:
> + * r8--;
> + * r3 = src[DT_STRLEN_BYTES + r8];
> + * r3 <<= 56;
> + * if (r3 == c) goto Lfound;
> + * if (r8 > 0) goto Lloop;
> + *
> + * Lnone:
> + * return -1;
> + *
> + * Lfound:
> + * r6 -= r8;
> + * dt_strlen_store(r6, dst);
> + * r8 += DT_STRLEN_BYTES;
> + * bpf_probe_read(dst + DT_STRLEN_BYTES, r6, src + r8);
> + *
> + * r6 += DT_STRLEN_BYTES;
> + * dst[r6] = '\0';
> + *
> + * return 0;
> + * }
> + */
> + .text
> + .align 4
> + .global dt_strrchr
> +dt_strrchr :
> + lsh %r2, 56 /* c <<= 56 */
> +
> + stxdw [%fp+-8], %r1 /* Spill src */
> + stxdw [%fp+-16], %r2 /* Spill c */
> + stxdw [%fp+-24], %r3 /* Spill dst */
> +
> + ldxdw %r1, [%fp+-24]
> + lddw %r2, STRSZ
> + ldxdw %r3, [%fp+-8]
> + add %r3, DT_STRLEN_BYTES
> + call BPF_FUNC_probe_read_str /* r6 = bpf_probe_read_str(dst, STRSZ, src + DT_STRLEN_BYTES) */
> + jsle %r0, 0, .Lnone
> + mov %r6, %r0
> +
> + sub %r6, 1 /* r6-- */
> +
> + mov %r8, %r6 /* r8 = r6 */
> +
> + ldxdw %r5, [%fp+-16]
> + jsle %r8, 0, .Lnone
> +.Lloop:
> + sub %r8, 1 /* r8-- */
> + mov %r4, %r8
> + add %r4, DT_STRLEN_BYTES
> + ldxdw %r3, [%fp+-8]
> + add %r3, %r4
> + ldxb %r3, [%r3+0] /* r3 = src[DT_STRLEN_BYTES+ r8] */
> + lsh %r3, 56 /* r3 <<= 56 */
> + jeq %r3, %r5, .Lfound /* if (r3 == c) goto Lfound */
> + jgt %r8, 0, .Lloop /* if (r8 > 0) goto Lloop */
> +
> +.Lnone:
> + mov %r0, -1 /* return -1 */
> + exit
> +
> +.Lfound:
> + sub %r6, %r8 /* r6 -= r8 */
> + jsle %r6, 0, .Lnone
> +
> + mov %r1, %r6
> + ldxdw %r2, [%fp+-24]
> + call dt_strlen_store /* dt_strlen_store(r6, dst) */
> +
> + add %r8, DT_STRLEN_BYTES /* r8 += DT_STRLEN_BYTES */
> +
> + ldxdw %r1, [%fp+-24]
> + add %r1, DT_STRLEN_BYTES
> + mov %r2, %r6
> + ldxdw %r3, [%fp+-8]
> + add %r3, %r8
> + call BPF_FUNC_probe_read /* bpf_probe_read(dst + DT_STRLEN_BYTES, r6, src + r8) */
> +
> + add %r6, DT_STRLEN_BYTES /* r6 += DT_STRLEN_BYTES */
> + ldxdw %r1, [%fp+-24]
> + add %r1, %r6 /* dst[r6] = '\0' */
> + mov %r2, 0
> + stxb [%r1+0], %r2
> +
> + mov %r0, 0 /* return 0 */
> + exit
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 7d7dcaa6..cb58f230 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -3237,6 +3237,57 @@ dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> TRACE_REGSET(" subr-strchr:End ");
> }
>
> +static void
> +dt_cg_subr_strrchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> + dt_ident_t *idp;
> + dt_node_t *str = dnp->dn_args;
> + dt_node_t *chr = str->dn_list;
> + uint_t Lfound = dt_irlist_label(dlp);
> +
> + TRACE_REGSET(" subr-strrchr:Begin");
> + dt_cg_node(str, dlp, drp);
> + dt_cg_check_notnull(dlp, drp, str->dn_reg);
> + dt_cg_node(chr, dlp, drp);
> +
> + if (dt_regset_xalloc_args(drp) == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> +
> + emit(dlp, BPF_MOV_REG(BPF_REG_1, str->dn_reg));
> + dt_regset_free(drp, str->dn_reg);
> + if (str->dn_tstring)
> + dt_cg_tstring_free(yypcb, str);
> + emit(dlp, BPF_MOV_REG(BPF_REG_2, chr->dn_reg));
> + dt_regset_free(drp, chr->dn_reg);
> +
> + /*
> + * The result needs be be a temporary string, so we request one.
> + */
> + dnp->dn_reg = dt_regset_alloc(drp);
> + if (dnp->dn_reg == -1)
> + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> + dt_cg_tstring_alloc(yypcb, dnp);
> +
> + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, BPF_REG_FP, DT_STK_DCTX));
> + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, dnp->dn_reg, DCTX_MEM));
> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, dnp->dn_tstring->dn_value));
> + emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
> +
> + dt_regset_xalloc(drp, BPF_REG_0);
> + idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strrchr");
> + assert(idp != NULL);
> + emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
> + dt_regset_free_args(drp);
> +
> + emit (dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, Lfound));
> + emit (dlp, BPF_MOV_IMM(dnp->dn_reg, 0));
> + emitl(dlp, Lfound,
> + BPF_NOP());
> + dt_regset_free(drp, BPF_REG_0);
> +
> + TRACE_REGSET(" subr-strrchr:End ");
> +}
> +
> static void
> dt_cg_subr_strlen(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> {
> @@ -3408,7 +3459,7 @@ static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
> [DIF_SUBR_DIRNAME] = NULL,
> [DIF_SUBR_CLEANPATH] = NULL,
> [DIF_SUBR_STRCHR] = &dt_cg_subr_strchr,
> - [DIF_SUBR_STRRCHR] = NULL,
> + [DIF_SUBR_STRRCHR] = &dt_cg_subr_strrchr,
> [DIF_SUBR_STRSTR] = NULL,
> [DIF_SUBR_STRTOK] = NULL,
> [DIF_SUBR_SUBSTR] = &dt_cg_subr_substr,
> diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> index c4e495e0..d9836f40 100644
> --- a/libdtrace/dt_dlibs.c
> +++ b/libdtrace/dt_dlibs.c
> @@ -62,6 +62,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> DT_BPF_SYMBOL(dt_strchr, DT_IDENT_SYMBOL),
> DT_BPF_SYMBOL(dt_strcmp, DT_IDENT_SYMBOL),
> DT_BPF_SYMBOL(dt_strjoin, DT_IDENT_SYMBOL),
> + DT_BPF_SYMBOL(dt_strrchr, DT_IDENT_SYMBOL),
> DT_BPF_SYMBOL(dt_substr, DT_IDENT_SYMBOL),
> /* BPF maps */
> DT_BPF_SYMBOL(aggs, DT_IDENT_PTR),
> diff --git a/test/unittest/funcs/tst.strchr.d b/test/unittest/funcs/tst.strchr.d
> index 9d8262df..a562b72e 100644
> --- a/test/unittest/funcs/tst.strchr.d
> +++ b/test/unittest/funcs/tst.strchr.d
> @@ -4,7 +4,6 @@
> * Licensed under the Universal Permissive License v 1.0 as shown at
> * http://oss.oracle.com/licenses/upl.
> */
> -/* @@xfail: dtv2 */
>
> #pragma D option quiet
>
> --
> 2.18.4
>
>
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list