[DTrace-devel] [PATCH v2 1/5] Work in Progress: Add support for string comparisons

eugene.loh at oracle.com eugene.loh at oracle.com
Thu Aug 26 14:29:49 PDT 2021


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

Need to clean up the "signed comparison" issues.  Specifically,
make sure the BPF code is doing the right comparison and clean up
the ugly way dt_cg is handling this now.

Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
 bpf/Build                                     |   1 +
 bpf/strcmp.S                                  | 173 ++++++++++++++++++
 libdtrace/dt_cg.c                             |  51 +++++-
 libdtrace/dt_dlibs.c                          |   1 +
 test/unittest/operators/tst.str_comparison.d  |  73 ++++++++
 test/unittest/operators/tst.str_comparison2.d |  42 +++++
 6 files changed, 337 insertions(+), 4 deletions(-)
 create mode 100644 bpf/strcmp.S
 create mode 100644 test/unittest/operators/tst.str_comparison.d
 create mode 100644 test/unittest/operators/tst.str_comparison2.d

diff --git a/bpf/Build b/bpf/Build
index fe155c3a..287e6f6c 100644
--- a/bpf/Build
+++ b/bpf/Build
@@ -26,6 +26,7 @@ bpf_dlib_SOURCES = \
 	get_bvar.c \
 	get_tvar.c set_tvar.c \
 	probe_error.c \
+	strcmp.S \
 	strnlen.c \
 	substr.S \
 	varint.c
diff --git a/bpf/strcmp.S b/bpf/strcmp.S
new file mode 100644
index 00000000..eb6e09e1
--- /dev/null
+++ b/bpf/strcmp.S
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
+ */
+
+#define BPF_FUNC_probe_read	4
+#define BPF_FUNC_probe_read_str	45
+
+	.text
+
+	.align	4
+	.global	dt_strcmp_xor
+dt_strcmp_xor :
+	mov	%r2, 0
+.Lxor:
+	ldxdw	%r3, [%r1+0]
+	ldxdw	%r4, [%r1+256]
+
+	xor	%r3, %r4
+	stxdw	[%r1+0], %r3
+
+	add	%r1, 8
+	add	%r2, 8
+	jlt	%r2, 256, .Lxor
+	exit
+
+	.align	4
+	.global	dt_strcmp_not
+dt_strcmp_not :
+	mov	%r2, 0
+.Lnot:
+	ldxb	%r3, [%r1+0]
+	add	%r3, 0xff
+	sub	%r3, 1
+	rsh	%r3, 63
+	stxb	[%r1+0], %r3
+	add	%r1, 1
+	add	%r2, 1
+	jlt	%r2, 256, .Lnot
+	exit
+
+/*
+ * int dt_strcmp(char *s, char *t, char *u) {
+ *
+ *     [%fp-8]=s
+ *     [%fp-16]=t
+ *     [%fp-24]=u
+ *
+ *     r6 = dt_vint2int(s);
+ *     r7 = dt_vint_size(r6);
+ *     r8 = dt_vint2int(t);
+ *     r9 = dt_vint_size(r8);
+ *
+ *     bpf_probe_read(u      , r6, s + r7);
+ *     bpf_probe_read(u + 256, r8, t + r9);
+ *     u[r6] = '\0';
+ *     u[r8 + 256] = '\0';
+ *
+ *     dt_stcmp_xor(u);
+ *     dt_stcmp_not(u);
+ *
+ *     r0 = bpf_probe_read_str(u + 256, 256, u);
+ *     r0 -= 1;
+ *     if (r0 > r6) goto Lsame;
+ *     if (r0 > r8) goto Lsame;
+ *     if (s[r7 + r0] > t[r9 + r0]) return +1;
+ *     if (s[r7 + r0] < t[r9 + r0]) return +1;
+ * Lsame:
+ *     if (r6 > r8) return +1
+ *     if (r6 < r8) return -1
+ *     return 0;
+ * }
+ */
+	.align	4
+	.global	dt_strcmp
+dt_strcmp :
+
+	stxdw	[%fp+-8], %r1		/* Spill s */
+	stxdw	[%fp+-16], %r2		/* Spill t */
+	stxdw	[%fp+-24], %r3		/* Spill u */
+
+	call	dt_vint2int		/* r6 = dt_vint2int(s) */
+
+	and	%r0, 0xfffffff		/* bound r6 */
+	lddw	%r1, STRSZ
+	jle	%r0, %r1, 1
+	mov	%r0, %r1
+	mov	%r6, %r0
+
+	mov	%r1, %r6
+	call	dt_vint_size		/* r7 = dt_vint_size(r6) */
+	mov	%r7, %r0
+
+	ldxdw	%r1, [%fp+-16]
+	call	dt_vint2int		/* r8 = dt_vint2int(t) */
+
+	and	%r0, 0xfffffff		/* bound r8 */
+	lddw	%r1, STRSZ
+	jle	%r0, %r1, 1
+	mov	%r0, %r1
+	mov	%r8, %r0
+
+	mov	%r1, %r8
+	call	dt_vint_size		/* r9 = dt_vint_size(r8) */
+	mov	%r9, %r0
+
+	ldxdw	%r1, [%fp+-24]
+	mov	%r2, %r6
+	ldxdw	%r3, [%fp+-8]
+	add	%r3, %r7
+	call	BPF_FUNC_probe_read	/* bpf_probe_read(u      , r6, s + r7) */
+
+	ldxdw	%r1, [%fp+-24]
+	add	%r1, 256
+	mov	%r2, %r8
+	ldxdw	%r3, [%fp+-16]
+	add	%r3, %r9
+	call	BPF_FUNC_probe_read	/* bpf_probe_read(u + 256, r8, t + r9) */
+
+	mov	%r2, 0
+	ldxdw	%r1, [%fp+-24]
+	add	%r1, %r6
+	stxb	[%r1+0], %r2		/* u[r6] = '\0' */
+	ldxdw	%r1, [%fp+-24]
+	add	%r1, %r8
+	stxb	[%r1+256], %r2		/* u[r8 + 256] = '\0' */
+
+
+
+	ldxdw	%r1, [%fp+-24]
+	call dt_strcmp_xor
+	ldxdw	%r1, [%fp+-24]
+	call dt_strcmp_not
+
+
+
+	ldxdw	%r3, [%fp+-24]		/* r0 = bpf_probe_read_str(u + 256, 256, u) */
+	mov	%r1, %r3
+	add	%r1, 256
+	mov	%r2, 256
+	call	BPF_FUNC_probe_read_str
+
+	sub	%r0, 1			/* r0 -= 1 */
+	jgt	%r0, %r6, .Lsame	/* if (r0 > r6) goto Lsame */
+	jgt	%r0, %r8, .Lsame	/* if (r0 > r8) goto Lsame */
+
+	add	%r7, %r0
+	ldxdw	%r4, [%fp+-8]
+	add	%r4, %r7
+	ldxb	%r4, [%r4+0]		/* s[r7 + r0] */
+	and	%r4, 0xff
+
+	add	%r9, %r0
+	ldxdw	%r5, [%fp+-16]
+	add	%r5, %r9
+	ldxb	%r5, [%r5+0]		/* t[r9 + r0] */
+	and	%r5, 0xff
+
+	jle	%r4, %r5, 2		/* if (s[] > t[]) return +1 */
+	mov	%r0, 1
+	exit
+	jge	%r4, %r5, 2		/* if (s[] < t[]) return +1 */
+	mov	%r0, -1
+	exit
+.Lsame:
+	jle	%r6, %r8, 2		/* if (r6 > r8) return +1 */
+	mov	%r0, 1
+	exit
+	jge	%r6, %r8, 2		/* if (r6 < r8) return -1 */
+	mov	%r0, -1
+	exit
+	mov	%r0, 0			/* return 0 */
+	exit
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index a841272c..f627ce61 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -2540,10 +2540,53 @@ dt_cg_compare_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op)
 	dt_cg_node(dnp->dn_left, dlp, drp);
 	dt_cg_node(dnp->dn_right, dlp, drp);
 
