[DTrace-devel] [PATCH 3/3] String args to dt_strcmp() need to be copied if not DPTR

[Kris Van Hees]

The dt_strcmp() implementation makes use of indirect load instructions
to retrieve characters from either string argument.  That only works for
DTrace-managed storage pointers.  So, any argument that is not marked
DT_NF_DPTR must be copied into a tstring before the indirect loads are
done.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 bpf/strcmp.S      | 48 ++++++++++++++++++++++++++++++++++++-----------
 libdtrace/dt_cg.c | 11 +++++++++++
 2 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/bpf/strcmp.S b/bpf/strcmp.S
index 75177469..bdfbb169 100644
--- a/bpf/strcmp.S
+++ b/bpf/strcmp.S
@@ -58,7 +58,7 @@ dt_strcmp_not :
 	.size	dt_strcmp_not, .-dt_strcmp_not
 
 /*
- * int dt_strcmp(char *s, char *t, char *tmp1, char *tmp2) {
+ * int dt_strcmp(char *s, char *t, char *tmp1, char *tmp2, uint64_t flags) {
  *
  *     [%fp-8]=s
  *     [%fp-16]=t
@@ -86,6 +86,8 @@ dt_strcmp_not :
  *     // based on this location, judge if the strings are >, <, or ==
  *     if (r0 > r6) goto Lsame;
  *     if (r0 > r7) goto Lsame;
+ *     if (flags & 1) { bpf_probe_read_str(tmp1, STRSZ, s); s = tmp1; }
+ *     if (flags & 2) { bpf_probe_read_str(tmp2, STRSZ, t); t = tmp2; }
  *     if (s[r0] > t[r0]) return +1;
  *     if (s[r0] < t[r0]) return +1;
  *
@@ -105,6 +107,7 @@ dt_strcmp :
 	stxdw	[%fp+-16], %r2		/* Spill t */
 	stxdw	[%fp+-24], %r3		/* Spill tmp1 */
 	stxdw	[%fp+-32], %r4		/* Spill tmp2 */
+	mov	%r9, %r5		/* flags */
 
 	lddw	%r8, STRSZ		/* r8 = STRSZ */
 
@@ -157,21 +160,44 @@ dt_strcmp :
 
 	jgt	%r0, %r6, .Lsame	/* if (r0 > r6) goto Lsame */
 	jgt	%r0, %r8, .Lsame	/* if (r0 > r8) goto Lsame */
+	mov	%r8, %r0
 
-	ldxdw	%r4, [%fp+-8]
-	add	%r4, %r0
-	ldxb	%r4, [%r4+0]		/* s[r0] */
-	and	%r4, 0xff
+	jset	%r9, 1, .Ls_is_dptr
+	ldxdw	%r6, [%fp+-24]		/* s = tmp1 */
 
-	ldxdw	%r5, [%fp+-16]
-	add	%r5, %r0
-	ldxb	%r5, [%r5+0]		/* t[r0] */
-	and	%r5, 0xff
+	mov	%r1, %r6
+	lddw	%r2, STRSZ
+	ldxdw	%r3, [%fp+-8]
+	call	BPF_FUNC_probe_read_str	/* bpf_probe_read_str(tmp1, r8, s) */
+	ja	.Lis_t_dptr
+.Ls_is_dptr:
+	ldxdw	%r6, [%fp+-8]		/* Load s */
+
+.Lis_t_dptr:
+	jset	%r9, 2, .Lt_is_dptr
+	ldxdw	%r7, [%fp+-32]		/* t = tmp2 */
+
+	mov	%r1, %r7
+	lddw	%r2, STRSZ
+	ldxdw	%r3, [%fp+-16]
+	call	BPF_FUNC_probe_read_str	/* bpf_probe_read_str(tmp2, r8, t) */
+	ja	.Lflags_done
+.Lt_is_dptr:
+	ldxdw	%r7, [%fp+-16]		/* Load t */
+.Lflags_done:
+
+	add	%r6, %r8
+	ldxb	%r6, [%r6+0]		/* s[r8] */
+	and	%r6, 0xff
+
+	add	%r7, %r8
+	ldxb	%r7, [%r7+0]		/* t[r8] */
+	and	%r7, 0xff
 
-	jle	%r4, %r5, 2		/* if (s[r0] > t[r0]) return +1 */
+	jle	%r6, %r7, 2		/* if (s[r8] > t[r8]) return +1 */
 	mov	%r0, 1
 	exit
-	jge	%r4, %r5, 2		/* if (s[r0] < t[r0]) return +1 */
+	jge	%r6, %r7, 2		/* if (s[r8] < t[r8]) return +1 */
 	mov	%r0, -1
 	exit
 
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index c4e49129..62456268 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -3294,12 +3294,22 @@ dt_cg_compare_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op)
 	    dt_node_is_string(dnp->dn_right)) {
 		dt_ident_t *idp;
 		uint_t Lcomp = dt_irlist_label(dlp);
+		uint32_t flags = 0;
 		uint64_t off1, off2;
 
 		/* if either pointer is NULL, just compare pointers */
 		emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_left->dn_reg, 0, Lcomp));
 		emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_right->dn_reg, 0, Lcomp));
 
+		/*
+		 * Set flags to indicate which arguments (if any) are pointers
+		 * to DTrace-managed storage.
+		 */
+		if (dnp->dn_left->dn_flags & DT_NF_DPTR)
+			flags |= 1;
+		if (dnp->dn_right->dn_flags & DT_NF_DPTR)
+			flags |= 2;
+
 		/*
 		 * Otherwise, replace pointers with relative string values.
 		 * Specifically, replace left with strcmp(left,right)+1 and
@@ -3318,6 +3328,7 @@ dt_cg_compare_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op)
 		emit(dlp,  BPF_MOV_REG(BPF_REG_4, BPF_REG_3));
 		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, off1));
 		emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, off2));
+		emit(dlp,  BPF_MOV_IMM(BPF_REG_5, flags));
 		idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strcmp");
 		assert(idp != NULL);
 		dt_regset_xalloc(drp, BPF_REG_0);
-- 
2.37.2