[DTrace-devel] [PATCH] Avoid checkng for NULL pointers multiple times

[Kris Van Hees]

The dt_cg_load_var() function included a check to see that a loaded
pointer value was not NULL (and to generate a probe error if it was).
However, callers of this function are already checking for NULL before
using the pointer (if such a check is necessary).  Therefore, this
does not need to be done in dt_cg_load_var().

One case that was affected by this extra NULL-checking is pointer
arithmetic.  An explicit dt_cg_check_notnull() call added to that to
ensure that the source pointer is not NULL.

The dt_cg_load_var() function was also creting two labels that were
only actually used in one branch.  In addition, Eugene Loh pointed
out that a MOV instruction could be saved as well.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_cg.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 296fcd42..716d7bed 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -2066,8 +2066,6 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
 	/* thread-local variables */
 	if (idp->di_flags & DT_IDFLG_TLS) {	/* TLS var */
 		uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;
-		uint_t	lbl_notnull = dt_irlist_label(dlp);
-		uint_t	lbl_done = dt_irlist_label(dlp);
 
 		idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
 		assert(idp != NULL);
@@ -2087,9 +2085,10 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
 		if (dst->dn_flags & DT_NF_REF) {
 			emit(dlp,  BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
 			dt_regset_free(drp, BPF_REG_0);
-			dt_cg_check_notnull(dlp, drp, dst->dn_reg);
 		} else {
 			size_t	size = dt_node_type_size(dst);
+			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);
@@ -2098,11 +2097,9 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
 			emit(dlp,  BPF_MOV_IMM(dst->dn_reg, 0));
 			emit(dlp,  BPF_JUMP(lbl_done));
 			emitl(dlp, lbl_notnull,
-				   BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
+				   BPF_LOAD(ldstw[size], dst->dn_reg, BPF_REG_0, 0));
 			dt_regset_free(drp, BPF_REG_0);
 
-			emit(dlp, BPF_LOAD(ldstw[size], dst->dn_reg, dst->dn_reg, 0));
-
 			emitl(dlp, lbl_done,
 				   BPF_NOP());
 		}
@@ -2569,8 +2566,14 @@ dt_cg_arithmetic_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
 		dt_cg_ptrsize(dnp, dlp, drp, BPF_MUL, dnp->dn_left->dn_reg);
 
 	dt_cg_node(dnp->dn_right, dlp, drp);
-	if (is_ptr_op && lp_is_ptr)
-		dt_cg_ptrsize(dnp, dlp, drp, BPF_MUL, dnp->dn_right->dn_reg);
+	if (is_ptr_op) {
+		if (lp_is_ptr) {
+			dt_cg_ptrsize(dnp, dlp, drp, BPF_MUL,
+				      dnp->dn_right->dn_reg);
+			dt_cg_check_notnull(dlp, drp, dnp->dn_left->dn_reg);
+		} else
+			dt_cg_check_notnull(dlp, drp, dnp->dn_right->dn_reg);
+	}
 
 	if ((op == BPF_MOD || op == BPF_DIV) &&
 	    (dnp->dn_flags & DT_NF_SIGNED)) {
-- 
2.34.1