[DTrace-devel] [PATCH 3/9] Dedicate space for call stacks
Kris Van Hees
kris.van.hees at oracle.com
Thu Dec 21 06:13:03 UTC 2023
On Thu, Oct 05, 2023 at 05:14:01PM -0400, eugene.loh at oracle.com wrote:
>
> In the dctx->mem memory block, storage for a call stack and storage
> for tstrings overlapped. The rationale was that a stack() action and
> temporary strings would never co-exist.
>
> However, the call stack storage wasn't being used for stack() (or ustack
> or jstack) actions at all. It was being used for stackdepth (and
> ustackdepth and jstackdepth) built-in variables. And they could
> co-exist with temporary strings.
>
> So make the call stack storage its own area within dctx->mem.
>
> Add a test to check for the overlap bug.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
... with the small name changes listed below
> ---
> bpf/get_bvar.c | 3 ++-
> libdtrace/dt_bpf.h | 1 +
> libdtrace/dt_cc.c | 3 +++
> libdtrace/dt_dctx.h | 14 +++++++------
> libdtrace/dt_dlibs.c | 1 +
> .../codegen/tst.tstring_overlap_bug.d | 21 +++++++++++++++++++
> .../codegen/tst.tstring_overlap_bug.r | 4 ++++
> 7 files changed, 40 insertions(+), 7 deletions(-)
> create mode 100644 test/unittest/codegen/tst.tstring_overlap_bug.d
> create mode 100644 test/unittest/codegen/tst.tstring_overlap_bug.r
>
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index de3fdbc6..ee8f756f 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -27,6 +27,7 @@ extern uint64_t PC;
> extern uint64_t STBSZ;
> extern uint64_t STKSIZ;
> extern uint64_t BOOTTM;
> +extern uint64_t MEM_STACK;
For consistency, I think STACK_OFF might be a better name.
> #define error(dctx, fault, illval) \
> ({ \
> @@ -64,7 +65,7 @@ noinline uint64_t dt_get_bvar(const dt_dctx_t *dctx, uint32_t id, uint32_t idx)
> case DIF_VAR_USTACKDEPTH: {
> uint32_t bufsiz = (uint32_t) (uint64_t) (&STKSIZ);
> uint64_t flags = 0 & BPF_F_SKIP_FIELD_MASK;
> - char *buf = dctx->mem;
> + char *buf = dctx->mem + (uint64_t)(&MEM_STACK);
STACK_OFF
> uint64_t stacksize;
>
> if (id == DIF_VAR_USTACKDEPTH)
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index 0fee533b..0b883936 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -40,6 +40,7 @@ extern "C" {
> #define DT_CONST_RODATA_OFF 19
> #define DT_CONST_RODATA_SIZE 20
> #define DT_CONST_ZERO_OFF 21
> +#define DT_CONST_MEM_STACK 22
DT_CONST_STACK_OFF
> #define DT_BPF_LOG_SIZE_DEFAULT (UINT32_MAX >> 8)
> #define DT_BPF_LOG_SIZE_SMALL 4096
> diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> index a42109f1..b35b8821 100644
> --- a/libdtrace/dt_cc.c
> +++ b/libdtrace/dt_cc.c
> @@ -1173,6 +1173,9 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> case DT_CONST_ZERO_OFF:
> nrp->dofr_data = dtp->dt_zerooffset;
> continue;
> + case DT_CONST_MEM_STACK:
DT_CONST_STACK_OFF
> + nrp->dofr_data = DMEM_STACK(dtp);
> + continue;
> default:
> /* probe name -> value is probe id */
> if (strchr(idp->di_name, ':') != NULL)
> diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
> index 319e4233..1422ad24 100644
> --- a/libdtrace/dt_dctx.h
> +++ b/libdtrace/dt_dctx.h
> @@ -97,11 +97,11 @@ typedef struct dt_dctx {
> * The dctx->mem pointer references a block of memory that contains:
> *
> * +----------------+----------------+
> - * 0 -> | Stack : tstring | \
> - * | trace (shared) storage | |
> - * | storage : | |
> - * +----------------+----------------+ > DMEM_SIZE
> + * 0 -> | tstring storage | \
> + * +----------------+----------------+ |
> * DMEM_STRTOK -> | strtok() internal state | |
> + * +---------------------------------+ > DMEM_SIZE
> + * DMEM_STACK -> | stack storage | |
> * +---------------------------------+ |
> * DMEM_TUPLE -> | tuple assembly area | /
> * +---------------------------------+
> @@ -127,9 +127,11 @@ typedef struct dt_dctx {
> * Macros to determine the offset of the components of dctx->mem.
> */
> #define DMEM_STRTOK(dtp) \
> - MAX(DMEM_STACK_SZ(dtp), DMEM_TSTR_SZ(dtp))
> -#define DMEM_TUPLE(dtp) \
> + DMEM_TSTR_SZ(dtp)
> +#define DMEM_STACK(dtp) \
> (DMEM_STRTOK(dtp) + DMEM_STRTOK_SZ(dtp))
> +#define DMEM_TUPLE(dtp) \
> + (DMEM_STACK(dtp) + DMEM_STACK_SZ(dtp))
>
> /*
> * Macro to determine the total size of the mem area.
> diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
> index 02a24ad2..bdd02f4a 100644
> --- a/libdtrace/dt_dlibs.c
> +++ b/libdtrace/dt_dlibs.c
> @@ -94,6 +94,7 @@ static const dt_ident_t dt_bpf_symbols[] = {
> DT_BPF_SYMBOL_ID(RODATA_OFF, DT_IDENT_SCALAR, DT_CONST_RODATA_OFF),
> DT_BPF_SYMBOL_ID(RODATA_SIZE, DT_IDENT_SCALAR, DT_CONST_RODATA_SIZE),
> DT_BPF_SYMBOL_ID(ZERO_OFF, DT_IDENT_SCALAR, DT_CONST_ZERO_OFF),
> + DT_BPF_SYMBOL_ID(MEM_STACK, DT_IDENT_SCALAR, DT_CONST_MEM_STACK),
STACK_OFF
> /* End-of-list marker */
> { NULL, }
> diff --git a/test/unittest/codegen/tst.tstring_overlap_bug.d b/test/unittest/codegen/tst.tstring_overlap_bug.d
> new file mode 100644
> index 00000000..df2f87db
> --- /dev/null
> +++ b/test/unittest/codegen/tst.tstring_overlap_bug.d
> @@ -0,0 +1,21 @@
> +/*
> + * Oracle Linux DTrace.
> + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
> + * Licensed under the Universal Permissive License v 1.0 as shown at
> + * http://oss.oracle.com/licenses/upl.
> + */
> +
> +/*
> + * Check for a bug in which tstrings and stackdepth scratch overlapped.
> + */
> +
> +#pragma D option quiet
> +
> +BEGIN
> +{
> + printf("stack depth is %d\n", stackdepth);
> + i = stackdepth;
> + printf("substring is |%s|\n", substr(strjoin("abcdefghijkl", "mnopqrstuvwxyz"), i));
> + printf("substring is |%s|\n", substr(strjoin("abcdefghijkl", "mnopqrstuvwxyz"), stackdepth));
> + exit(0);
> +}
> diff --git a/test/unittest/codegen/tst.tstring_overlap_bug.r b/test/unittest/codegen/tst.tstring_overlap_bug.r
> new file mode 100644
> index 00000000..dc92dbbc
> --- /dev/null
> +++ b/test/unittest/codegen/tst.tstring_overlap_bug.r
> @@ -0,0 +1,4 @@
> +stack depth is 0
> +substring is |abcdefghijklmnopqrstuvwxyz|
> +substring is |abcdefghijklmnopqrstuvwxyz|
> +
> --
> 2.18.4
>
>
More information about the DTrace-devel
mailing list