From: Kris Van Hees <kris.van.hees@oracle.com>
To: Kris Van Hees <kris.van.hees@oracle.com>, dtrace@lists.linux.dev
Cc: "eugene.loh@oracle.com" <eugene.loh@oracle.com>,
dtrace-devel@oss.oracle.com
Subject: Re: [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack
Date: Wed, 19 Mar 2025 11:18:03 -0400 [thread overview]
Message-ID: <Z9rgKzWMHSqLVQdK@oracle.com> (raw)
In-Reply-To: <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com>
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
next prev parent reply other threads:[~2025-03-19 15:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-20 4:43 [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack eugene.loh
2025-02-20 4:43 ` [PATCH 2/2] Extend the USDT bit mask to multiple words eugene.loh
[not found] ` <Z9rXYvNQxZqODC3o@kvh-deb-bpf.us.oracle.com>
2025-03-19 15:18 ` Kris Van Hees [this message]
2025-03-19 16:30 ` [DTrace-devel] [PATCH 1/2] Clarify how the usdt_prids key is stored on the BPF stack Eugene Loh
-- strict thread matches above, loose matches on Subject: below --
2025-03-19 14:47 Kris Van Hees
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z9rgKzWMHSqLVQdK@oracle.com \
--to=kris.van.hees@oracle.com \
--cc=dtrace-devel@oss.oracle.com \
--cc=dtrace@lists.linux.dev \
--cc=eugene.loh@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox