[DTrace-devel] [PATCH v2 22/61] Fix typecast
Kris Van Hees
kris.van.hees at oracle.com
Mon Aug 1 18:59:02 UTC 2022
I made a modified versidon of this patch because testing showed that actual
casting is only necessary in some specific cases. The tests you include pass
with my modified version (which I will post in a moment for review).
The modified patch is either fully sufficient or the testcases are not
sufficiently exercising all possible cases.
Kris
On Wed, Jul 13, 2022 at 09:03:47PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
>
> DTrace internally keeps integers basically in a 64-bit format. Shorter,
> signed integers are already sign-extended.
>
> The current logic makes an error when an integer is extended and the
> signedness changes. It truncates the source and then extends. E.g.,
> consider a 1-byte source and 2-byte destination:
>
> signed?
> src dst input expected currently
>
> N Y 0xff 0x00ff -1
> Y N 0xff 0xffff 0xff
>
> Note:
>
> *) In the first case, no casting is even necessary. If the integer
> is being extended and it is unsigned, then higher-order bits can simply
> be left alone.
>
> *) In the second case, the signed input is supposed to be sign-extended
> before truncating to the new size. We currently truncate first, at the
> smaller size.
>
> Fix. Add tests.
>
> Notice that:
>
> *) Casting constants like "var = (type) constant" is handled
> in the parser.
>
> *) Casting variables like "var1 = (type) var2" is handled
> in the code generator.
>
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
> libdtrace/dt_cg.c | 41 ++++++----
> libdtrace/dt_parser.c | 24 ++++--
> test/unittest/arithmetic/tst.cast-exp.d | 101 ++++++++++++++++++++++++
> test/unittest/arithmetic/tst.cast-exp.r | 43 ++++++++++
> 4 files changed, 186 insertions(+), 23 deletions(-)
> create mode 100644 test/unittest/arithmetic/tst.cast-exp.d
> create mode 100644 test/unittest/arithmetic/tst.cast-exp.r
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 9b47dc02..a84359ea 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2563,23 +2563,13 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
> if (dst->dn_ctfp == NULL && dst->dn_type == CTF_ERR)
> return;
>
> - srcsize = dt_node_type_size(src);
> - dstsize = dt_node_type_size(dst);
> -
> - if (dstsize < srcsize)
> - n = sizeof(uint64_t) * NBBY - dstsize * NBBY;
> - else
> - n = sizeof(uint64_t) * NBBY - srcsize * NBBY;
> -
> if (!dt_node_is_scalar(dst))
> return;
>
> - if (n != 0 && (dstsize < srcsize ||
> - (src->dn_flags & DT_NF_SIGNED) ^ (dst->dn_flags & DT_NF_SIGNED))) {
> - emit(dlp, BPF_MOV_REG(dst->dn_reg, src->dn_reg));
> - emit(dlp, BPF_ALU64_IMM(BPF_LSH, dst->dn_reg, n));
> - emit(dlp, BPF_ALU64_IMM((dst->dn_flags & DT_NF_SIGNED) ? BPF_ARSH : BPF_RSH, dst->dn_reg, n));
> - } else if (dt_node_is_arith(dst) && dt_node_is_pointer(src) &&
> + srcsize = dt_node_type_size(src);
> + dstsize = dt_node_type_size(dst);
> +
> + if (dt_node_is_arith(dst) && dt_node_is_pointer(src) &&
> (src->dn_flags & DT_NF_ALLOCA)) {
> int mst;
>
> @@ -2592,8 +2582,29 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
> emit(dlp, BPF_LOAD(BPF_DW, dst->dn_reg, mst, DMST_SCALARIZER));
>
> dt_regset_free(drp, mst);
> - } else
> + } else {
> + int cast = 1;
> + int srcsigned = src->dn_flags & DT_NF_SIGNED;
> + int dstsigned = dst->dn_flags & DT_NF_SIGNED;
> +
> emit(dlp, BPF_MOV_REG(dst->dn_reg, src->dn_reg));
> +
> + n = sizeof(uint64_t) * NBBY - dstsize * NBBY;
> + if (n == 0) {
> + cast = 0;
> + } else if (dstsize > srcsize) {
> + if (dstsigned || !srcsigned)
> + cast = 0;
> + } else if (dstsize == srcsize) {
> + if (dstsigned == srcsigned)
> + cast = 0;
> + }
> +
> + if (cast) {
> + emit(dlp, BPF_ALU64_IMM(BPF_LSH, dst->dn_reg, n));
> + emit(dlp, BPF_ALU64_IMM(dstsigned ? BPF_ARSH : BPF_RSH, dst->dn_reg, n));
> + }
> + }
> }
>
> /*
> diff --git a/libdtrace/dt_parser.c b/libdtrace/dt_parser.c
> index 4377bb06..c9b6e88e 100644
> --- a/libdtrace/dt_parser.c
> +++ b/libdtrace/dt_parser.c
> @@ -2082,18 +2082,26 @@ dt_node_op2(int op, dt_node_t *lp, dt_node_t *rp)
> dt_node_is_integer(lp)) {
> size_t srcsize = dt_node_type_size(rp);
> size_t dstsize = dt_node_type_size(lp);
> + int n = sizeof(uint64_t) * NBBY - dstsize * NBBY, cast = 1;
> + int srcsigned = rp->dn_flags & DT_NF_SIGNED;
> + int dstsigned = lp->dn_flags & DT_NF_SIGNED;
> +
> + if (n == 0) {
> + cast = 0;
> + } else if (dstsize > srcsize) {
> + if (dstsigned || !srcsigned)
> + cast = 0;
> + } else if (dstsize == srcsize) {
> + if (dstsigned == srcsigned)
> + cast = 0;
> + }
>
> - if ((dstsize < srcsize) || ((lp->dn_flags & DT_NF_SIGNED) ^
> - (rp->dn_flags & DT_NF_SIGNED))) {
> - int n = dstsize < srcsize ?
> - (sizeof(uint64_t) * NBBY - dstsize * NBBY) :
> - (sizeof(uint64_t) * NBBY - srcsize * NBBY);
> -
> + if (cast) {
> rp->dn_value <<= n;
> - if (lp->dn_flags & DT_NF_SIGNED)
> + if (dstsigned)
> rp->dn_value = (intmax_t)rp->dn_value >> n;
> else
> - rp->dn_value = rp->dn_value >> n;
> + rp->dn_value >>= n;
> }
>
> dt_node_type_propagate(lp, rp);
> diff --git a/test/unittest/arithmetic/tst.cast-exp.d b/test/unittest/arithmetic/tst.cast-exp.d
> new file mode 100644
> index 00000000..6939586b
> --- /dev/null
> +++ b/test/unittest/arithmetic/tst.cast-exp.d
> @@ -0,0 +1,101 @@
> +/*
> + * 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: Integers are typecast correctly. (explicit)
> + * Compare results to a similar C program,
> + * albeit with the format string
> + * "%hhd %hd %d %lld %hhu %hu %u %llu\n".
> + *
> + * SECTION: Types, Operators, and Expressions/Arithmetic Operators
> + */
> +/* @@runtest-opts: -qC */
> +
> +signed char c;
> +short s;
> +int i;
> +long long l;
> +unsigned char C;
> +unsigned short S;
> +unsigned int I;
> +unsigned long long L;
> +
> +#define TEST(x) \
> + c = (char)(x); \
> + s = (short)(x); \
> + i = (int)(x); \
> + l = (long long)(x); \
> + C = (unsigned char)(x); \
> + S = (unsigned short)(x); \
> + I = (unsigned int)(x); \
> + L = (unsigned long long)(x); \
> + printf("%d %d %d %d %d %d %d %d\n", c, s, i, l, C, S, I, L)
> +
> +BEGIN
> +{
> + /* from scalar */
> + TEST(-2);
> + TEST(0xfffffffffffffffe);
> + TEST(0xfffffffe);
> + TEST(0xfffe);
> + TEST(0xfe);
> + TEST(2);
> + TEST(0x55);
> + TEST(0x5555);
> + TEST(0x55555555);
> + TEST(0x5555555555555555);
> +
> + /* from char */
> + c = -2; TEST(c);
> + c = 0xfe; TEST(c);
> + c = 2; TEST(c);
> + c = 0x55; TEST(c);
> +
> + /* from short */
> + s = -2; TEST(s);
> + s = 0xfffe; TEST(s);
> + s = 2; TEST(s);
> + s = 0x5555; TEST(s);
> +
> + /* from int */
> + i = -2; TEST(i);
> + i = 0xfffffffe; TEST(i);
> + i = 2; TEST(i);
> + i = 0x55555555; TEST(i);
> +
> + /* from long long */
> + l = -2; TEST(l);
> + l = 0xfffffffffffffffe; TEST(l);
> + l = 2; TEST(l);
> + l = 0x5555555555555555; TEST(l);
> +
> + /* from unsigned char */
> + C = -2; TEST(C);
> + C = 0xfe; TEST(C);
> + C = 2; TEST(C);
> + C = 0x55; TEST(C);
> +
> + /* from unsigned short */
> + S = -2; TEST(S);
> + S = 0xfffe; TEST(S);
> + S = 2; TEST(S);
> + S = 0x5555; TEST(S);
> +
> + /* from unsigned int */
> + I = -2; TEST(I);
> + I = 0xfffffffe; TEST(I);
> + I = 2; TEST(I);
> + I = 0x55555555; TEST(I);
> +
> + /* from unsigned long long */
> + L = -2; TEST(L);
> + L = 0xfffffffffffffffe; TEST(L);
> + L = 2; TEST(L);
> + L = 0x5555555555555555; TEST(L);
> +
> + exit (0);
> +}
> diff --git a/test/unittest/arithmetic/tst.cast-exp.r b/test/unittest/arithmetic/tst.cast-exp.r
> new file mode 100644
> index 00000000..0ae3e3f1
> --- /dev/null
> +++ b/test/unittest/arithmetic/tst.cast-exp.r
> @@ -0,0 +1,43 @@
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 4294967294 254 65534 4294967294 4294967294
> +-2 -2 65534 65534 254 65534 65534 65534
> +-2 254 254 254 254 254 254 254
> +2 2 2 2 2 2 2 2
> +85 85 85 85 85 85 85 85
> +85 21845 21845 21845 85 21845 21845 21845
> +85 21845 1431655765 1431655765 85 21845 1431655765 1431655765
> +85 21845 1431655765 6148914691236517205 85 21845 1431655765 6148914691236517205
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +2 2 2 2 2 2 2 2
> +85 85 85 85 85 85 85 85
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +2 2 2 2 2 2 2 2
> +85 21845 21845 21845 85 21845 21845 21845
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +2 2 2 2 2 2 2 2
> +85 21845 1431655765 1431655765 85 21845 1431655765 1431655765
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +2 2 2 2 2 2 2 2
> +85 21845 1431655765 6148914691236517205 85 21845 1431655765 6148914691236517205
> +-2 254 254 254 254 254 254 254
> +-2 254 254 254 254 254 254 254
> +2 2 2 2 2 2 2 2
> +85 85 85 85 85 85 85 85
> +-2 -2 65534 65534 254 65534 65534 65534
> +-2 -2 65534 65534 254 65534 65534 65534
> +2 2 2 2 2 2 2 2
> +85 21845 21845 21845 85 21845 21845 21845
> +-2 -2 -2 4294967294 254 65534 4294967294 4294967294
> +-2 -2 -2 4294967294 254 65534 4294967294 4294967294
> +2 2 2 2 2 2 2 2
> +85 21845 1431655765 1431655765 85 21845 1431655765 1431655765
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +-2 -2 -2 -2 254 65534 4294967294 18446744073709551614
> +2 2 2 2 2 2 2 2
> +85 21845 1431655765 6148914691236517205 85 21845 1431655765 6148914691236517205
> +
> --
> 2.18.4
>
>
> _______________________________________________
> 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