[DTrace-devel] [PATCH v3 3/3] String args to dt_strcmp() need to be copied if not DPTR
Kris Van Hees
kris.van.hees at oracle.com
Sat Oct 8 17:19:09 UTC 2022
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>
Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
---
bpf/strcmp.S | 52 ++++++++++++++-----
libdtrace/dt_cg.c | 13 ++++-
.../codegen/tst.execname-cmp-bvar_lhs.d | 19 +++++++
.../codegen/tst.execname-cmp-bvar_rhs.d | 19 +++++++
.../codegen/tst.execname-cmp-const_lhs.d | 19 +++++++
.../codegen/tst.execname-cmp-const_rhs.d | 19 +++++++
.../codegen/tst.kernel_str-cmp-bvar_lhs.d | 19 +++++++
.../codegen/tst.kernel_str-cmp-bvar_rhs.d | 19 +++++++
.../codegen/tst.kernel_str-cmp-const_lhs.d | 19 +++++++
.../codegen/tst.kernel_str-cmp-const_rhs.d | 19 +++++++
10 files changed, 202 insertions(+), 15 deletions(-)
create mode 100644 test/unittest/codegen/tst.execname-cmp-bvar_lhs.d
create mode 100644 test/unittest/codegen/tst.execname-cmp-bvar_rhs.d
create mode 100644 test/unittest/codegen/tst.execname-cmp-const_lhs.d
create mode 100644 test/unittest/codegen/tst.execname-cmp-const_rhs.d
create mode 100644 test/unittest/codegen/tst.kernel_str-cmp-bvar_lhs.d
create mode 100644 test/unittest/codegen/tst.kernel_str-cmp-bvar_rhs.d
create mode 100644 test/unittest/codegen/tst.kernel_str-cmp-const_lhs.d
create mode 100644 test/unittest/codegen/tst.kernel_str-cmp-const_rhs.d
diff --git a/bpf/strcmp.S b/bpf/strcmp.S
index 75177469..49d1e64a 100644
--- a/bpf/strcmp.S
+++ b/bpf/strcmp.S
@@ -58,12 +58,13 @@ 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
* [%fp-24]=tmp1
* [%fp-32]=tmp2
+ * r9=flags
*
* r8 = STRSZ
*
@@ -86,8 +87,14 @@ dt_strcmp_not :
* // based on this location, judge if the strings are >, <, or ==
* if (r0 > r6) goto Lsame;
* if (r0 > r7) goto Lsame;
- * if (s[r0] > t[r0]) return +1;
- * if (s[r0] < t[r0]) return +1;
+ *
+ * s += r0;
+ * t += r0;
+ *
+ * if (flags & 1) { } else { bpf_probe_read(tmp1, 1, s); s = tmp1; }
+ * if (flags & 2) { } else { bpf_probe_read(tmp2, 1, t); t = tmp2; }
+ * if (s[0] > t[0]) return +1;
+ * return -1;
*
* // if all chars are the same, break tie on string length
* Lsame:
@@ -105,6 +112,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 */
@@ -156,22 +164,38 @@ dt_strcmp :
sub %r0, 1 /* r0 -= 1 */
jgt %r0, %r6, .Lsame /* if (r0 > r6) goto Lsame */
- jgt %r0, %r8, .Lsame /* if (r0 > r8) goto Lsame */
+ jgt %r0, %r7, .Lsame /* if (r0 > r7) goto Lsame */
+
+ ldxdw %r6, [%fp+-8]
+ add %r6, %r0 /* s += r0 */
+ ldxdw %r7, [%fp+-16]
+ add %r7, %r0 /* t += r0 */
+
+ jset %r9, 1, .Ls_is_dptr
+ ldxdw %r1, [%fp+-24]
+ mov %r2, 1
+ mov %r3, %r6
+ mov %r6, %r1 /* s = tmp1 (after the helper call) */
+ call BPF_FUNC_probe_read /* bpf_probe_read(tmp1, 1, s) */
+.Ls_is_dptr:
+
+ jset %r9, 2, .Lt_is_dptr
+ ldxdw %r1, [%fp+-32]
+ mov %r2, 1
+ mov %r3, %r7
+ mov %r7, %r1 /* t = tmp2 (after the helper call) */
+ call BPF_FUNC_probe_read /* bpf_probe_read(tmp2, 1, t) */
+.Lt_is_dptr:
- ldxdw %r4, [%fp+-8]
- add %r4, %r0
- ldxb %r4, [%r4+0] /* s[r0] */
- and %r4, 0xff
+ ldxb %r6, [%r6+0] /* s[0] */
+ and %r6, 0xff
- ldxdw %r5, [%fp+-16]
- add %r5, %r0
- ldxb %r5, [%r5+0] /* t[r0] */
- and %r5, 0xff
+ ldxb %r7, [%r7+0] /* t[0] */
+ and %r7, 0xff
- jle %r4, %r5, 2 /* if (s[r0] > t[r0]) return +1 */
+ jle %r6, %r7, 2 /* if (s[0] > t[0]) return +1 */
mov %r0, 1
exit
- jge %r4, %r5, 2 /* if (s[r0] < t[r0]) return +1 */
mov %r0, -1
exit
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index c4e49129..ef639161 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -3294,6 +3294,7 @@ 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 */
@@ -3301,7 +3302,16 @@ dt_cg_compare_op(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp, uint_t op)
emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_right->dn_reg, 0, Lcomp));
/*
- * Otherwise, replace pointers with relative string values.
+ * 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;
+
+ /*
+ * Replace pointers with relative string values.
* Specifically, replace left with strcmp(left,right)+1 and
* right with 1, so we can use unsigned comparisons.
*/
@@ -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);
diff --git a/test/unittest/codegen/tst.execname-cmp-bvar_lhs.d b/test/unittest/codegen/tst.execname-cmp-bvar_lhs.d
new file mode 100644
index 00000000..d36bbf07
--- /dev/null
+++ b/test/unittest/codegen/tst.execname-cmp-bvar_lhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(probename == execname);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.execname-cmp-bvar_rhs.d b/test/unittest/codegen/tst.execname-cmp-bvar_rhs.d
new file mode 100644
index 00000000..dcdf3507
--- /dev/null
+++ b/test/unittest/codegen/tst.execname-cmp-bvar_rhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(execname == probename);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.execname-cmp-const_lhs.d b/test/unittest/codegen/tst.execname-cmp-const_lhs.d
new file mode 100644
index 00000000..63e8d987
--- /dev/null
+++ b/test/unittest/codegen/tst.execname-cmp-const_lhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace("dtrace" == execname);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.execname-cmp-const_rhs.d b/test/unittest/codegen/tst.execname-cmp-const_rhs.d
new file mode 100644
index 00000000..e256f3a9
--- /dev/null
+++ b/test/unittest/codegen/tst.execname-cmp-const_rhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(execname == "dtrace");
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.kernel_str-cmp-bvar_lhs.d b/test/unittest/codegen/tst.kernel_str-cmp-bvar_lhs.d
new file mode 100644
index 00000000..ef95815a
--- /dev/null
+++ b/test/unittest/codegen/tst.kernel_str-cmp-bvar_lhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(probename == curthread->comm);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.kernel_str-cmp-bvar_rhs.d b/test/unittest/codegen/tst.kernel_str-cmp-bvar_rhs.d
new file mode 100644
index 00000000..1dd9a4ed
--- /dev/null
+++ b/test/unittest/codegen/tst.kernel_str-cmp-bvar_rhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(curthread->comm == probename);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.kernel_str-cmp-const_lhs.d b/test/unittest/codegen/tst.kernel_str-cmp-const_lhs.d
new file mode 100644
index 00000000..e877966e
--- /dev/null
+++ b/test/unittest/codegen/tst.kernel_str-cmp-const_lhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace("dtrace" == curthread->comm);
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
diff --git a/test/unittest/codegen/tst.kernel_str-cmp-const_rhs.d b/test/unittest/codegen/tst.kernel_str-cmp-const_rhs.d
new file mode 100644
index 00000000..baebe17f
--- /dev/null
+++ b/test/unittest/codegen/tst.kernel_str-cmp-const_rhs.d
@@ -0,0 +1,19 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2022, 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.
+ */
+
+#pragma D option quiet
+
+BEGIN
+{
+ trace(curthread->comm == "dtrace");
+ exit(0);
+}
+
+ERROR
+{
+ exit(1);
+}
--
2.37.2
More information about the DTrace-devel
mailing list