[DTrace-devel] [PATCH 13/20] parser: support slices of typedefs, etc
Kris Van Hees
kris.van.hees at oracle.com
Fri Jul 29 14:09:59 UTC 2022
On Wed, Jul 27, 2022 at 04:31:14PM -0400, Kris Van Hees via DTrace-devel wrote:
> On Wed, May 11, 2022 at 10:12:55PM +0100, Nick Alcock via DTrace-devel wrote:
> > Slices in CTF are almost totally transparent to the user, *except* that
> > ctf_type_resolve will halt when it encounters a slice and will drill no
> > further, since going further would lose information about the encoding
> > of the slice. Since you can have slices of typedefs, this means you
> > can't rely on ctf_type_resolve getting rid of all typedefs any more.
> >
> > This change drills through to base types where necessary. We have to do
> > it by hand in every place it's needed, which is fairly unpleasant. I
> > think I caught them all (most are covered by the tests added here). If
> > we find other places where spurious errors citing bitfields which should
> > have been treated as integral types appear, we can fix them as they crop
> > up.
> >
> > Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> > ---
> > libdtrace/dt_parser.c | 78 ++++++++++++++-----
> > .../bitfields/tst.bitfield-is-integer.d | 25 ++++++
> > test/unittest/bitfields/tst.slices.d | 28 +++++++
>
> Both tests FAIL for me. With a BPF verifier dump.
>
> Can you take current 'dev' and apply this patch and see what is going on?
I retested after a query from Eugene and this must have been a mistake on my
part. The tests pass (and the code looks right) so...
Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>
... queued for dev
>
> > 3 files changed, 113 insertions(+), 18 deletions(-)
> > create mode 100644 test/unittest/bitfields/tst.bitfield-is-integer.d
> > create mode 100644 test/unittest/bitfields/tst.slices.d
> >
> > diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> > index 3769887fb40d..bd2536882adb 100644
> > --- a/libdtrace/dt_parser.c
> > +++ b/libdtrace/dt_parser.c
> > @@ -287,6 +287,36 @@ dt_type_name(ctf_file_t *ctfp, ctf_id_t type, char *buf, size_t len)
> > return buf;
> > }
> >
> > +/* Peer through slices, cv-quals and typedefs to their base type. */
> > +ctf_id_t
> > +dt_type_basetype(ctf_file_t *fp, ctf_id_t type)
> > +{
> > + ctf_id_t newtype;
> > + uint_t kind;
> > +
> > + type = ctf_type_resolve(fp, type);
> > + kind = ctf_type_kind(fp, type);
> > +
> > + /*
> > + * Bitfields can be of typedef type, but we don't want to
> > + * elide pointers.
> > + */
> > + while ((kind == CTF_K_INTEGER || kind == CTF_K_ENUM ||
> > + kind == CTF_K_FLOAT || kind == CTF_K_TYPEDEF)
> > + && (newtype = ctf_type_reference(fp, type)) != CTF_ERR) {
> > + type = ctf_type_resolve(fp, newtype);
> > + kind = ctf_type_kind (fp, type);
> > + }
> > +
> > + return type;
> > +}
> > +
> > +ctf_id_t
> > +dt_node_basetype(const dt_node_t *dnp)
> > +{
> > + return dt_type_basetype(dnp->dn_ctfp, dnp->dn_type);
> > +}
> > +
> > /*
> > * Perform the "usual arithmetic conversions" to determine which of the two
> > * input operand types should be promoted and used as a result type. The
> > @@ -297,15 +327,16 @@ dt_type_promote(dt_node_t *lp, dt_node_t *rp, ctf_file_t **ofp, ctf_id_t *otype)
> > {
> > ctf_file_t *lfp = lp->dn_ctfp;
> > ctf_id_t ltype = lp->dn_type;
> > + ctf_id_t lbasetype = dt_node_basetype(lp);
> > + uint_t lkind = ctf_type_kind(lfp, lbasetype);
> >
> > ctf_file_t *rfp = rp->dn_ctfp;
> > ctf_id_t rtype = rp->dn_type;
> > + ctf_id_t rbasetype = dt_node_basetype(rp);
> > + uint_t rkind = ctf_type_kind(rfp, rbasetype);
> >
> > ctf_id_t lbase = ctf_type_resolve(lfp, ltype);
> > - uint_t lkind = ctf_type_kind(lfp, lbase);
> > -
> > ctf_id_t rbase = ctf_type_resolve(rfp, rtype);
> > - uint_t rkind = ctf_type_kind(rfp, rbase);
> >
> > dtrace_hdl_t *dtp = yypcb->pcb_hdl;
> > ctf_encoding_t le, re;
> > @@ -640,7 +671,8 @@ void
> > dt_node_type_assign(dt_node_t *dnp, ctf_file_t *fp, ctf_id_t type)
> > {
> > ctf_id_t base = ctf_type_resolve(fp, type);
> > - uint_t kind = ctf_type_kind(fp, base);
> > + ctf_id_t basetype = dt_type_basetype(fp, base);
> > + uint_t kind = ctf_type_kind(fp, basetype);
> > ctf_encoding_t e;
> >
> > /*
> > @@ -823,12 +855,14 @@ dt_node_is_integer(const dt_node_t *dnp)
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > ctf_id_t type;
> > + ctf_id_t basetype;
> > uint_t kind;
> >
> > assert(dnp->dn_flags & DT_NF_COOKED);
> >
> > type = ctf_type_resolve(fp, dnp->dn_type);
> > - kind = ctf_type_kind(fp, type);
> > + basetype = dt_node_basetype(dnp);
> > + kind = ctf_type_kind(fp, basetype);
> >
> > if (kind == CTF_K_INTEGER &&
> > ctf_type_encoding(fp, type, &e) == 0 && IS_VOID(e))
> > @@ -842,13 +876,14 @@ dt_node_is_float(const dt_node_t *dnp)
> > {
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > - ctf_id_t type;
> > + ctf_id_t type, basetype;
> > uint_t kind;
> >
> > assert(dnp->dn_flags & DT_NF_COOKED);
> >
> > type = ctf_type_resolve(fp, dnp->dn_type);
> > - kind = ctf_type_kind(fp, type);
> > + basetype = dt_node_basetype(dnp);
> > + kind = ctf_type_kind(fp, basetype);
> >
> > return kind == CTF_K_FLOAT &&
> > ctf_type_encoding(dnp->dn_ctfp, type, &e) == 0 && (
> > @@ -861,13 +896,14 @@ dt_node_is_scalar(const dt_node_t *dnp)
> > {
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > - ctf_id_t type;
> > + ctf_id_t type, basetype;
> > uint_t kind;
> >
> > assert(dnp->dn_flags & DT_NF_COOKED);
> >
> > type = ctf_type_resolve(fp, dnp->dn_type);
> > - kind = ctf_type_kind(fp, type);
> > + basetype = dt_node_basetype(dnp);
> > + kind = ctf_type_kind(fp, basetype);
> >
> > if (kind == CTF_K_INTEGER &&
> > ctf_type_encoding(fp, type, &e) == 0 && IS_VOID(e))
> > @@ -882,13 +918,14 @@ dt_node_is_arith(const dt_node_t *dnp)
> > {
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > - ctf_id_t type;
> > + ctf_id_t type, basetype;
> > uint_t kind;
> >
> > assert(dnp->dn_flags & DT_NF_COOKED);
> >
> > type = ctf_type_resolve(fp, dnp->dn_type);
> > - kind = ctf_type_kind(fp, type);
> > + basetype = dt_node_basetype(dnp);
> > + kind = ctf_type_kind(fp, basetype);
> >
> > if (kind == CTF_K_INTEGER)
> > return ctf_type_encoding(fp, type, &e) == 0 && !IS_VOID(e);
> > @@ -901,7 +938,7 @@ dt_node_is_vfptr(const dt_node_t *dnp)
> > {
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > - ctf_id_t type;
> > + ctf_id_t type, basetype;
> > uint_t kind;
> >
> > assert(dnp->dn_flags & DT_NF_COOKED);
> > @@ -911,7 +948,8 @@ dt_node_is_vfptr(const dt_node_t *dnp)
> > return 0; /* type is not a pointer */
> >
> > type = ctf_type_resolve(fp, ctf_type_reference(fp, type));
> > - kind = ctf_type_kind(fp, type);
> > + basetype = dt_type_basetype(fp, type);
> > + kind = ctf_type_kind(fp, basetype);
> >
> > return kind == CTF_K_FUNCTION || (kind == CTF_K_INTEGER &&
> > ctf_type_encoding(fp, type, &e) == 0 && IS_VOID(e));
> > @@ -1006,7 +1044,7 @@ dt_node_is_void(const dt_node_t *dnp)
> > {
> > ctf_file_t *fp = dnp->dn_ctfp;
> > ctf_encoding_t e;
> > - ctf_id_t type;
> > + ctf_id_t type, basetype;
> >
> > if (dt_node_is_dynamic(dnp))
> > return 0; /* <DYN> is an alias for void but not the same */
> > @@ -1018,8 +1056,9 @@ dt_node_is_void(const dt_node_t *dnp)
> > return 0;
> >
> > type = ctf_type_resolve(fp, dnp->dn_type);
> > + basetype = dt_node_basetype(dnp);
> >
> > - return ctf_type_kind(fp, type) == CTF_K_INTEGER &&
> > + return ctf_type_kind(fp, basetype) == CTF_K_INTEGER &&
> > ctf_type_encoding(fp, type, &e) == 0 && IS_VOID(e);
> > }
> >
> > @@ -1667,12 +1706,14 @@ dt_node_decl(void)
> > ctf_encoding_t cte;
> > ctf_arinfo_t r;
> > ctf_id_t etype;
> > + ctf_id_t basetype;
> > uint_t kind;
> > uint_t alignment = 8;
> > uint_t size;
> >
> > type = ctf_type_resolve(dtt.dtt_ctfp, dtt.dtt_type);
> > - kind = ctf_type_kind(dtt.dtt_ctfp, type);
> > + basetype = dt_type_basetype(dtt.dtt_ctfp, dtt.dtt_type);
> > + kind = ctf_type_kind(dtt.dtt_ctfp, basetype);
> > size = ctf_type_size(dtt.dtt_ctfp, dtt.dtt_type);
> >
> > switch (kind) {
> > @@ -2987,7 +3028,7 @@ dt_cook_op1(dt_node_t *dnp, uint_t idflags)
> >
> > ctf_encoding_t e;
> > ctf_arinfo_t r;
> > - ctf_id_t type, base;
> > + ctf_id_t type, base, basetype;
> > uint_t kind;
> >
> > if (dnp->dn_op == DT_TOK_PREINC || dnp->dn_op == DT_TOK_POSTINC ||
> > @@ -3054,7 +3095,8 @@ dt_cook_op1(dt_node_t *dnp, uint_t idflags)
> >
> > dt_node_type_assign(dnp, cp->dn_ctfp, type);
> > base = ctf_type_resolve(cp->dn_ctfp, type);
> > - kind = ctf_type_kind(cp->dn_ctfp, base);
> > + basetype = dt_type_basetype (cp->dn_ctfp, type);
> > + kind = ctf_type_kind(cp->dn_ctfp, basetype);
> >
> > if (kind == CTF_K_INTEGER && ctf_type_encoding(cp->dn_ctfp,
> > base, &e) == 0 && IS_VOID(e)) {
> > diff --git a/test/unittest/bitfields/tst.bitfield-is-integer.d b/test/unittest/bitfields/tst.bitfield-is-integer.d
> > new file mode 100644
> > index 000000000000..013544b259c7
> > --- /dev/null
> > +++ b/test/unittest/bitfields/tst.bitfield-is-integer.d
> > @@ -0,0 +1,25 @@
> > +/*
> > + * 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.
> > + */
> > +
> > +/*
> > + * ASSERTION:
> > + * Bitfields are treated as integers.
> > + *
> > + * SECTION: Types, Operators, and Expressions/Bitwise Operators
> > + */
> > +
> > +#pragma D option quiet
> > +
> > +/* @@runtest-opts: -e */
> > +
> > +/* @@trigger: bogus-ioctl */
> > +
> > +syscall::ioctl:entry / curthread->frozen == 0 /
> > +{ exit(0); }
> > +
> > +syscall::ioctl:entry / curthread->frozen != 0 /
> > +{ exit(0); }
> > diff --git a/test/unittest/bitfields/tst.slices.d b/test/unittest/bitfields/tst.slices.d
> > new file mode 100644
> > index 000000000000..d69068b4435e
> > --- /dev/null
> > +++ b/test/unittest/bitfields/tst.slices.d
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Oracle Linux DTrace.
> > + * Copyright (c) 2020, 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.
> > + */
> > +
> > +/*
> > + * Check that lookup of types in CTF that happen to be bitfields works.
> > + * (These types are represented as slices.)
> > + * We are only interested in whether they parse, so exit immediately
> > + * after that.
> > + */
> > +
> > +/* @@runtest-opts: -e */
> > +
> > +#pragma D option quiet
> > +
> > +syscall::ioctl:entry
> > +{
> > + trace(curthread->sched_reset_on_fork);
> > +}
> > +
> > +fbt::tcp_parse_options:entry
> > +{
> > + /* dtv2: use args[2] later */
> > + trace(((struct sk_buff *)arg2)->nohdr);
> > +}
> > --
> > 2.36.1.263.g194b774378.dirty
> >
> >
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel at oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel
>
> _______________________________________________
> 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