[DTrace-devel] [PATCH 2/3] Combine assertion with conversion of size to BPF width

eugene.loh at oracle.com eugene.loh at oracle.com
Tue Aug 22 21:46:55 UTC 2023


From: Eugene Loh <eugene.loh at oracle.com>

In dt_cg.c, we often convert a size 1, 2, 4, or 8 into a BPF width
BPF_B, BPF_H, BPF_W, or BPF_DW, respectively.  Each time, we first
check that the size satisfies
    assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
and then we look up ldstw[size].

Combine the assertion and the conversion to simplify the multiple
call sites.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 libdtrace/dt_cg.c | 83 +++++++++++++++--------------------------------
 1 file changed, 26 insertions(+), 57 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 0a7149b3..ea3745bc 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -1267,11 +1267,16 @@ dt_cg_check_ptr_arg(dt_irlist_t *dlp, dt_regset_t *drp, dt_node_t *dnp,
 		dt_cg_check_notnull(dlp, drp, dnp->dn_reg);
 }
 
-static const uint_t	ldstw[] = {
-					0,
-					BPF_B,	BPF_H,	0, BPF_W,
-					0,	0,	0, BPF_DW,
-				};
+static int BPF_width(size_t sz)
+{
+	switch(sz) {
+	case 1: return BPF_B;
+	case 2: return BPF_H;
+	case 4: return BPF_W;
+	case 8: return BPF_DW;
+	default: assert(0);
+	}
+}
 
 /*
  * When loading bit-fields, we want to convert a byte count in the range
@@ -1364,9 +1369,7 @@ dt_cg_store_val(dt_pcb_t *pcb, dt_node_t *dnp, dtrace_actkind_t kind,
 		off = dt_rec_add(dtp, dt_cg_fill_gap, kind, size, size, pfp,
 				 arg);
 
-		assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
-
-		emit(dlp, BPF_STORE(ldstw[size], BPF_REG_9, off, dnp->dn_reg));
+		emit(dlp, BPF_STORE(BPF_width(size), BPF_REG_9, off, dnp->dn_reg));
 		dt_regset_free(drp, dnp->dn_reg);
 
 		goto ok;
@@ -2558,15 +2561,10 @@ dt_cg_ldsize(dt_node_t *dnp, ctf_file_t *ctfp, ctf_id_t type, ssize_t *ret_size)
 	else
 		size = ctf_type_size(ctfp, type);
 
-	if (size < 1 || size > 8 || (size & (size - 1)) != 0) {
-		xyerror(D_UNKNOWN, "internal error -- cg cannot load "
-		    "size %ld when passed by value\n", (long)size);
-	}
-
 	if (ret_size)
 		*ret_size = size;
 
-	return ldstw[size];
+	return BPF_width(size);
 }
 
 /*
@@ -2714,14 +2712,11 @@ dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 			uint_t	lbl_notnull = dt_irlist_label(dlp);
 			uint_t	lbl_done = dt_irlist_label(dlp);
 
-			assert(size > 0 && size <= 8 &&
-			       (size & (size - 1)) == 0);
-
 			emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, 0, lbl_notnull));
 			emit(dlp,  BPF_MOV_IMM(dnp->dn_reg, 0));
 			emit(dlp,  BPF_JUMP(lbl_done));
 			emitl(dlp, lbl_notnull,
-				   BPF_LOAD(ldstw[size], dnp->dn_reg, BPF_REG_0, 0));
+				   BPF_LOAD(BPF_width(size), dnp->dn_reg, BPF_REG_0, 0));
 			dt_cg_promote(dnp, size, dlp, drp);
 
 			emitl(dlp, lbl_done,
@@ -2758,12 +2753,10 @@ dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 			uint_t	L2 = dt_irlist_label(dlp);
 
 			assert(dnp->dn_flags & DT_NF_REF);
-			assert(size > 0 && size <= 8 &&
-			       (size & (size - 1)) == 0);
 
 			if ((reg = dt_regset_alloc(drp)) == -1)
 				longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
-			emit(dlp,  BPF_LOAD(ldstw[size], reg, dnp->dn_reg, idp->di_offset));
+			emit(dlp,  BPF_LOAD(BPF_width(size), reg, dnp->dn_reg, idp->di_offset));
 			emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, reg, DT_NULL_STRING, L1));
 			emit(dlp,  BPF_MOV_IMM(dnp->dn_reg, 0));
 			emit(dlp,  BPF_JUMP(L2));
@@ -2778,10 +2771,7 @@ dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
 		} else {
 			size_t	size = dt_node_type_size(dnp);
 
-			assert(size > 0 && size <= 8 &&
-			       (size & (size - 1)) == 0);
-
-			emit(dlp, BPF_LOAD(ldstw[size], dnp->dn_reg, dnp->dn_reg, idp->di_offset));
+			emit(dlp, BPF_LOAD(BPF_width(size), dnp->dn_reg, dnp->dn_reg, idp->di_offset));
 			dt_cg_promote(dnp, size, dlp, drp);
 		}
 
@@ -3029,9 +3019,7 @@ dt_cg_store(dt_node_t *src, dt_irlist_t *dlp, dt_regset_t *drp, dt_node_t *dst)
 		else
 			sreg = src->dn_reg;
 
-		assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
-
-		emit(dlp, BPF_STORE(ldstw[size], dreg, 0, sreg));
+		emit(dlp, BPF_STORE(BPF_width(size), dreg, 0, sreg));
 
 		if (sreg != src->dn_reg)
 			dt_regset_free(drp, sreg);
@@ -3211,14 +3199,11 @@ empty_args:
 		size = dt_node_sizeof(&isp->dis_args[i]);
 
 		if (dt_node_is_scalar(dnp) || dt_node_is_float(dnp)) {
-			assert(size > 0 && size <= 8 &&
-			       (size & (size - 1)) == 0);
-
 			nextoff = (tuplesize + (size - 1)) & ~(size - 1);
 			if (tuplesize < nextoff)
 				emit(dlp,  BPF_ALU64_IMM(BPF_ADD, treg, nextoff - tuplesize));
 
-			emit(dlp,  BPF_STORE(ldstw[size], treg, 0, BPF_REG_0));
+			emit(dlp,  BPF_STORE(BPF_width(size), treg, 0, BPF_REG_0));
 			dt_regset_free(drp, BPF_REG_0);
 
 			emit(dlp,  BPF_ALU64_IMM(BPF_ADD, treg, size));
@@ -3381,9 +3366,7 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 
 			dt_cg_memcpy(dlp, drp, reg, dnp->dn_reg, size);
 		} else {
-			assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
-
-			emit(dlp, BPF_STORE(ldstw[size], reg, 0, dnp->dn_reg));
+			emit(dlp, BPF_STORE(BPF_width(size), reg, 0, dnp->dn_reg));
 		}
 
 		dt_regset_free(drp, reg);
@@ -3425,11 +3408,9 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 		assert(idp->di_size == dt_node_type_size(dnp->dn_left));
 		assert(dt_node_is_integer(dnp->dn_right) == (dnp->dn_right->dn_kind == DT_NODE_INT));
 		assert(dnp->dn_right->dn_value == 0);
-		assert(size > 0 && size <= 8 &&
-		       (size & (size - 1)) == 0);
 
 		emit(dlp, BPF_ALU64_IMM(BPF_ADD, reg, idp->di_offset));
-		emit(dlp, BPF_STORE_IMM(ldstw[size], reg, 0, DT_NULL_STRING));
+		emit(dlp, BPF_STORE_IMM(BPF_width(size), reg, 0, DT_NULL_STRING));
 
 		/*
 		 * Since strings are passed by value, we need to force
@@ -3461,10 +3442,8 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 
 		/* Start by zeroing out the first bytes */
 		size = sizeof(DT_NULL_STRING);
