[DTrace-devel] [PATCH 02/12] cg: Dummy implementation for d_path()
Kris Van Hees
kris.van.hees at oracle.com
Sat Jan 6 00:21:04 UTC 2024
On Fri, Jan 05, 2024 at 03:56:12PM -0500, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
>
> But why do the .r results files change fi_dirname to .? Why not to
> <unknown>?
Because fi_dirname is defined by the translator as dirname(d_path(...)) so it
is dirname("<unknown>") which is ".".
> How about adding a @@nosort to tst.fds.d while we're at it? Not strictly
> part of this patch, but this could be a convenient time to add it.
Sure, but not really relevant since each line already has a numeric id (the
fd) and member name, so all lines are unique, and while the members are sorted
alphabetically, they are still all there and order does not really matter.
> How about a more descriptive name than <unknown>? Say, <d_path not
> implemented>? I can see someone thinking that's distasteful, so I won't
> push that suggestion.
I thought of that but other translators that provide fileinfo use <unknown>
as value for unknown values, so it seems to make sense to use the same for
consistency.
> How about adding a comment to the commit message about the BPF d_path helper
> function?
I don't think that would be appropriate because the commit message should
describe the patch. Adding an explanation about the BPF helper and why it is
not (yet or ever) used does not seem to be relevant here. That is more
something for when we actually do use it or for discussions about whether we
can (or why not).
I don't think it would adds value to this patch to discuss that in this
commit message.
> On 1/5/24 00:24, Kris Van Hees via DTrace-devel wrote:
> > Hardwire d_path() to always return "<unknown>" pending an implementation
> > that really works. This stub allows io and procfs translators to work.
> > Tests that will have their expected output adjusted for this stub will
> > FAIL when a real implementation is introduced, and should be adjusted at
> > that time.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_cg.c | 34 +++++++++++++++++++++++++++++-
> > test/unittest/io/tst.fds.aarch64.r | 20 +++++++++---------
> > test/unittest/io/tst.fds.d | 3 +--
> > test/unittest/io/tst.fds.sparc64.r | 20 +++++++++---------
> > test/unittest/io/tst.fds.x86_64.r | 20 +++++++++---------
> > 5 files changed, 64 insertions(+), 33 deletions(-)
> >
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index 4332954e..30b1da16 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -6257,6 +6257,38 @@ dt_cg_subr_inet_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > tnp->dn_tstring = NULL;
> > }
> > +static void
> > +dt_cg_subr_d_path(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > +{
> > + dt_node_t *arg = dnp->dn_args;
> > +
> > + dt_cg_node(arg, dlp, drp);
> > + dt_cg_check_ptr_arg(dlp, drp, arg, NULL);
> > +
> > + /* Allocate a temporary string for the result. */
> > + dnp->dn_reg = dt_regset_alloc(drp);
> > + if (dnp->dn_reg == -1)
> > + longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> > + dt_cg_tstring_alloc(yypcb, dnp);
> > + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, BPF_REG_FP, DT_STK_DCTX));
> > + emit(dlp, BPF_LOAD(BPF_DW, dnp->dn_reg, dnp->dn_reg, DCTX_MEM));
> > + emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, dnp->dn_tstring->dn_value));
> > +
> > + /*
> > + * We do not have a real implementation yet, so throw away the result
> > + * of evaluating the argument, and return a default value "<unknown>"
> > + * in a temporary string.
> > + *
> > + * <unknown> is 3c 75 6e 6b 6e 6f 77 6e 3e 00
> > + */
> > + dt_regset_free(drp, arg->dn_reg);
> > + dt_cg_tstring_free(yypcb, arg);
> > +
> > + emit(dlp, BPF_STORE_IMM(BPF_W, dnp->dn_reg, 0, 0x6b6e753c));
> > + emit(dlp, BPF_STORE_IMM(BPF_W, dnp->dn_reg, 4, 0x6e776f6e));
> > + emit(dlp, BPF_STORE_IMM(BPF_H, dnp->dn_reg, 8, 0x003e));
> > +}
> > +
> > static void
> > dt_cg_subr_link_ntop(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> > {
> > @@ -6323,7 +6355,7 @@ static dt_cg_subr_f *_dt_cg_subr[DIF_SUBR_MAX + 1] = {
> > [DIF_SUBR_INET_NTOP] = &dt_cg_subr_inet_ntop,
> > [DIF_SUBR_INET_NTOA] = &dt_cg_subr_inet_ntoa,
> > [DIF_SUBR_INET_NTOA6] = &dt_cg_subr_inet_ntoa6,
> > - [DIF_SUBR_D_PATH] = NULL,
> > + [DIF_SUBR_D_PATH] = &dt_cg_subr_d_path,
> > [DIF_SUBR_LINK_NTOP] = &dt_cg_subr_link_ntop,
> > };
> > diff --git a/test/unittest/io/tst.fds.aarch64.r b/test/unittest/io/tst.fds.aarch64.r
> > index e953785c..e1160e5d 100644
> > --- a/test/unittest/io/tst.fds.aarch64.r
> > +++ b/test/unittest/io/tst.fds.aarch64.r
> > @@ -1,31 +1,31 @@
> > -fds[0] fi_dirname = /proc/#
> > +fds[0] fi_dirname = .
> > fds[0] fi_mount = <unknown>
> > fds[0] fi_name = mem
> > fds[0] fi_offset = 0
> > fds[0] fi_oflags = 20000
> > -fds[0] fi_pathname = /proc/#/mem
> > -fds[1] fi_dirname = /proc/#
> > +fds[0] fi_pathname = <unknown>
> > +fds[1] fi_dirname = .
> > fds[1] fi_mount = <unknown>
> > fds[1] fi_name = mem
> > fds[1] fi_offset = 0
> > fds[1] fi_oflags = 20001
> > -fds[1] fi_pathname = /proc/#/mem
> > -fds[2] fi_dirname = /proc/#
> > +fds[1] fi_pathname = <unknown>
> > +fds[2] fi_dirname = .
> > fds[2] fi_mount = <unknown>
> > fds[2] fi_name = mem
> > fds[2] fi_offset = 0
> > fds[2] fi_oflags = 20002
> > -fds[2] fi_pathname = /proc/#/mem
> > -fds[3] fi_dirname = /proc/#
> > +fds[2] fi_pathname = <unknown>
> > +fds[3] fi_dirname = .
> > fds[3] fi_mount = <unknown>
> > fds[3] fi_name = mem
> > fds[3] fi_offset = 0
> > fds[3] fi_oflags = 121c02
> > -fds[3] fi_pathname = /proc/#/mem
> > -fds[4] fi_dirname = /proc/#
> > +fds[3] fi_pathname = <unknown>
> > +fds[4] fi_dirname = .
> > fds[4] fi_mount = <unknown>
> > fds[4] fi_name = mem
> > fds[4] fi_offset = 123
> > fds[4] fi_oflags = 20002
> > -fds[4] fi_pathname = /proc/#/mem
> > +fds[4] fi_pathname = <unknown>
> > diff --git a/test/unittest/io/tst.fds.d b/test/unittest/io/tst.fds.d
> > index b762e6ac..06caefe4 100644
> > --- a/test/unittest/io/tst.fds.d
> > +++ b/test/unittest/io/tst.fds.d
> > @@ -1,11 +1,10 @@
> > /*
> > * Oracle Linux DTrace.
> > - * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
> > + * Copyright (c) 2006, 2024, Oracle and/or its affiliates. All rights reserved.
> > * Licensed under the Universal Permissive License v 1.0 as shown at
> > * http://oss.oracle.com/licenses/upl.
> > */
> > -/* @@xfail: dtv2 */
> > /* @@trigger: bogus-ioctl */
> > /* @@trigger-timing: after */
> > /* @@runtest-opts: -C $_pid */
> > diff --git a/test/unittest/io/tst.fds.sparc64.r b/test/unittest/io/tst.fds.sparc64.r
> > index d787a14b..71cc8309 100644
> > --- a/test/unittest/io/tst.fds.sparc64.r
> > +++ b/test/unittest/io/tst.fds.sparc64.r
> > @@ -1,31 +1,31 @@
> > -fds[0] fi_dirname = /proc/#
> > +fds[0] fi_dirname = .
> > fds[0] fi_mount = <unknown>
> > fds[0] fi_name = mem
> > fds[0] fi_offset = 0
> > fds[0] fi_oflags = 40000
> > -fds[0] fi_pathname = /proc/#/mem
> > -fds[1] fi_dirname = /proc/#
> > +fds[0] fi_pathname = <unknown>
> > +fds[1] fi_dirname = .
> > fds[1] fi_mount = <unknown>
> > fds[1] fi_name = mem
> > fds[1] fi_offset = 0
> > fds[1] fi_oflags = 40001
> > -fds[1] fi_pathname = /proc/#/mem
> > -fds[2] fi_dirname = /proc/#
> > +fds[1] fi_pathname = <unknown>
> > +fds[2] fi_dirname = .
> > fds[2] fi_mount = <unknown>
> > fds[2] fi_name = mem
> > fds[2] fi_offset = 0
> > fds[2] fi_oflags = 40002
> > -fds[2] fi_pathname = /proc/#/mem
> > -fds[3] fi_dirname = /proc/#
> > +fds[2] fi_pathname = <unknown>
> > +fds[3] fi_dirname = .
> > fds[3] fi_mount = <unknown>
> > fds[3] fi_name = mem
> > fds[3] fi_offset = 0
> > fds[3] fi_oflags = 84600e
> > -fds[3] fi_pathname = /proc/#/mem
> > -fds[4] fi_dirname = /proc/#
> > +fds[3] fi_pathname = <unknown>
> > +fds[4] fi_dirname = .
> > fds[4] fi_mount = <unknown>
> > fds[4] fi_name = mem
> > fds[4] fi_offset = 123
> > fds[4] fi_oflags = 40002
> > -fds[4] fi_pathname = /proc/#/mem
> > +fds[4] fi_pathname = <unknown>
> > diff --git a/test/unittest/io/tst.fds.x86_64.r b/test/unittest/io/tst.fds.x86_64.r
> > index ec42330c..799e8562 100644
> > --- a/test/unittest/io/tst.fds.x86_64.r
> > +++ b/test/unittest/io/tst.fds.x86_64.r
> > @@ -1,31 +1,31 @@
> > -fds[0] fi_dirname = /proc/#
> > +fds[0] fi_dirname = .
> > fds[0] fi_mount = <unknown>
> > fds[0] fi_name = mem
> > fds[0] fi_offset = 0
> > fds[0] fi_oflags = 8000
> > -fds[0] fi_pathname = /proc/#/mem
> > -fds[1] fi_dirname = /proc/#
> > +fds[0] fi_pathname = <unknown>
> > +fds[1] fi_dirname = .
> > fds[1] fi_mount = <unknown>
> > fds[1] fi_name = mem
> > fds[1] fi_offset = 0
> > fds[1] fi_oflags = 8001
> > -fds[1] fi_pathname = /proc/#/mem
> > -fds[2] fi_dirname = /proc/#
> > +fds[1] fi_pathname = <unknown>
> > +fds[2] fi_dirname = .
> > fds[2] fi_mount = <unknown>
> > fds[2] fi_name = mem
> > fds[2] fi_offset = 0
> > fds[2] fi_oflags = 8002
> > -fds[2] fi_pathname = /proc/#/mem
> > -fds[3] fi_dirname = /proc/#
> > +fds[2] fi_pathname = <unknown>
> > +fds[3] fi_dirname = .
> > fds[3] fi_mount = <unknown>
> > fds[3] fi_name = mem
> > fds[3] fi_offset = 0
> > fds[3] fi_oflags = 109c02
> > -fds[3] fi_pathname = /proc/#/mem
> > -fds[4] fi_dirname = /proc/#
> > +fds[3] fi_pathname = <unknown>
> > +fds[4] fi_dirname = .
> > fds[4] fi_mount = <unknown>
> > fds[4] fi_name = mem
> > fds[4] fi_offset = 123
> > fds[4] fi_oflags = 8002
> > -fds[4] fi_pathname = /proc/#/mem
> > +fds[4] fi_pathname = <unknown>
>
> _______________________________________________
> 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