[DTrace-devel] [PATCH v2] Fix stack layout definitions to match the new dt_dctx design

Kris Van Hees kris.van.hees at oracle.com
Mon Sep 14 12:50:11 PDT 2020


The stack layout was changed with the move of the DTrace machine state
out of the stack.  The changes were not properly reflected everywhere,
both in comments and in code:

- dt_cg.c: dt_cg_tramp_prologue()

    The comment describing the BPF code to pass the 2nd argument to the
    bpf_map_lookup_elem() helper was referring to the wrong stack offset
    constant.

    The pseudo-code in the comment also incorrectly passed 'mem' rather
    than '&mem'.

- dt_dctx.h:

    The definition of DCTX_SIZE now explicitly casts to int16_t to
    avoid compilation warnings in code that uses it in BPF instructions.

    The definition of DCTX_FP(off) is now based on DT_STK_DCTX as the
    base offset of the dt_dctx struct (defined in dt_impl.h).

- dt_impl.h:

    The comment block describing the stack layout now reflects the
    correct layout in view of dt_dctx.

    The definitions of DT_STK_BASE and DT_STK_SLOT_SZ now explicitly
    cast to int16_t to avoid compilation warnings in code that uses
    them in BPF instructions.

    The definition of DT_STK_DCTX has been corrected based on its
    actual size (DCTX_SIZE).

- dt_prov_*.c:

    The comment blocks describing the population of the probe args now
    reflects that the args ae stored in dt_mstate_t.

- tst.stack_layout.r:

    Updated the expected output.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_cg.c                        | 14 +++++++-----
 libdtrace/dt_dctx.h                      |  4 ++--
 libdtrace/dt_impl.h                      | 20 ++++++++---------
 libdtrace/dt_prov_dtrace.c               |  8 +++----
 libdtrace/dt_prov_fbt.c                  |  8 +++----
 libdtrace/dt_prov_profile.c              |  2 +-
 libdtrace/dt_prov_sdt.c                  |  2 +-
 libdtrace/dt_prov_syscall.c              |  6 ++---
 test/unittest/codegen/tst.stack_layout.r | 28 ++++++++++++------------
 9 files changed, 47 insertions(+), 45 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 0d3463c1..2dcf8023 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -64,10 +64,10 @@ dt_cg_tramp_prologue(dt_pcb_t *pcb, uint_t lbl_exit)
 
 	/*
 	 *	key = 0;                // stw [%fp + DCTX_FP(DCTX_MST)], 0
-	 *	rc = bpf_map_lookup_elem(mem, &key);
+	 *	rc = bpf_map_lookup_elem(&mem, &key);
 	 *				// lddw %r1, &mem
 	 *				// mov %r2, %fp
-	 *				// add %r2, DT_STK_LVA_BASE
+	 *				// add %r2, DCTX_FP(DCTX_MST)
 	 *				// call bpf_map_lookup_elem
 	 *				//     (%r1 ... %r5 clobbered)
 	 *				//     (%r0 = 'mem' BPF map value)
@@ -116,6 +116,11 @@ dt_cg_call_clause(dtrace_hdl_t *dtp, dt_ident_t *idp, dt_irlist_t *dlp)
 {
 	struct bpf_insn	instr;
 
+	/*
+	 *	dt_clause(dctx);	// mov %r1, %fp
+	 *				// add %r1, DCTX_FP(0)
+	 *				// call dt_program
+	 */
 	instr = BPF_MOV_REG(BPF_REG_1, BPF_REG_FP);
 	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
 	instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DCTX_FP(0));
@@ -134,10 +139,7 @@ dt_cg_tramp_epilogue(dt_pcb_t *pcb, uint_t lbl_exit)
 	struct bpf_insn	instr;
 
 	/*
-	 *	dt_program(dctx);	// mov %r1, %fp
-	 *				// add %fp, DCTX_FP(0)
-	 *				// call dt_program
-	 *	(repeated for each clause)
+	 *	dt_clause(dctx);	//     (repeated for each clause)
 	 */
 	dt_probe_clause_iter(pcb->pcb_hdl, pcb->pcb_probe,
 			     (dt_clause_f *)dt_cg_call_clause, dlp);
diff --git a/libdtrace/dt_dctx.h b/libdtrace/dt_dctx.h
index 79ef7105..b315f522 100644
--- a/libdtrace/dt_dctx.h
+++ b/libdtrace/dt_dctx.h
@@ -36,13 +36,13 @@ typedef struct dt_dctx {
 #define DCTX_CTX	offsetof(dt_dctx_t, ctx)
 #define DCTX_MST	offsetof(dt_dctx_t, mst)
 #define DCTX_BUF	offsetof(dt_dctx_t, buf)
-#define DCTX_SIZE	sizeof(dt_dctx_t)
+#define DCTX_SIZE	(int16_t)sizeof(dt_dctx_t)
 
 /*
  * Macro to determine the (negative) offset from the frame pointer (%fp) for
  * the given offset in dt_dctx_t.
  */
-#define DCTX_FP(off)	(-(ushort_t)DCTX_SIZE + (ushort_t)(off))
+#define DCTX_FP(off)	(DT_STK_DCTX + (int16_t)(off))
 
 #define DMST_EPID	offsetof(dt_mstate_t, epid)
 #define DMST_TAG	offsetof(dt_mstate_t, tag)
diff --git a/libdtrace/dt_impl.h b/libdtrace/dt_impl.h
index fb06032d..0f79b45f 100644
--- a/libdtrace/dt_impl.h
+++ b/libdtrace/dt_impl.h
@@ -494,29 +494,29 @@ struct dtrace_hdl {
  *                             +----------------+
  *         SCRATCH_BASE = -512 | Scratch Memory |
  *                             +----------------+
- *   LVAR_END = LVAR(n) = -256 | LVAR n         | (n = DT_VAR_LOCAL_MAX = 20)
+ *   LVAR_END = LVAR(n) = -256 | LVAR n         | (n = DT_VAR_LOCAL_MAX = 19)
  *                             +----------------+
  *                             |      ...       |
  *                             +----------------+
- *               LVAR(1) = -96 | LVAR 1         |
+ *              LVAR(1) = -112 | LVAR 1         |
  *                             +----------------+
- *   LVAR_BASE = LVAR(0) = -88 | LVAR 0         |
+ *  LVAR_BASE = LVAR(0) = -104 | LVAR 0         |
  *                             +----------------+
- *              SPILL(n) = -80 | %r8            | (n = DT_STK_NREGS - 1 = 8)
+ *              SPILL(n) = -96 | %r8            | (n = DT_STK_NREGS - 1 = 8)
  *                             +----------------+
  *                             |      ...       |
  *                             +----------------+
- *              SPILL(1) = -24 | %r1            |
+ *              SPILL(1) = -40 | %r1            |
  *                             +----------------+
- * SPILL_BASE = SPILL(0) = -16 | %r0            |
+ * SPILL_BASE = SPILL(0) = -32 | %r0            |
  *                             +----------------+
- *                   DCTX = -8 | DTrace Context | -1
+ *                  DCTX = -24 | DTrace Context | -1
  *                             +----------------+
  */
-#define DT_STK_BASE		(0)
-#define DT_STK_SLOT_SZ		((int)sizeof(uint64_t))
+#define DT_STK_BASE		((int16_t)0)
+#define DT_STK_SLOT_SZ		((int16_t)sizeof(uint64_t))
 
-#define DT_STK_DCTX		(DT_STK_BASE - DT_STK_SLOT_SZ)
+#define DT_STK_DCTX		(DT_STK_BASE - DCTX_SIZE)
 #define DT_STK_SPILL_BASE	(DT_STK_DCTX - DT_STK_SLOT_SZ)
 #define DT_STK_SPILL(n)		(DT_STK_SPILL_BASE - (n) * DT_STK_SLOT_SZ)
 #define DT_STK_LVAR_BASE	(DT_STK_SPILL(DT_STK_NREGS - 1) - \
diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
index a0d0e38a..76e60d87 100644
--- a/libdtrace/dt_prov_dtrace.c
+++ b/libdtrace/dt_prov_dtrace.c
@@ -103,16 +103,16 @@ static void trampoline(dt_pcb_t *pcb)
 	/*
 	 *	dctx->mst->argv[0] = PT_REGS_PARAM1((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG0]
-	 *				// stdw [%r7 + DMST_ARG(0)0], %r0
+	 *				// stdw [%r7 + DMST_ARG(0)], %r0
 	 *	dctx->mst->argv[1] = PT_REGS_PARAM2((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG1]
-	 *				// stdw [%r7 + DMST_ARG(1)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(1)], %r0
 	 *	dctx->mst->argv[2] = PT_REGS_PARAM3((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG2]
-	 *				// stdw [%r7 + DMST_ARG(2)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(2)], %r0
 	 *	dctx->mst->argv[3] = PT_REGS_PARAM4((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG3]
-	 *				// stdw [%r7 + DMST_ARG(3)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(3)], %r0
 	 *	dctx->mst->argv[4] = PT_REGS_PARAM5((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG4]
 	 *				// stdw [%r7 + DMST_ARG(4)], %r0
diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index c9845fe6..fb48cc75 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -195,16 +195,16 @@ static void trampoline(dt_pcb_t *pcb)
 	/*
 	 *	dctx->mst->argv[0] = PT_REGS_PARAM1((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG0]
-	 *				// stdw [%r7 + DMST_ARG(0)0], %r0
+	 *				// stdw [%r7 + DMST_ARG(0)], %r0
 	 *	dctx->mst->argv[1] = PT_REGS_PARAM2((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG1]
-	 *				// stdw [%r7 + DMST_ARG(1)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(1)], %r0
 	 *	dctx->mst->argv[2] = PT_REGS_PARAM3((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG2]
-	 *				// stdw [%r7 + DMST_ARG(2)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(2)], %r0
 	 *	dctx->mst->argv[3] = PT_REGS_PARAM4((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG3]
-	 *				// stdw [%r7 + DMST_ARG(3)8], %r0
+	 *				// stdw [%r7 + DMST_ARG(3)], %r0
 	 *	dctx->mst->argv[4] = PT_REGS_PARAM5((dt_pt_regs *)dctx->ctx);
 	 *				// lddw %r0, [%r8 + PT_REGS_ARG4]
 	 *				// stdw [%r7 + DMST_ARG(4)], %r0
diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
index 6e066694..320095d1 100644
--- a/libdtrace/dt_prov_profile.c
+++ b/libdtrace/dt_prov_profile.c
@@ -262,7 +262,7 @@ static void trampoline(dt_pcb_t *pcb)
 	 * For now, we can only provide the first argument:
 	 *     dctx->mst->argv[0] = PT_REGS_IP((dt_pt_regs *)&dctx->ctx->regs);
 	 *                              //  lddw %r0, [%r8 + PT_REGS_IP]
-	 *                              //  stdw [%r7 + DMST_ARG(0)0], %r0
+	 *                              //  stdw [%r7 + DMST_ARG(0)], %r0
 	 */
 	instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_IP);
 	dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
diff --git a/libdtrace/dt_prov_sdt.c b/libdtrace/dt_prov_sdt.c
index 3c2edcc1..aa60fdc0 100644
--- a/libdtrace/dt_prov_sdt.c
+++ b/libdtrace/dt_prov_sdt.c
@@ -428,7 +428,7 @@ static void trampoline(dt_pcb_t *pcb)
 	/*
 	 *	for (i = 0; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
 	 *		dctx->mst->argv[i] = 0
-	 *				// stdw [%r7 + DCTX_ARG(0), 0
+	 *				// stdw [%r7 + DMST_ARG(i)], 0
 	 */
 	for (i = 0; i < pcb->pcb_pinfo.dtp_argc; i++) {
 		instr = BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0);
diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
index 754422f0..e25bb151 100644
--- a/libdtrace/dt_prov_syscall.c
+++ b/libdtrace/dt_prov_syscall.c
@@ -182,8 +182,8 @@ static void trampoline(dt_pcb_t *pcb)
 	 *	for (i = 0; i < argc; i++)
 	 *		dctx->mst->argv[i] =
 	 *			((struct syscall_data *)dctx->ctx)->arg[i];
-	 *				// lddw %r0, [%r8 + SCD_ARG(0)]
-	 *				// stdw [%r7 + DCTX_ARG(0), %r0
+	 *				// lddw %r0, [%r8 + SCD_ARG(i)]
+	 *				// stdw [%r7 + DMST_ARG(i)], %r0
 	 */
 	for (i = 0; i < pcb->pcb_probe->argc; i++) {
 		instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, SCD_ARG(i));
@@ -195,7 +195,7 @@ static void trampoline(dt_pcb_t *pcb)
 	/*
 	 *	for i = argc; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
 	 *		dctx->mst->argv[i] = 0;
-	 *				// stdw [%r7 + DCTX_ARG(i), %r0
+	 *				// stdw [%r7 + DMST_ARG(i)], 0
 	 */
 	for ( ; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++) {
 		instr = BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0);
diff --git a/test/unittest/codegen/tst.stack_layout.r b/test/unittest/codegen/tst.stack_layout.r
index d0081e53..2155e785 100644
--- a/test/unittest/codegen/tst.stack_layout.r
+++ b/test/unittest/codegen/tst.stack_layout.r
@@ -1,17 +1,17 @@
 Base:          0
-dctx:         -8
-%r0:         -16
-%r1:         -24
-%r2:         -32
-%r3:         -40
-%r4:         -48
-%r5:         -56
-%r6:         -64
-%r7:         -72
-%r8:         -80
-lvar[ -1]:   -87 (ID  -1)
-lvar[  0]:   -88 (ID   0)
-lvar[  1]:   -96 (ID   1)
-lvar[ 21]:  -256 (ID  21)
+dctx:        -24
+%r0:         -32
+%r1:         -40
+%r2:         -48
+%r3:         -56
+%r4:         -64
+%r5:         -72
+%r6:         -80
+%r7:         -88
+%r8:         -96
+lvar[ -1]:  -103 (ID  -1)
+lvar[  0]:  -104 (ID   0)
+lvar[  1]:  -112 (ID   1)
+lvar[ 19]:  -256 (ID  19)
 lvar[ -1]:  -257 (ID  -1)
 scratch:    -257 .. -512
-- 
2.28.0




More information about the DTrace-devel mailing list