[DTrace-devel] [PATCH 13/20] parser: support slices of typedefs, etc
Kris Van Hees
kris.van.hees at oracle.com
Wed Jul 27 20:31:14 UTC 2022
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?
> 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
More information about the DTrace-devel
mailing list