* [PATCH 1/4] cg, providers: refactor clearing probe arguments
@ 2025-12-17 5:09 Kris Van Hees
2025-12-18 19:12 ` [DTrace-devel] " Eugene Loh
0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2025-12-17 5:09 UTC (permalink / raw)
To: dtrace, dtrace-devel
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
libdtrace/dt_cg.c | 34 +++++++++++++++++++++++++++++++---
libdtrace/dt_cg.h | 1 +
libdtrace/dt_prov_cpc.c | 17 -----------------
libdtrace/dt_prov_dtrace.c | 8 ++------
libdtrace/dt_prov_profile.c | 23 +----------------------
libdtrace/dt_prov_syscall.c | 10 ++--------
6 files changed, 37 insertions(+), 56 deletions(-)
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index ec4e3123..b3084ddb 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -454,6 +454,32 @@ dt_cg_tramp_copy_regs(dt_pcb_t *pcb)
}
}
+/*
+ * Clear the content of the 'argv' member of the machine state, from the given
+ * index (idx).
+ *
+ * The caller must ensure that %r7 contains the value set by the
+ * dt_cg_tramp_prologue*() functions.
+ */
+void
+dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int idx)
+{
+ dt_irlist_t *dlp = &pcb->pcb_ir;
+ int i, argc = ARRAY_SIZE(((dt_mstate_t *)0)->argv);
+
+ if (idx >= argc)
+ return;
+
+ /*
+ * memset(&dctx->mst->argv[idx], 0, sizeof(dt_pt_regs);
+ * // stdw [%7 + DMST_ARG(idx)], 0
+ * // stdw [%7 + DMST_ARG(idx + 1)], 0
+ * // (...)
+ */
+ for (i = idx; i < argc; i++)
+ emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
+}
+
/*
* Copy arguments from a dt_pt_regs structure referenced by %r8.
* If 'called' is nonzero, the registers are laid out as when inside the
@@ -626,6 +652,10 @@ dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb)
/* done */
emitl(dlp, Ldone,
BPF_NOP());
+
+ /* clear the rest of the arguments */
+ dt_cg_tramp_clear_argv(pcb, 2);
+
dt_regset_free_args(drp);
}
@@ -640,15 +670,13 @@ void
dt_cg_tramp_copy_rval_from_regs(dt_pcb_t *pcb)
{
dt_irlist_t *dlp = &pcb->pcb_ir;
- int i;
emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(0), 0));
emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, PT_REGS_RET));
emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(1), BPF_REG_0));
- for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
+ dt_cg_tramp_clear_argv(pcb, 2);
}
/*
diff --git a/libdtrace/dt_cg.h b/libdtrace/dt_cg.h
index 9493edbb..401ac0e0 100644
--- a/libdtrace/dt_cg.h
+++ b/libdtrace/dt_cg.h
@@ -26,6 +26,7 @@ extern void dt_cg_tramp_prologue_cpu(dt_pcb_t *pcb);
extern void dt_cg_tramp_prologue(dt_pcb_t *pcb);
extern void dt_cg_tramp_clear_regs(dt_pcb_t *pcb);
extern void dt_cg_tramp_copy_regs(dt_pcb_t *pcb);
+extern void dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int i);
extern void dt_cg_tramp_copy_args_from_regs(dt_pcb_t *pcb, int called);
extern void dt_cg_tramp_copy_pc_from_regs(dt_pcb_t *pcb);
extern void dt_cg_tramp_copy_rval_from_regs(dt_pcb_t *pcb);
diff --git a/libdtrace/dt_prov_cpc.c b/libdtrace/dt_prov_cpc.c
index 57b11b13..fd33990f 100644
--- a/libdtrace/dt_prov_cpc.c
+++ b/libdtrace/dt_prov_cpc.c
@@ -393,26 +393,9 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
*/
static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
{
- int i;
- dt_irlist_t *dlp = &pcb->pcb_ir;
-
dt_cg_tramp_prologue_cpu(pcb);
-
- /*
- * After the dt_cg_tramp_prologue_cpu() call, we have:
- * // (%r7 = dctx->mst)
- * // (%r8 = dctx->ctx)
- */
-
dt_cg_tramp_copy_regs(pcb);
-
- /*
- * Use the PC to set arg0 and arg1, then clear the other args.
- */
dt_cg_tramp_copy_pc_from_regs(pcb);
- for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
-
dt_cg_tramp_epilogue(pcb);
return 0;
diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
index 34b5d8e2..453d3e58 100644
--- a/libdtrace/dt_prov_dtrace.c
+++ b/libdtrace/dt_prov_dtrace.c
@@ -83,7 +83,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
{
dt_irlist_t *dlp = &pcb->pcb_ir;
dt_activity_t act = DT_ACTIVITY_ACTIVE;
- uint32_t i, key = 0;
+ uint32_t key = 0;
/*
* The ERROR probe isn't really a trace event that a BPF program is
@@ -174,11 +174,7 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_update_elem));
dt_cg_tramp_copy_regs(pcb);
-
- /* zero the probe args */
- for (i = 0; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
-
+ dt_cg_tramp_clear_argv(pcb, 0);
dt_cg_tramp_epilogue_advance(pcb, act);
return 0;
diff --git a/libdtrace/dt_prov_profile.c b/libdtrace/dt_prov_profile.c
index e1369ca9..cb65ca43 100644
--- a/libdtrace/dt_prov_profile.c
+++ b/libdtrace/dt_prov_profile.c
@@ -216,24 +216,9 @@ static int provide(dtrace_hdl_t *dtp, const dtrace_probedesc_t *pdp)
*/
static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
{
- int i;
- dt_irlist_t *dlp = &pcb->pcb_ir;
-
dt_cg_tramp_prologue_cpu(pcb);
-
- /*
- * After the dt_cg_tramp_prologue_cpu() call, we have:
- * // (%r7 = dctx->mst)
- * // (%r8 = dctx->ctx)
- */
-
dt_cg_tramp_copy_regs(pcb);
-
- /*
- * dctx->mst->argv[0] = kernel PC
- * dctx->mst->argv[1] = userspace PC
- */
- dt_cg_tramp_copy_pc_from_regs(pcb);
+ dt_cg_tramp_copy_pc_from_regs(pcb);
/*
* TODO: For profile-n probes:
@@ -246,12 +231,6 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
* and update expected+=interval.
*/
- /*
- * (we clear dctx->mst->argv[2] and on)
- */
- for (i = 2; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
-
dt_cg_tramp_epilogue(pcb);
return 0;
diff --git a/libdtrace/dt_prov_syscall.c b/libdtrace/dt_prov_syscall.c
index 20843c6f..237f2728 100644
--- a/libdtrace/dt_prov_syscall.c
+++ b/libdtrace/dt_prov_syscall.c
@@ -159,14 +159,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
}
- /*
- * Zero the remaining probe args.
- * for ( ; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- * dctx->mst->argv[i] = 0;
- * // stdw [%r7 + DMST_ARG(i)], 0
- */
- for ( ; i < ARRAY_SIZE(((dt_mstate_t *)0)->argv); i++)
- emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
+ /* Zero the remaining probe args. */
+ dt_cg_tramp_clear_argv(pcb, i);
/*
* For return probes, store the errno. That is, examine arg0.
--
2.51.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [DTrace-devel] [PATCH 1/4] cg, providers: refactor clearing probe arguments
2025-12-17 5:09 [PATCH 1/4] cg, providers: refactor clearing probe arguments Kris Van Hees
@ 2025-12-18 19:12 ` Eugene Loh
2025-12-18 19:23 ` Kris Van Hees
0 siblings, 1 reply; 3+ messages in thread
From: Eugene Loh @ 2025-12-18 19:12 UTC (permalink / raw)
To: Kris Van Hees, dtrace, dtrace-devel
Two things...
On 12/17/25 00:09, Kris Van Hees via DTrace-devel wrote:
> Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> ---
> libdtrace/dt_cg.c | 34 +++++++++++++++++++++++++++++++---
> libdtrace/dt_cg.h | 1 +
> libdtrace/dt_prov_cpc.c | 17 -----------------
> libdtrace/dt_prov_dtrace.c | 8 ++------
> libdtrace/dt_prov_profile.c | 23 +----------------------
> libdtrace/dt_prov_syscall.c | 10 ++--------
Is there also another clean-up site in dt_prov_sdt.c in trampoline()?
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> @@ -454,6 +454,32 @@ dt_cg_tramp_copy_regs(dt_pcb_t *pcb)
> }
> }
>
> +/*
> + * Clear the content of the 'argv' member of the machine state, from the given
> + * index (idx).
> + *
> + * The caller must ensure that %r7 contains the value set by the
> + * dt_cg_tramp_prologue*() functions.
> + */
> +void
> +dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int idx)
> +{
> + dt_irlist_t *dlp = &pcb->pcb_ir;
> + int i, argc = ARRAY_SIZE(((dt_mstate_t *)0)->argv);
> +
> + if (idx >= argc)
> + return;
> +
> + /*
> + * memset(&dctx->mst->argv[idx], 0, sizeof(dt_pt_regs);
> + * // stdw [%7 + DMST_ARG(idx)], 0
> + * // stdw [%7 + DMST_ARG(idx + 1)], 0
> + * // (...)
> + */
Actually, the code is clearer and more concise than the comment. Plus,
the comment is wrong? What's sizeof(dt_pt_regs) doing in there? Just
drop the "memset" comment block. The two lines of code speak for
themselves.
> + for (i = idx; i < argc; i++)
> + emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [DTrace-devel] [PATCH 1/4] cg, providers: refactor clearing probe arguments
2025-12-18 19:12 ` [DTrace-devel] " Eugene Loh
@ 2025-12-18 19:23 ` Kris Van Hees
0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2025-12-18 19:23 UTC (permalink / raw)
To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel
On Thu, Dec 18, 2025 at 02:12:01PM -0500, Eugene Loh wrote:
> Two things...
>
> On 12/17/25 00:09, Kris Van Hees via DTrace-devel wrote:
>
> > Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
> > ---
> > libdtrace/dt_cg.c | 34 +++++++++++++++++++++++++++++++---
> > libdtrace/dt_cg.h | 1 +
> > libdtrace/dt_prov_cpc.c | 17 -----------------
> > libdtrace/dt_prov_dtrace.c | 8 ++------
> > libdtrace/dt_prov_profile.c | 23 +----------------------
> > libdtrace/dt_prov_syscall.c | 10 ++--------
>
> Is there also another clean-up site in dt_prov_sdt.c in trampoline()?
Ah right, good catch.
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > @@ -454,6 +454,32 @@ dt_cg_tramp_copy_regs(dt_pcb_t *pcb)
> > }
> > }
> > +/*
> > + * Clear the content of the 'argv' member of the machine state, from the given
> > + * index (idx).
> > + *
> > + * The caller must ensure that %r7 contains the value set by the
> > + * dt_cg_tramp_prologue*() functions.
> > + */
> > +void
> > +dt_cg_tramp_clear_argv(dt_pcb_t *pcb, int idx)
> > +{
> > + dt_irlist_t *dlp = &pcb->pcb_ir;
> > + int i, argc = ARRAY_SIZE(((dt_mstate_t *)0)->argv);
> > +
> > + if (idx >= argc)
> > + return;
> > +
> > + /*
> > + * memset(&dctx->mst->argv[idx], 0, sizeof(dt_pt_regs);
> > + * // stdw [%7 + DMST_ARG(idx)], 0
> > + * // stdw [%7 + DMST_ARG(idx + 1)], 0
> > + * // (...)
> > + */
>
> Actually, the code is clearer and more concise than the comment. Plus, the
> comment is wrong? What's sizeof(dt_pt_regs) doing in there? Just drop the
> "memset" comment block. The two lines of code speak for themselves.
Good point. Removing.
v2 on the way.
> > + for (i = idx; i < argc; i++)
> > + emit(dlp, BPF_STORE_IMM(BPF_DW, BPF_REG_7, DMST_ARG(i), 0));
> > +}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-18 19:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-17 5:09 [PATCH 1/4] cg, providers: refactor clearing probe arguments Kris Van Hees
2025-12-18 19:12 ` [DTrace-devel] " Eugene Loh
2025-12-18 19:23 ` Kris Van Hees
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox