[DTrace-devel] [PATCH] Built-in variable "timestamp": semantics should agree with v1
Kris Van Hees
kris.van.hees at oracle.com
Tue Jul 21 14:20:21 PDT 2020
This is really two patches:
1. ensuring that the 'timestamp' value is cached within the scope of a clause
2. moving the evaluation of the predicate until the machine state has been
set up
I broke it up like that, added a testcase for each, and integrated the
patches.
So, effectively a reviewed-by, with modifications during the integration.
On Sat, Jul 18, 2020 at 11:12:42AM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> The semantics of the built-in variable "timestamp" are not 100% clear from
> the documentation. Specifically, should it record a fresh time stamp each
> time it is called? Or should it be measured once upon probe (or clause)
> entry and then reused?
>
> Regardless, "legacy" DTrace seems to record a fresh timestamp for each
> clause, reusing the same value throughout the clause. Consider:
>
> # dtrace -qn 'BEGIN {trace(timestamp);trace(timestamp);}
> BEGIN {trace(timestamp);trace(timestamp);exit(0)};'
> 3823604430297094
> 3823604430297094
> 3823604430297975
> 3823604430297975
>
> In contrast, DTv2 appears to use a fresh timestamp each instance,
> even within the same clause:
>
> # dtrace -qn 'BEGIN {trace(timestamp);trace(timestamp); exit(0);}'
> 1645711727876826
> 1645711727876873
>
> The timestamp value should be consistent throughout an entire clause
> and its predicate. E.g., with DTv1:
>
> # dtrace -qn 'BEGIN
> /(t = timestamp) == timestamp/
> { trace(t);
> trace(timestamp);
> trace(timestamp); exit(0);
> }'
>
> trace() reports the same value three times, while with DTv2 the
> predicate never even evaluates to true at all and the test hangs.
>
> Fix the DTv2 version to match the legacy behavior, including in
> dt_cg_prologue() moving mstate initialization before predicate evaluation.
> Also, free the register used for any predicate, fixing a register leak.
>
> Orabug: 31632630, 31638431
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> bpf/get_bvar.c | 4 +++-
> libdtrace/dt_cg.c | 32 ++++++++++++++++++--------------
> libdtrace/dt_dctx.h | 4 +++-
> 3 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index 40045fb0..1d0f3e46 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -21,7 +21,9 @@ noinline uint64_t dt_get_bvar(dt_mstate_t *mst, uint32_t id)
> case DIF_VAR_CURTHREAD:
> return bpf_get_current_task();
> case DIF_VAR_TIMESTAMP:
> - return bpf_ktime_get_ns();
> + if (mst->tstamp == 0)
> + mst->tstamp = bpf_ktime_get_ns();
> + return mst->tstamp;
> case DIF_VAR_EPID:
> return mst->epid;
> case DIF_VAR_ARG0: case DIF_VAR_ARG1: case DIF_VAR_ARG2:
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 2435b84d..2746ccd7 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -181,20 +181,6 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)
> instr = BPF_STORE(BPF_DW, BPF_REG_FP, DT_STK_DCTX, BPF_REG_1);
> dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>
> - /*
> - * If there is a predicate:
> - *
> - * rc = predicate; // (evaluate predicate into %rX)
> - * if (rc == 0) // jeq %rX, 0, pcb->pcb_exitlbl
> - * goto exit;
> - */
> - if (pred != NULL) {
> - dt_cg_node(pred, &pcb->pcb_ir, pcb->pcb_regs);
> - instr = BPF_BRANCH_IMM(BPF_JEQ, pred->dn_reg, 0,
> - pcb->pcb_exitlbl);
> - dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> - }
> -
> /*
> * buf = dctx->buf; // lddw %r0, [%fp + DT_STK_DCTX]
> * // lddw %r9, [%r0 + DCTX_BUF]
> @@ -207,6 +193,7 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)
> /*
> * dctx->mst->fault = 0; // lddw %r0, [%r0 + DCTX_MST]
> * // stdw [%r0 + DMST_FAULT], 0
> + * dctx->mst->tstamp = 0; // stdw [%r0 + DMST_TSTAMP], 0
> * dctx->mst->epid = EPID; // stw [%r0 + DMST_EPID], EPID
> * *((uint32_t *)&buf[0]) = EPID;
> * // stw [%r9 + 0], EPID
> @@ -215,6 +202,8 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)
> dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> instr = BPF_STORE_IMM(BPF_DW, BPF_REG_0, DMST_FAULT, 0);
> dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> + instr = BPF_STORE_IMM(BPF_DW, BPF_REG_0, DMST_TSTAMP, 0);
> + dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> instr = BPF_STORE_IMM(BPF_W, BPF_REG_0, DMST_EPID, -1);
> dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> dlp->dl_last->di_extern = epid;
> @@ -232,6 +221,21 @@ dt_cg_prologue(dt_pcb_t *pcb, dt_node_t *pred)
> instr = BPF_STORE_IMM(BPF_W, BPF_REG_9, 4, 0);
> dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>
> + /*
> + * If there is a predicate:
> + *
> + * rc = predicate; // (evaluate predicate into %rX)
> + * if (rc == 0) // jeq %rX, 0, pcb->pcb_exitlbl
> + * goto exit;
> + */
> + if (pred != NULL) {
> + dt_cg_node(pred, &pcb->pcb_ir, pcb->pcb_regs);
> + instr = BPF_BRANCH_IMM(BPF_JEQ, pred->dn_reg, 0,
> + pcb->pcb_exitlbl);
> + dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> + dt_regset_free(pcb->pcb_regs, pred->dn_reg);
> + }
> +
> /*
> * Account for 32-bit epid (at offset 0) and 32-bit tag (at offset 4).
> */
> diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
> index 647d181a..df4ff6fa 100644
> --- a/libdtrace/dt_dctx.h
> +++ b/libdtrace/dt_dctx.h
> @@ -11,12 +11,13 @@
> #include <dt_pt_regs.h>
>
> /*
> - * The DTrace machien state.
> + * The DTrace machine state.
> */
> typedef struct dt_mstate {
> uint32_t epid; /* Enabled probe ID */
> uint32_t tag; /* Tag (for future use) */
> uint64_t fault; /* DTrace fault flags */
> + uint64_t tstamp; /* timestamp built-in variable */
> #if 0
> dt_pt_regs regs; /* CPU registers */
> #endif
> @@ -46,6 +47,7 @@ typedef struct dt_dctx {
> #define DMST_EPID offsetof(dt_mstate_t, epid)
> #define DMST_TAG offsetof(dt_mstate_t, tag)
> #define DMST_FAULT offsetof(dt_mstate_t, fault)
> +#define DMST_TSTAMP offsetof(dt_mstate_t, tstamp)
> #define DMST_REGS offsetof(dt_mstate_t, regs)
> #define DMST_ARG(n) offsetof(dt_mstate_t, argv[n])
>
> --
> 2.18.2
>
>
> _______________________________________________
> 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