[DTrace-devel] [PATCH] cg: correct bitfield offset determination

Kris Van Hees kris.van.hees at oracle.com
Fri Jul 28 04:57:52 UTC 2023


The C compiler (together with binutils) can represent bitfields using
one of two representations: store the actual offset of the bitfield in
ctm_offset, or store the base offset of the underlying type in ctm_offset
and store the offset within the underlying type as cte_offset.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_cg.c                             | 80 ++++++++++++-------
 test/unittest/bitfields/tst.bitfield-offset.d | 33 ++++++++
 2 files changed, 84 insertions(+), 29 deletions(-)
 create mode 100644 test/unittest/bitfields/tst.bitfield-offset.d

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 7b65447d..51444a38 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -2858,19 +2858,39 @@ dt_cg_ptrsize(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
  */
 static void
 dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
-    ctf_file_t *fp, const ctf_membinfo_t *mp)
+		ctf_file_t *fp, const ctf_membinfo_t *mp)
 {
-	ctf_encoding_t e;
-	uint64_t shift;
-	int r1;
+	ctf_encoding_t	e;
+	uint64_t	shift;
+	ssize_t		size;
+	size_t		offset;
+	uint_t		op;
+	int		reg;
 
 	if (ctf_type_encoding(fp, mp->ctm_type, &e) != 0 || e.cte_bits > 64) {
-		xyerror(D_UNKNOWN, "cg: bad field: off %lu type <%ld> "
-		    "bits %u\n", mp->ctm_offset, mp->ctm_type, e.cte_bits);
+		xyerror(D_UNKNOWN, "cg: bad field: member off %lu type <%ld> "
+		    "oofset %u bits %u\n", mp->ctm_offset, mp->ctm_type,
+		    e.cte_offset, e.cte_bits);
 	}
 
 	assert(dnp->dn_op == DT_TOK_PTR || dnp->dn_op == DT_TOK_DOT);
-	r1 = dnp->dn_left->dn_reg;
+
+	offset = mp->ctm_offset + e.cte_offset;
+
+	/* Advance to the byte where the bitfield starts (if needed). */
+	if (offset >= NBBY)
+		emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_left->dn_reg, offset / NBBY));
+
+	/* If this is a REF, we are done. */
+	if (dnp->dn_flags & DT_NF_REF)
+		return;
+
+	op = dt_cg_ldsize(dnp, fp, mp->ctm_type, &size);
+	reg = dnp->dn_left->dn_reg;
+	if (dnp->dn_left->dn_flags & (DT_NF_ALLOCA | DT_NF_DPTR))
+		emit(dlp, BPF_LOAD(op, reg, reg, 0));
+	else
+		dt_cg_load_scalar(dnp->dn_left, op, size, dlp, drp);
 
 	/*
 	 * On little-endian architectures, ctm_offset counts from the right so
@@ -2889,12 +2909,13 @@ dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 		 */
 #ifdef _BIG_ENDIAN
 		shift = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY) * NBBY -
-		    mp->ctm_offset % NBBY;
+			offset % NBBY;
 #else
-		shift = mp->ctm_offset % NBBY + e.cte_bits;
+		shift = offset % NBBY + e.cte_bits;
 #endif
-		emit(dlp, BPF_ALU64_IMM(BPF_LSH, r1, 64 - shift));
-		emit(dlp, BPF_ALU64_IMM(BPF_ARSH, r1, 64 - e.cte_bits));
+		if (shift < 64)
+			emit(dlp, BPF_ALU64_IMM(BPF_LSH, reg, 64 - shift));
+		emit(dlp, BPF_ALU64_IMM(BPF_ARSH, reg, 64 - e.cte_bits));
 	} else {
 		/*
 		 * r1 >>= shift
@@ -2902,12 +2923,13 @@ dt_cg_field_get(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 		 */
 #ifdef _BIG_ENDIAN
 		shift = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY) * NBBY -
-		    (mp->ctm_offset % NBBY + e.cte_bits);
+			(offset % NBBY + e.cte_bits);
 #else
-		shift = mp->ctm_offset % NBBY;
+		shift = offset % NBBY;
 #endif
-		emit(dlp, BPF_ALU64_IMM(BPF_RSH, r1, shift));
-		emit(dlp, BPF_ALU64_IMM(BPF_AND, r1, (1ULL << e.cte_bits) - 1));
+		if (shift)
+			emit(dlp, BPF_ALU64_IMM(BPF_RSH, reg, shift));
+		emit(dlp, BPF_ALU64_IMM(BPF_AND, reg, (1ULL << e.cte_bits) - 1));
 	}
 }
 
@@ -2926,6 +2948,7 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
 {
 	uint64_t cmask, fmask, shift;
 	int r1, r2;
+	size_t offset;
 
 	ctf_membinfo_t m;
 	ctf_encoding_t e;
@@ -2954,6 +2977,8 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
 		    "bits %u\n", m.ctm_offset, m.ctm_type, e.cte_bits);
 	}
 
+	offset = m.ctm_offset + e.cte_offset;
+
 	if ((r1 = dt_regset_alloc(drp)) == -1 ||
 	    (r2 = dt_regset_alloc(drp)) == -1)
 		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
@@ -2968,9 +2993,9 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
 	 */
 #ifdef _BIG_ENDIAN
 	shift = clp2(P2ROUNDUP(e.cte_bits, NBBY) / NBBY) * NBBY -
-	    (m.ctm_offset % NBBY + e.cte_bits);
+		(offset % NBBY + e.cte_bits);
 #else
-	shift = m.ctm_offset % NBBY;
+	shift = offset % NBBY;
 #endif
 	fmask = (1ULL << e.cte_bits) - 1;
 	cmask = ~(fmask << shift);
@@ -2989,7 +3014,8 @@ dt_cg_field_set(dt_node_t *src, dt_irlist_t *dlp,
 	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));
+	if (shift > 0)
+		emit(dlp, BPF_ALU64_IMM(BPF_LSH, r2, shift));
 	emit(dlp, BPF_ALU64_REG(BPF_OR, r1, r2));
 	dt_regset_free(drp, r2);
 
@@ -6389,6 +6415,7 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 		}
 
 		dt_cg_check_notnull(dlp, drp, dnp->dn_left->dn_reg);
+		dnp->dn_reg = dnp->dn_left->dn_reg;
 
 		ctfp = dnp->dn_left->dn_ctfp;
 		type = ctf_type_resolve(ctfp, dnp->dn_left->dn_type);
@@ -6404,15 +6431,14 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 			longjmp(yypcb->pcb_jmpbuf, EDT_CTF);
 		}
 
-		if (m.ctm_offset != 0) {
-			/*
-			 * If the offset is not aligned on a byte boundary, it
-			 * is a bit-field member and we will extract the value
-			 * bits below after we generate the appropriate load.
-			 */
-			emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_left->dn_reg, m.ctm_offset / NBBY));
+		if (dnp->dn_flags & DT_NF_BITFIELD) {
+			dt_cg_field_get(dnp, dlp, drp, ctfp, &m);
+			break;
 		}
 
+		if (m.ctm_offset != 0)
+			emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_left->dn_reg, m.ctm_offset / NBBY));
+
 		if (!(dnp->dn_flags & DT_NF_REF)) {
 			uint_t	op;
 			ssize_t	size;
@@ -6423,12 +6449,8 @@ dt_cg_node(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 				emit(dlp, BPF_LOAD(op, dnp->dn_left->dn_reg, dnp->dn_left->dn_reg, 0));
 			else
 				dt_cg_load_scalar(dnp->dn_left, op, size, dlp, drp);
-
-			if (dnp->dn_flags & DT_NF_BITFIELD)
-				dt_cg_field_get(dnp, dlp, drp, ctfp, &m);
 		}
 
-		dnp->dn_reg = dnp->dn_left->dn_reg;
 		break;
 	}
 
diff --git a/test/unittest/bitfields/tst.bitfield-offset.d b/test/unittest/bitfields/tst.bitfield-offset.d
new file mode 100644
index 00000000..fa8c568b
--- /dev/null
+++ b/test/unittest/bitfields/tst.bitfield-offset.d
@@ -0,0 +1,33 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2023, 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.
+ */
+
+/*
+ * Verify that DTrace is correctly determining the offset of a bitfield.  It
+ * used to only look at ctm_offset to know the location of the bitfield in the
+ * parent type, but some compiler/libctf combinations use ctm_offset for the
+ * offset of the underlying type and depend on cte_offset instead to provide
+ * the offset within that underlying type.
+ */
+#pragma D option quiet
+
+struct iphdr	iph;
+
+BEGIN
+{
+	iph.ihl = 5;
+	iph.version = 4;
+
+	trace(iph.ihl);
+	trace(iph.version);
+
+	exit(iph.ihl == 5 && iph.version == 4 ? 0 : 1);
+}
+
+ERROR
+{
+	exit(1);
+}
-- 
2.39.3




More information about the DTrace-devel mailing list