-	/* FIXME: No support for string comparison yet */
-	if (dt_node_is_string(dnp->dn_left) || dt_node_is_string(dnp->dn_right))
-		xyerror(D_UNKNOWN, "internal error -- no support for "
-			"string comparison yet\n");
+	/* by now, we have checked and both args are strings or neither is */
+	if (dt_node_is_string(dnp->dn_left) ||
+	    dt_node_is_string(dnp->dn_right)) {
+		dt_ident_t *idp;
+		uint_t Lnull = dt_irlist_label(dlp);
+		uint_t Lcomp = dt_irlist_label(dlp);
+		uint_t L1 = dt_irlist_label(dlp);
+
+		/* if either pointer is NULL, just compare pointers */
+		emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_left->dn_reg, 0, Lnull));
+		emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_right->dn_reg, 0, Lnull));
+
+		/* otherwise, replace pointers with relative string values */
+		if (dt_regset_xalloc_args(drp) == -1)
+			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
+		emit(dlp,  BPF_MOV_REG(BPF_REG_1, dnp->dn_left->dn_reg));
+		emit(dlp,  BPF_MOV_REG(BPF_REG_2, dnp->dn_right->dn_reg));
+		emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_FP, DT_STK_DCTX));
+		emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_3, BPF_REG_3, DCTX_BUF));
+		emit(dlp,  BPF_ALU64_IMM(BPF_SUB, BPF_REG_3, 512 + 8));
+		idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strcmp");
+		assert(idp != NULL);
+		dt_regset_xalloc(drp, BPF_REG_0);
+		emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
+		dt_regset_free_args(drp);
+		emit(dlp,  BPF_MOV_REG(dnp->dn_left->dn_reg, BPF_REG_0));
+		dt_regset_free(drp, BPF_REG_0);
+		emit(dlp,  BPF_MOV_IMM(dnp->dn_right->dn_reg, 0));
+		emit(dlp,  BPF_JUMP(Lcomp));
+
+		/*
+		 * Deal with pointer-NULL comparisons.  We just want to compare an
+		 * unsigned pointer with 0.  Typically, however, string comparison
+		 * uses signed comparison.  So if at least one pointer is NULL,
+		 * just turn any non-NULL pointer into "1".
+		 */
+		emitl(dlp, Lnull,
+		           BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_left->dn_reg, 0, L1));
+		emit (dlp, BPF_MOV_IMM(dnp->dn_left->dn_reg, 1));
+		emitl(dlp, L1,
+		           BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_right->dn_reg, 0, Lcomp));
+		emit (dlp, BPF_MOV_IMM(dnp->dn_right->dn_reg, 1));
+
+		/* proceed with the comparison */
+		emitl(dlp, Lcomp,
+			   BPF_NOP());
+	}
 
 	emit(dlp,  BPF_BRANCH_REG(op, dnp->dn_left->dn_reg, dnp->dn_right->dn_reg, lbl_true));
 	dt_regset_free(drp, dnp->dn_right->dn_reg);