-		assert(size > 0 && size <= 8 &&
-		       (size & (size - 1)) == 0);
 
-		emit(dlp, BPF_STORE_IMM(ldstw[size], reg, 0, 0));
+		emit(dlp, BPF_STORE_IMM(BPF_width(size), reg, 0, 0));
 
 		/*
 		 * Determine the amount of data to be copied.  It is
@@ -3485,8 +3464,7 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 			emitl(dlp, Lnull,
 				   BPF_NOP());
 			size = sizeof(DT_NULL_STRING);
-			assert(size > 0 && size <= 8 && (size & (size - 1)) == 0);
-			emit(dlp, BPF_STORE_IMM(ldstw[size], reg, 0, DT_NULL_STRING));
+			emit(dlp, BPF_STORE_IMM(BPF_width(size), reg, 0, DT_NULL_STRING));
 			emitl(dlp, Ldone,
 				   BPF_NOP());
 		}
@@ -3521,10 +3499,7 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 		/* Store by value */
 		size = idp->di_size;
 
-		assert(size > 0 && size <= 8 &&
-		       (size & (size - 1)) == 0);
-
-		emit(dlp, BPF_STORE(ldstw[size], reg, idp->di_offset, dnp->dn_reg));
+		emit(dlp, BPF_STORE(BPF_width(size), reg, idp->di_offset, dnp->dn_reg));
 		dt_regset_free(drp, reg);
 	}
 
