[DTrace-devel] [PATCH] Fix DIFO strtab handling
Eugene Loh
eugene.loh at oracle.com
Thu Mar 10 18:34:09 UTC 2022
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/
> 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.
> + 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.
> + sub(/[0-9a-f]{8} add/, "XXXXXXXX add");
> + sub(/%r[0-9], [0-9]+ +/, "%rX, D ");
> + print;
> + }'
> +
> +exit $?
More information about the DTrace-devel
mailing list