diff --git a/libdtrace/dt_dlibs.c b/libdtrace/dt_dlibs.c
index ac5d5d0d..547fd10f 100644
--- a/libdtrace/dt_dlibs.c
+++ b/libdtrace/dt_dlibs.c
@@ -59,6 +59,7 @@ static const dt_ident_t		dt_bpf_symbols[] = {
 	DT_BPF_SYMBOL(dt_get_string, DT_IDENT_SYMBOL),
 	DT_BPF_SYMBOL(dt_get_tvar, DT_IDENT_SYMBOL),
 	DT_BPF_SYMBOL(dt_set_tvar, DT_IDENT_SYMBOL),
+	DT_BPF_SYMBOL(dt_strcmp, DT_IDENT_SYMBOL),
 	DT_BPF_SYMBOL(dt_strnlen, DT_IDENT_SYMBOL),
 	DT_BPF_SYMBOL(dt_substr, DT_IDENT_SYMBOL),
 	/* BPF maps */
diff --git a/test/unittest/operators/tst.str_comparison.d b/test/unittest/operators/tst.str_comparison.d
new file mode 100644
index 00000000..d91b04b8
--- /dev/null
+++ b/test/unittest/operators/tst.str_comparison.d
@@ -0,0 +1,73 @@
+/*
+ * 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: String comparisons work.
+ *
+ * SECTION:  Operators
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	nerrors = 0;
+
+	s1 = "abcdefghi";
+	s2 = "jklmnopqr";
+	s3 = "stuvwxyz!";
+
+	nerrors += (s1 <= s2 ? 0 : 1);
+	nerrors += (s1 <  s2 ? 0 : 1);
+	nerrors += (s1 == s2 ? 1 : 0);
+	nerrors += (s1 != s2 ? 0 : 1);
+	nerrors += (s1 >= s2 ? 1 : 0);
+	nerrors += (s1 >  s2 ? 1 : 0);
+
+	nerrors += (s2 <= s2 ? 0 : 1);
+	nerrors += (s2 <  s2 ? 1 : 0);
+	nerrors += (s2 == s2 ? 0 : 1);
+	nerrors += (s2 != s2 ? 1 : 0);
+	nerrors += (s2 >= s2 ? 0 : 1);
+	nerrors += (s2 >  s2 ? 1 : 0);
+
+	nerrors += (s3 <= s2 ? 1 : 0);
+	nerrors += (s3 <  s2 ? 1 : 0);
+	nerrors += (s3 == s2 ? 1 : 0);
+	nerrors += (s3 != s2 ? 0 : 1);
+	nerrors += (s3 >= s2 ? 0 : 1);
+	nerrors += (s3 >  s2 ? 0 : 1);
+
+	s2 = NULL;
+	nerrors += (s3 <= s2 ? 1 : 0);
+	nerrors += (s3 <  s2 ? 1 : 0);
+	nerrors += (s3 == s2 ? 1 : 0);
+	nerrors += (s3 != s2 ? 0 : 1);
+	nerrors += (s3 >= s2 ? 0 : 0);
+	nerrors += (s3 >  s2 ? 0 : 0);
+
+	nerrors += (s2 <= s3 ? 0 : 1);
+	nerrors += (s2 <  s3 ? 0 : 1);
+	nerrors += (s2 == s3 ? 1 : 0);
+	nerrors += (s2 != s3 ? 0 : 1);
+	nerrors += (s2 >= s3 ? 1 : 0);
+	nerrors += (s2 >  s3 ? 1 : 0);
+
+	s3 = NULL;
+	nerrors += (s2 <= s3 ? 0 : 1);
+	nerrors += (s2 <  s3 ? 1 : 0);
+	nerrors += (s2 == s3 ? 0 : 1);
+	nerrors += (s2 != s3 ? 1 : 0);
+	nerrors += (s2 >= s3 ? 0 : 1);
+	nerrors += (s2 >  s3 ? 1 : 0);
+
+	exit(nerrors == 0 ? 0 : 1);
+}
+ERROR
+{
+	exit(1);
+}
diff --git a/test/unittest/operators/tst.str_comparison2.d b/test/unittest/operators/tst.str_comparison2.d
new file mode 100644
index 00000000..23fa74fb
--- /dev/null
+++ b/test/unittest/operators/tst.str_comparison2.d
@@ -0,0 +1,42 @@
+/*
+ * 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: String comparisons work.
+ *
+ * SECTION:  Operators
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+	nerrors = 0;
+
+	s1 = "abcdefghi";
+	s2 = "abcdefghijklmnopqrstuvwxyz";
+
+	nerrors += (s1 <= s2 ? 0 : 1);
+	nerrors += (s1 <  s2 ? 0 : 1);
+	nerrors += (s1 == s2 ? 1 : 0);
+	nerrors += (s1 != s2 ? 0 : 1);
+	nerrors += (s1 >= s2 ? 1 : 0);
+	nerrors += (s1 >  s2 ? 1 : 0);
+
+	nerrors += (s2 <= s1 ? 1 : 0);
+	nerrors += (s2 <  s1 ? 1 : 0);
+	nerrors += (s2 == s1 ? 1 : 0);
+	nerrors += (s2 != s1 ? 0 : 1);
+	nerrors += (s2 >= s1 ? 0 : 1);
+	nerrors += (s2 >  s1 ? 0 : 1);
+
+	exit(nerrors == 0 ? 0 : 1);
+}
+ERROR
+{
+	exit(1);
+}
-- 
2.18.4




More information about the DTrace-devel mailing list