@@ -3671,9 +3646,6 @@ dt_cg_incdec_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op,
 
 	TRACE_REGSET("    incdec: Begin");
 
-	assert(size > 0 && size <= 8 &&
-	       (size & (size - 1)) == 0);
-
 	if (dt_node_is_pointer(dnp)) {
 		ctf_file_t	*ctfp = dnp->dn_ctfp;
 		ctf_id_t	type;
@@ -3737,16 +3709,16 @@ dt_cg_incdec_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op,
 			   BPF_NOP());
 	}
 
-	emit(dlp,  BPF_LOAD(ldstw[size], dnp->dn_reg, dnp->dn_child->dn_reg, 0));
+	emit(dlp,  BPF_LOAD(BPF_width(size), dnp->dn_reg, dnp->dn_child->dn_reg, 0));
 	if (post) {
 		dt_regset_xalloc(drp, BPF_REG_0);
 		emit(dlp,  BPF_MOV_REG(BPF_REG_0, dnp->dn_reg));
 		emit(dlp,  BPF_ALU64_IMM(op, BPF_REG_0, adj));
-		emit(dlp,  BPF_STORE(ldstw[size], dnp->dn_child->dn_reg, 0, BPF_REG_0));
+		emit(dlp,  BPF_STORE(BPF_width(size), dnp->dn_child->dn_reg, 0, BPF_REG_0));
 		dt_regset_free(drp, BPF_REG_0);
 	} else {
 		emit(dlp,  BPF_ALU64_IMM(op, dnp->dn_reg, adj));
-		emit(dlp,  BPF_STORE(ldstw[size], dnp->dn_child->dn_reg, 0, dnp->dn_reg));
+		emit(dlp,  BPF_STORE(BPF_width(size), dnp->dn_child->dn_reg, 0, dnp->dn_reg));
 	}
 
 	dt_regset_free(drp, dnp->dn_child->dn_reg);
@@ -4261,9 +4233,6 @@ dt_cg_uregs(unsigned int idx, dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp
 			/* Add the thread->member offset. */
 			offset += dt_cg_ctf_offsetof("struct thread_struct",
 						     memnames[idx - 21], &size);
-			if (size < 1 || size > 8 || (size & (size - 1)) != 0)
-				xyerror(D_UNKNOWN, "internal error -- cg cannot load "
-				    "size %ld when passed by value\n", (long)size);
 
 			/* Get task. */
 			if (dt_regset_xalloc_args(drp) == -1)
@@ -4278,7 +4247,7 @@ dt_cg_uregs(unsigned int idx, dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp
 			emit(dlp, BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, offset));
 
 			/* Dereference it safely (the BPF verifier has no idea what it is). */
-			dt_cg_load_scalar(dnp, ldstw[size], size, dlp, drp);
+			dt_cg_load_scalar(dnp, BPF_width(size), size, dlp, drp);
 
 			return;
 		}
-- 
2.18.4




More information about the DTrace-devel mailing list