[DTrace-devel] [PATCH 4/6] Fix bit-field operations
eugene.loh at oracle.com
eugene.loh at oracle.com
Fri Mar 19 10:45:32 PDT 2021
From: Eugene Loh <eugene.loh at oracle.com>
Fix bugs in the dt_cg_field_[set|get] functions.
Add tests for the functionality. Note that, due to an earlier patch
("6d9c2398 Provide a more consistent implementation of the trace() action"),
results for signed bit fields with a nonzero sign bit differ from the
incorrect results of DTv1. Current results match GCC.
(Also note that bit fields are not packed together as efficiently as
possible. Rather, following legacy DTrace behavior, each bit field is
aligned to the size of the next largest integer type. Thus, simplifications
in BPF code generation can be imagined. Or, bit fields should be packed more
tightly together, in a departure from earlier DTrace implementations but
bringing behavior in line with common user expectations.)
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_cg.c | 65 ++++++++-----------
test/unittest/bitfields/tst.BasicSigned.d | 54 +++++++++++++++
test/unittest/bitfields/tst.BasicSigned.r | 3 +
test/unittest/bitfields/tst.BasicUnsigned.d | 54 +++++++++++++++
test/unittest/bitfields/tst.BasicUnsigned.r | 3 +
.../bitfields/tst.BitFieldPromotion.d | 3 +-
6 files changed, 143 insertions(+), 39 deletions(-)
create mode 100644 test/unittest/bitfields/tst.BasicSigned.d
create mode 100644 test/unittest/bitfields/tst.BasicSigned.r
create mode 100644 test/unittest/bitfields/tst.BasicUnsigned.d
create mode 100644 test/unittest/bitfields/tst.BasicUnsigned.r
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index da3e9ca6..27b32c87 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -1698,20 +1698,7 @@ dt_cg_ptrsize(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
/*
* If the result of a "." or "->" operation is a bit-field, we use this routine
- * to generate an epilogue to the load instruction that extracts the value. In
- * the diagrams below the "ld??" is the load instruction that is generated to
- * load the containing word that is generating prior to calling this function.
- *
- * Epilogue for unsigned fields: Epilogue for signed fields:
- *
- * ldu? [r1], r1 lds? [r1], r1
- * setx USHIFT, r2 setx 64 - SSHIFT, r2
- * srl r1, r2, r1 sll r1, r2, r1
- * setx (1 << bits) - 1, r2 setx 64 - bits, r2
- * and r1, r2, r1 sra r1, r2, r1
- *
- * The *SHIFT constants above changes value depending on the endian-ness of our
- * target architecture. Refer to the comments below for more details.
+ * to generate an epilogue to the load instruction that extracts the value.
*/
static void
dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
@@ -1737,10 +1724,13 @@ dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
* must subtract (ctm_offset % NBBY + cte_bits) from the size in bits
* we used for the load. The size of our load in turn is found by
* rounding cte_bits up to a byte boundary and then finding the
- * nearest power of two to this value (see clp2(), above). These
- * properties are used to compute shift as USHIFT or SSHIFT, below.
+ * nearest power of two to this value (see clp2(), above).
*/
if (dnp->dn_flags & DT_NF_SIGNED) {
+ /*
+ * r1 <<= 64 - shift
+ * r1 >>= 64 - bits
+ */
#ifdef _BIG_ENDIAN
shift = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY) * NBBY -
mp->ctm_offset % NBBY;
@@ -1750,35 +1740,26 @@ dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
emit(dlp, BPF_ALU64_IMM(BPF_LSH, r1, 64 - shift));
emit(dlp, BPF_ALU64_IMM(BPF_ARSH, r1, 64 - e.cte_bits));
} else {
+ /*
+ * r1 >>= shift
+ * r1 &= (1 << bits) - 1
+ */
#ifdef _BIG_ENDIAN
shift = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY) * NBBY -
(mp->ctm_offset % NBBY + e.cte_bits);
#else
shift = mp->ctm_offset % NBBY;
#endif
- emit(dlp, BPF_ALU64_IMM(BPF_LSH, r1, shift));
+ emit(dlp, BPF_ALU64_IMM(BPF_RSH, r1, shift));
emit(dlp, BPF_ALU64_IMM(BPF_AND, r1, (1ULL << e.cte_bits) - 1));
}
}
/*
- * If the destination of a store operation is a bit-field, we use this routine
- * to generate a prologue to the store instruction that loads the surrounding
- * bits, clears the destination field, and ORs in the new value of the field.
- * In the diagram below the "st?" is the store instruction that is generated to
- * store the containing word that is generating after calling this function.
- *
- * ld [dst->dn_reg], r1
- * setx ~(((1 << cte_bits) - 1) << (ctm_offset % NBBY)), r2
- * and r1, r2, r1
- *
- * setx (1 << cte_bits) - 1, r2
- * and src->dn_reg, r2, r2
- * setx ctm_offset % NBBY, r3
- * sll r2, r3, r2
- *
- * or r1, r2, r1
- * st? r1, [dst->dn_reg]
+ * If the destination of a store operation is a bit-field, we first use this
+ * routine to load the surrounding bits, clear the destination field, and OR
+ * in the new value of the field. After this routine, we will store the
+ * generated word.
*
* This routine allocates a new register to hold the value to be stored and
* returns it. The caller is responsible for freeing this register later.
@@ -1838,11 +1819,21 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
fmask = (1ULL << e.cte_bits) - 1;
cmask = ~(fmask << shift);
- /* FIXME: Does not handled signed or userland */
+ /*
+ * r1 = [dst->dn_reg]
+ * r2 = cmask
+ * r1 &= r2
+ * r2 = fmask
+ * r2 &= src->dn_reg
+ * r2 <<= shift
+ * r1 |= r2
+ */
+ /* FIXME: Does not handle userland */
emit(dlp, BPF_LOAD(dt_cg_load(dst, fp, m.ctm_type), r1, dst->dn_reg, 0));
- emit(dlp, BPF_ALU64_IMM(BPF_AND, r1, cmask));
- dt_cg_setx(dlp, r2, fmask);
+ dt_cg_setx(dlp, r2, cmask);
emit(dlp, BPF_ALU64_REG(BPF_AND, r1, r2));
+ dt_cg_setx(dlp, r2, fmask);
+ emit(dlp, BPF_ALU64_REG(BPF_AND, r2, src->dn_reg));
emit(dlp, BPF_ALU64_IMM(BPF_LSH, r2, shift));
emit(dlp, BPF_ALU64_REG(BPF_OR, r1, r2));
dt_regset_free(drp, r2);
diff --git a/test/unittest/bitfields/tst.BasicSigned.d b/test/unittest/bitfields/tst.BasicSigned.d
new file mode 100644
index 00000000..00c0441d
--- /dev/null
+++ b/test/unittest/bitfields/tst.BasicSigned.d
@@ -0,0 +1,54 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2021, 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: Bit-field store and load work properly.
+ *
+ * SECTION: Structs and Unions/Bit-Fields
+ */
+
+#pragma D option quiet
+
+struct bitRecord{
+ int a : 3;
+ int b : 7;
+ int c : 14;
+ long d : 35;
+ int e : 22;
+ int f : 32;
+ int g : 16;
+ int h : 8;
+} var;
+
+BEGIN
+{
+ /* sign bit is 0 */
+ var.a = 3;
+ var.b = 0x3f;
+ var.c = 0x1fff;
+ var.d = 0x3ffffffff;
+ var.e = 0x1fffff;
+ var.f = 0x7fffffff;
+ var.g = 0x7fff;
+ var.h = 0x7f;
+ printf("%d %d %d %d %d %d %d %d\n",
+ var.a, var.b, var.c, var.d, var.e, var.f, var.g, var.h);
+
+ /* sign bit is 1 */
+ var.a = 7;
+ var.b = 0x75;
+ var.c = 0x3555;
+ var.d = 0x755555555;
+ var.e = 0x355555;
+ var.f = 0xf5555555;
+ var.g = 0xf555;
+ var.h = 0xf5;
+ printf("%d %d %d %d %d %d %d %d\n",
+ var.a, var.b, var.c, var.d, var.e, var.f, var.g, var.h);
+
+ exit(0);
+}
diff --git a/test/unittest/bitfields/tst.BasicSigned.r b/test/unittest/bitfields/tst.BasicSigned.r
new file mode 100644
index 00000000..074bc249
--- /dev/null
+++ b/test/unittest/bitfields/tst.BasicSigned.r
@@ -0,0 +1,3 @@
+3 63 8191 17179869183 2097151 2147483647 32767 127
+-1 -11 -2731 -2863311531 -699051 -178956971 -2731 -11
+
diff --git a/test/unittest/bitfields/tst.BasicUnsigned.d b/test/unittest/bitfields/tst.BasicUnsigned.d
new file mode 100644
index 00000000..e60d9b1b
--- /dev/null
+++ b/test/unittest/bitfields/tst.BasicUnsigned.d
@@ -0,0 +1,54 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2021, 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: Bit-field store and load work properly.
+ *
+ * SECTION: Structs and Unions/Bit-Fields
+ */
+
+#pragma D option quiet
+
+struct bitRecord{
+ unsigned int a : 3;
+ unsigned int b : 7;
+ unsigned int c : 14;
+ unsigned long d : 35;
+ unsigned int e : 22;
+ unsigned int f : 32;
+ unsigned int g : 16;
+ unsigned int h : 8;
+} var;
+
+BEGIN
+{
+ /* sign bit is 0 */
+ var.a = 3;
+ var.b = 0x3f;
+ var.c = 0x1fff;
+ var.d = 0x3ffffffff;
+ var.e = 0x1fffff;
+ var.f = 0x7fffffff;
+ var.g = 0x7fff;
+ var.h = 0x7f;
+ printf("%d %d %d %d %d %d %d %d\n",
+ var.a, var.b, var.c, var.d, var.e, var.f, var.g, var.h);
+
+ /* sign bit is 1 */
+ var.a = 7;
+ var.b = 0x75;
+ var.c = 0x3555;
+ var.d = 0x755555555;
+ var.e = 0x355555;
+ var.f = 0xf5555555;
+ var.g = 0xf555;
+ var.h = 0xf5;
+ printf("%d %d %d %d %d %d %d %d\n",
+ var.a, var.b, var.c, var.d, var.e, var.f, var.g, var.h);
+
+ exit(0);
+}
diff --git a/test/unittest/bitfields/tst.BasicUnsigned.r b/test/unittest/bitfields/tst.BasicUnsigned.r
new file mode 100644
index 00000000..244be657
--- /dev/null
+++ b/test/unittest/bitfields/tst.BasicUnsigned.r
@@ -0,0 +1,3 @@
+3 63 8191 17179869183 2097151 2147483647 32767 127
+7 117 13653 31496426837 3495253 4116010325 62805 245
+
diff --git a/test/unittest/bitfields/tst.BitFieldPromotion.d b/test/unittest/bitfields/tst.BitFieldPromotion.d
index 535b8f11..56522216 100644
--- a/test/unittest/bitfields/tst.BitFieldPromotion.d
+++ b/test/unittest/bitfields/tst.BitFieldPromotion.d
@@ -1,10 +1,9 @@
/*
* Oracle Linux DTrace.
- * Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2006, 2021, 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.
*/
-/* @@xfail: dtv2 */
/*
* ASSERTION: Bit-field will be automatically promoted to the next largest
--
2.18.4
More information about the DTrace-devel
mailing list