* Re: [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack @ 2025-03-19 14:47 Kris Van Hees 0 siblings, 0 replies; 3+ messages in thread From: Kris Van Hees @ 2025-03-19 14:47 UTC (permalink / raw) To: dtrace On Wed, Feb 19, 2025 at 11:43:49PM -0500, eugene.loh@oracle.com wrote: > > While one can access the BPF stack relative to %r9, the whole > point of DT_TRAMP_SP_SLOT(0) is to make trampoline code more > readable. So use it. > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com> > --- > libdtrace/dt_prov_uprobe.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c > index 5d9f74244..f1323cc31 100644 > --- a/libdtrace/dt_prov_uprobe.c > +++ b/libdtrace/dt_prov_uprobe.c > @@ -1015,22 +1015,15 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl) > emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32)); > > /* > - * Look up in the BPF 'usdt_prids' map. Space for the look-up key > - * will be used on the BPF stack: > - * > - * offset value > - * > - * -sizeof(usdt_prids_map_key_t) pid (in %r0) > - * > - * -sizeof(usdt_prids_map_key_t) + sizeof(pid_t) > - * == > - * -sizeof(dtrace_id_t) underlying-probe prid > + * Look up in the BPF 'usdt_prids' map. The key should fit into > + * trampoline stack slot 0. > */ > - emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0)); > - emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id)); > + assert(sizeof(usdt_prids_map_key_t) <= DT_STK_SLOT_SZ); > + emit(dlp, BPF_STORE(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), BPF_REG_0)); > + emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); > dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id); > - emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_9)); > - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t)))); > + emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP)); > + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0))); > emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem)); > emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit)); > > -- > 2.43.5 > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack
@ 2025-02-20 4:43 eugene.loh
[not found] ` <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com>
0 siblings, 1 reply; 3+ messages in thread
From: eugene.loh @ 2025-02-20 4:43 UTC (permalink / raw)
To: dtrace, dtrace-devel
From: Eugene Loh <eugene.loh@oracle.com>
While one can access the BPF stack relative to %r9, the whole
point of DT_TRAMP_SP_SLOT(0) is to make trampoline code more
readable. So use it.
Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
libdtrace/dt_prov_uprobe.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 5d9f74244..f1323cc31 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -1015,22 +1015,15 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
/*
- * Look up in the BPF 'usdt_prids' map. Space for the look-up key
- * will be used on the BPF stack:
- *
- * offset value
- *
- * -sizeof(usdt_prids_map_key_t) pid (in %r0)
- *
- * -sizeof(usdt_prids_map_key_t) + sizeof(pid_t)
- * ==
- * -sizeof(dtrace_id_t) underlying-probe prid
+ * Look up in the BPF 'usdt_prids' map. The key should fit into
+ * trampoline stack slot 0.
*/
- emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0));
- emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id));
+ assert(sizeof(usdt_prids_map_key_t) <= DT_STK_SLOT_SZ);
+ emit(dlp, BPF_STORE(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), BPF_REG_0));
+ emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id));
dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id);
- emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_9));
- emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t))));
+ emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP));
+ emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0)));
emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem));
emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit));
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread[parent not found: <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com>]
* Re: [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack [not found] ` <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com> @ 2025-03-19 15:18 ` Kris Van Hees 2025-03-19 16:30 ` Eugene Loh 0 siblings, 1 reply; 3+ messages in thread From: Kris Van Hees @ 2025-03-19 15:18 UTC (permalink / raw) To: Kris Van Hees, dtrace; +Cc: eugene.loh@oracle.com, dtrace-devel On Wed, Mar 19, 2025 at 10:40:34AM -0400, Kris Van Hees via DTrace-devel wrote: > On Wed, Feb 19, 2025 at 11:43:49PM -0500, eugene.loh@oracle.com wrote: > > > > While one can access the BPF stack relative to %r9, the whole > > point of DT_TRAMP_SP_SLOT(0) is to make trampoline code more > > readable. So use it. > > > > Signed-off-by: Eugene Loh <eugene.loh@oracle.com> > > Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com> Still applies but see below... > > --- > > libdtrace/dt_prov_uprobe.c | 21 +++++++-------------- > > 1 file changed, 7 insertions(+), 14 deletions(-) > > > > diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c > > index 5d9f74244..f1323cc31 100644 > > --- a/libdtrace/dt_prov_uprobe.c > > +++ b/libdtrace/dt_prov_uprobe.c > > @@ -1015,22 +1015,15 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl) > > emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32)); > > > > /* > > - * Look up in the BPF 'usdt_prids' map. Space for the look-up key > > - * will be used on the BPF stack: > > - * > > - * offset value > > - * > > - * -sizeof(usdt_prids_map_key_t) pid (in %r0) > > - * > > - * -sizeof(usdt_prids_map_key_t) + sizeof(pid_t) > > - * == > > - * -sizeof(dtrace_id_t) underlying-probe prid > > + * Look up in the BPF 'usdt_prids' map. The key should fit into > > + * trampoline stack slot 0. > > */ > > - emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0)); > > - emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id)); > > + assert(sizeof(usdt_prids_map_key_t) <= DT_STK_SLOT_SZ); > > + emit(dlp, BPF_STORE(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), BPF_REG_0)); > > + emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); I get a compiler warning here: libdtrace/dt_prov_uprobe.c: In function â?~trampolineâ?T: include/bpf_asm.h:119:24: warning: overflow in conversion from â?~long unsigned intâ?T to â?~short intâ?T changes value from â?~18446744073709551524â?T to â?~-92â?T [-Woverflo] 119 | .off = (ofs), \ | ^ libdtrace/dt_as.h:42:69: note: in definition of macro â?~emitleâ?T 42 | dt_irnode_t *dip = dt_cg_node_alloc((lbl), (instr)); \ | ^~~~~ libdtrace/dt_prov_uprobe.c:1013:9: note: in expansion of macro â?~emitâ?T 1013 | emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); | ^~~~ libdtrace/dt_prov_uprobe.c:1013:20: note: in expansion of macro â?~BPF_STORE_IMMâ?T 1013 | emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); You need a (int) cast for sizeof(pid_t) similar to the casts that were in the code before. I'll add it in as I merge. > > dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id); > > - emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_9)); > > - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t)))); > > + emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP)); > > + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0))); > > emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem)); > > emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit)); > > > > -- > > 2.43.5 > > > > > > _______________________________________________ > DTrace-devel mailing list > DTrace-devel@oss.oracle.com > https://oss.oracle.com/mailman/listinfo/dtrace-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack 2025-03-19 15:18 ` [DTrace-devel] " Kris Van Hees @ 2025-03-19 16:30 ` Eugene Loh 0 siblings, 0 replies; 3+ messages in thread From: Eugene Loh @ 2025-03-19 16:30 UTC (permalink / raw) To: Kris Van Hees, dtrace; +Cc: dtrace-devel On 3/19/25 11:18, Kris Van Hees wrote: > On Wed, Mar 19, 2025 at 10:40:34AM -0400, Kris Van Hees via DTrace-devel wrote: >> On Wed, Feb 19, 2025 at 11:43:49PM -0500, eugene.loh@oracle.com wrote: >>> While one can access the BPF stack relative to %r9, the whole >>> point of DT_TRAMP_SP_SLOT(0) is to make trampoline code more >>> readable. So use it. >>> >>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com> >> Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com> > Still applies but see below... > >>> --- >>> libdtrace/dt_prov_uprobe.c | 21 +++++++-------------- >>> 1 file changed, 7 insertions(+), 14 deletions(-) >>> >>> diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c >>> index 5d9f74244..f1323cc31 100644 >>> --- a/libdtrace/dt_prov_uprobe.c >>> +++ b/libdtrace/dt_prov_uprobe.c >>> @@ -1015,22 +1015,15 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl) >>> emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32)); >>> >>> /* >>> - * Look up in the BPF 'usdt_prids' map. Space for the look-up key >>> - * will be used on the BPF stack: >>> - * >>> - * offset value >>> - * >>> - * -sizeof(usdt_prids_map_key_t) pid (in %r0) >>> - * >>> - * -sizeof(usdt_prids_map_key_t) + sizeof(pid_t) >>> - * == >>> - * -sizeof(dtrace_id_t) underlying-probe prid >>> + * Look up in the BPF 'usdt_prids' map. The key should fit into >>> + * trampoline stack slot 0. >>> */ >>> - emit(dlp, BPF_STORE(BPF_W, BPF_REG_9, (int)(-sizeof(usdt_prids_map_key_t)), BPF_REG_0)); >>> - emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_9, (int)(-sizeof(dtrace_id_t)), uprp->desc->id)); >>> + assert(sizeof(usdt_prids_map_key_t) <= DT_STK_SLOT_SZ); >>> + emit(dlp, BPF_STORE(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0), BPF_REG_0)); >>> + emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); > I get a compiler warning here: > > libdtrace/dt_prov_uprobe.c: In function â?~trampolineâ?T: > include/bpf_asm.h:119:24: warning: overflow in conversion from â?~long unsigned intâ?T to â?~short intâ?T changes value from â?~18446744073709551524â?T to â?~-92â?T [-Woverflo] > 119 | .off = (ofs), \ > | ^ > libdtrace/dt_as.h:42:69: note: in definition of macro â?~emitleâ?T > 42 | dt_irnode_t *dip = dt_cg_node_alloc((lbl), (instr)); \ > | ^~~~~ > > libdtrace/dt_prov_uprobe.c:1013:9: note: in expansion of macro â?~emitâ?T > 1013 | emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); > | ^~~~ > libdtrace/dt_prov_uprobe.c:1013:20: note: in expansion of macro â?~BPF_STORE_IMMâ?T > 1013 | emit(dlp, BPF_STORE_IMM(BPF_W, BPF_REG_FP, DT_TRAMP_SP_SLOT(0) + sizeof(pid_t), uprp->desc->id)); > > You need a (int) cast for sizeof(pid_t) similar to the casts that were in > the code before. I'll add it in as I merge. Thanks. Might this correction already be in the 2/2 patch? (Not that that's the right place for it, but...) >>> dt_cg_xsetx(dlp, usdt_prids, DT_LBL_NONE, BPF_REG_1, usdt_prids->di_id); >>> - emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_9)); >>> - emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, (int)(-sizeof(usdt_prids_map_key_t)))); >>> + emit(dlp, BPF_MOV_REG(BPF_REG_2, BPF_REG_FP)); >>> + emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, DT_TRAMP_SP_SLOT(0))); >>> emit(dlp, BPF_CALL_HELPER(BPF_FUNC_map_lookup_elem)); >>> emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_exit)); >>> >>> -- >>> 2.43.5 >>> >>> >> _______________________________________________ >> DTrace-devel mailing list >> DTrace-devel@oss.oracle.com >> https://oss.oracle.com/mailman/listinfo/dtrace-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-19 16:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-19 14:47 [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack Kris Van Hees
-- strict thread matches above, loose matches on Subject: below --
2025-02-20 4:43 eugene.loh
[not found] ` <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com>
2025-03-19 15:18 ` [DTrace-devel] " Kris Van Hees
2025-03-19 16:30 ` Eugene Loh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox