[DTrace-devel] [PATCH] Fix DIFO strtab handling
Kris Van Hees
kris.van.hees at oracle.com
Thu Mar 10 18:49:51 UTC 2022
On Thu, Mar 10, 2022 at 01:34:09PM -0500, Eugene Loh via DTrace-devel wrote:
> Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
> with a few comments...
>
> On 3/10/22 1:39 AM, Kris Van Hees via DTrace-devel wrote:
> > Incorrect hansling of the DIFO strtab caused DIFOs to have string
> s/hansling/handling/
I blame my fingers :)
> > constant tables that were missing items. This caused annotations to
> > report incorrect strings.
> >
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> > libdtrace/dt_cc.c | 28 ++++++--------
> > .../unittest/disasm/tst.ann-strconst-strtab.r | 2 +
> > .../disasm/tst.ann-strconst-strtab.sh | 37 +++++++++++++++++++
> > 3 files changed, 50 insertions(+), 17 deletions(-)
> > create mode 100644 test/unittest/disasm/tst.ann-strconst-strtab.r
> > create mode 100755 test/unittest/disasm/tst.ann-strconst-strtab.sh
> >
> > diff --git a/libdtrace/dt_cc.c b/libdtrace/dt_cc.c
> > index 49901710..99577b7d 100644
> > --- a/libdtrace/dt_cc.c
> > +++ b/libdtrace/dt_cc.c
> > @@ -2234,9 +2234,8 @@ static int get_boottime() {
> > static int
> > dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > - dt_ident_t *idp, const dtrace_difo_t *sdp, dt_strtab_t *stab,
> > - uint_t *pcp, uint_t *rcp, uint_t *vcp, dtrace_epid_t epid,
> > - uint_t clid)
> > + dt_ident_t *idp, const dtrace_difo_t *sdp, uint_t *pcp,
> > + uint_t *rcp, uint_t *vcp, dtrace_epid_t epid, uint_t clid)
> > {
> > uint_t pc = *pcp;
> > uint_t rc = *rcp;
> > @@ -2276,7 +2275,7 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > const char *name = dt_difo_getstr(sdp, vp->dtdv_name);
> > *nvp = *vp;
> > - nvp->dtdv_name = dt_strtab_insert(stab, name);
> > + nvp->dtdv_name = dt_strtab_insert(dtp->dt_ccstab, name);
> > nvp->dtdv_insn_from += pc;
> > nvp->dtdv_insn_to += pc;
> > }
> > @@ -2293,7 +2292,7 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > const char *name = dt_difo_getstr(sdp, rp->dofr_name);
> > dt_ident_t *idp = dt_dlib_get_func(dtp, name);
> > - nrp->dofr_name = dt_strtab_insert(stab, name);
> > + nrp->dofr_name = dt_strtab_insert(dtp->dt_ccstab, name);
> > nrp->dofr_type = rp->dofr_type;
> > nrp->dofr_offset = rp->dofr_offset +
> > pc * sizeof(struct bpf_insn);
> > @@ -2395,8 +2394,8 @@ dt_link_construct(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > clid++;
> > } else
> > nepid = 0;
> > - ipc = dt_link_construct(dtp, prp, dp, idp, rdp, stab,
> > - pcp, rcp, vcp, nepid, clid);
> > + ipc = dt_link_construct(dtp, prp, dp, idp, rdp, pcp,
> > + rcp, vcp, nepid, clid);
> > if (ipc == -1)
> > return -1;
> > @@ -2465,7 +2464,6 @@ dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > uint_t relc = 0;
> > uint_t varc = 0;
> > dtrace_difo_t *fdp = NULL;
> > - dt_strtab_t *stab;
> > int rc;
> > /*
> > @@ -2506,12 +2504,9 @@ dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > * string table, and variable table).
> > */
> > insc = relc = varc = 0;
> > - stab = dt_strtab_create(BUFSIZ);
> > - if (stab == NULL)
> > - goto nomem;
> > - rc = dt_link_construct(dtp, prp, fdp, idp, dp, stab, &insc, &relc,
> > - &varc, 0, 0);
> > + rc = dt_link_construct(dtp, prp, fdp, idp, dp, &insc, &relc, &varc, 0,
> > + 0);
> > dt_dlib_reset(dtp, B_FALSE);
> > if (rc == -1)
> > goto fail;
> > @@ -2535,18 +2530,17 @@ dt_link(dtrace_hdl_t *dtp, const dt_probe_t *prp, dtrace_difo_t *dp,
> > * Write out the new string table.
> > */
> > dt_free(dtp, dp->dtdo_strtab);
> > - dp->dtdo_strlen = dt_strtab_size(stab);
> > + dp->dtdo_strlen = dt_strtab_size(dtp->dt_ccstab);
> > if (dp->dtdo_strlen > 0) {
> > dp->dtdo_strtab = dt_zalloc(dtp, dp->dtdo_strlen);
> > if (dp->dtdo_strtab == NULL)
> > goto nomem;
> > - dt_strtab_write(stab, (dt_strtab_write_f *)dt_strtab_copystr,
> > + dt_strtab_write(dtp->dt_ccstab,
> > + (dt_strtab_write_f *)dt_strtab_copystr,
> > dp->dtdo_strtab);
> > } else
> > dp->dtdo_strtab = NULL;
> > - dt_strtab_destroy(stab);
> > -
> > /*
> > * Resolve the function relocation records.
> > */
> > diff --git a/test/unittest/disasm/tst.ann-strconst-strtab.r b/test/unittest/disasm/tst.ann-strconst-strtab.r
> > new file mode 100644
> > index 00000000..d3c2bfbd
> > --- /dev/null
> > +++ b/test/unittest/disasm/tst.ann-strconst-strtab.r
> > @@ -0,0 +1,2 @@
> > +linked 07 7 0 0000 XXXXXXXX add %rX, D ! "a"
> > +final 07 7 0 0000 XXXXXXXX add %rX, D ! "a"
> > diff --git a/test/unittest/disasm/tst.ann-strconst-strtab.sh b/test/unittest/disasm/tst.ann-strconst-strtab.sh
> > new file mode 100755
> > index 00000000..1becdf4c
> > --- /dev/null
> > +++ b/test/unittest/disasm/tst.ann-strconst-strtab.sh
> > @@ -0,0 +1,37 @@
> > +#!/bin/bash
> > +#
> > +# Oracle Linux DTrace.
> > +# Copyright (c) 2022, 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.
> > +#
> > +# The process of linking BPF functions together into a program caused the
> > +# strtab for a DIFO to not contain all the strings needed for the code. This
> > +# was evidenced through incorrect string constant annotations on the linked and
> > +# final program disassembler listings.
> > +#
> > +# This test verifies that this problem is no longer present.
> > +#
> > +
> > +dtrace=$1
> > +
> > +$dtrace $dt_flags -xdisasm=12 -Sn '
> > +BEGIN
> > +{
> > + a["a"] = 42;
> To my taste, it is much clearer if the variable and key string are
> different, so that it's easier to see what means what. Even just foo["bar"]
> would be better.
This test is rather bizarre and oddly specific. I am crafting a particular
case here where the variable name and the constant string used as index map
onto the same string constant in the strrab.
> > + exit(0);
> > +}
> > +' 2>&1 | \
> > + awk '/^Disassembly of/ {
> > + kind = $3;
> > + next;
> > + }
> > + / ! "/ {
> > + sub(/^[^:]+:/, kind);
> > + sub(/^07 [0-9] /, "07 X ");
> Does that do anything? We just prepended kind. So there is no more ^07.
Thanks for catching that mistake.
> > + sub(/[0-9a-f]{8} add/, "XXXXXXXX add");
> > + sub(/%r[0-9], [0-9]+ +/, "%rX, D ");
> > + print;
> > + }'
> > +
> > +exit $?
>
> _______________________________________________
> 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