Linux DTrace development list
 help / color / mirror / Atom feed
* [PATCH] test: Account for pid:::entry ucaller being correct
@ 2025-03-19  6:32 eugene.loh
  2025-03-19  6:32 ` [PATCH] Fix dt_bvar_probedesc() for late USDT processes eugene.loh
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: eugene.loh @ 2025-03-19  6:32 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

In commit f38bdf9ea ("test: Account for pid:::entry ustack() being correct")
we accounted for x86-specific heuristics introduced in Linux 6.11 that
dealt with pid:::entry uprobes firing so early in the function preamble
that the frame pointer is not yet set and the caller is not (yet)
correctly identified.

Update a related test to account for the same effect with ucaller.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/vars/tst.ucaller.r.p | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100755 test/unittest/vars/tst.ucaller.r.p

diff --git a/test/unittest/vars/tst.ucaller.r.p b/test/unittest/vars/tst.ucaller.r.p
new file mode 100755
index 000000000..8e03f110d
--- /dev/null
+++ b/test/unittest/vars/tst.ucaller.r.p
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+# A pid entry probe places a uprobe on the first instruction of a function.
+# Unfortunately, this is so early in the function preamble that the function
+# frame pointer has not yet been established and the actual caller of the
+# traced function is missed.
+#
+# In Linux 6.11, x86-specific heuristics are introduced to fix this problem.
+# See commit cfa7f3d
+# ("perf,x86: avoid missing caller address in stack traces captured in uprobe")
+# for both a description of the problem and an explanation of the heuristics.
+#
+# Add post processing to these test results to allow for both cases:
+# caller frame is missing or not missing.
+
+if [ $(uname -m) == "x86_64" ]; then
+        read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
+
+        if [ $MAJOR -ge 6 ]; then
+                if [ $MAJOR -gt 6 -o $MINOR -ge 11 ]; then
+                        awk '{ sub("myfunc_w", "myfunc_v"); print; }'
+                        exit 0
+                fi
+        fi
+fi
+
+# Otherwise, just pass the output through.
+cat
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] Fix dt_bvar_probedesc() for late USDT processes
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
@ 2025-03-19  6:32 ` eugene.loh
  2025-03-19 18:54   ` Kris Van Hees
  2025-03-19  6:32 ` [PATCH] Copy fprobes entry args with BPF helper function eugene.loh
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: eugene.loh @ 2025-03-19  6:32 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

With commit 8bd26415b
("bpf: separate bvar implementation into separate functions"),
test/unittest/usdt/tst.nusdtprobes.sh started failing reproducibly
on all platforms.

In that patch, the get_bvar() function is factored into separate
functions.  It includes a change that looks basically like this:

    uint32_t    key = mst->prid;

    if (key < ((uint64_t)&NPROBES)) {
        [...]
    } else {
        char *s = bpf_map_lookup_elem(&usdt_names, &key);
        switch (idx) {
  -     case DIF_VAR_PROBENAME: s += DTRACE_FUNCNAMELEN;
  +     case DIF_VAR_PROBEPROV: s += DTRACE_FUNCNAMELEN;
  -     case DIF_VAR_PROBEFUNC: s += DTRACE_MODNAMELEN;
  +     case DIF_VAR_PROBEMOD : s += DTRACE_MODNAMELEN;
  -     case DIF_VAR_PROBEMOD : s += DTRACE_PROVNAMELEN;
  +     case DIF_VAR_PROBEFUNC: s += DTRACE_PROVNAMELEN;
  -     case DIF_VAR_PROBEPROV:
  +     case DIF_VAR_PROBENAME:
        }
        return (uint64_t)s;
    }

That is, for the case of key>=NPROBES (that is, for USDT probes that
were added after the dtrace session was started), the meanings of
prov, mod, func, and name were exchanged.

Restore the correct meanings.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 bpf/get_bvar.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
index c760126da..fadb06c00 100644
--- a/bpf/get_bvar.c
+++ b/bpf/get_bvar.c
@@ -185,13 +185,13 @@ noinline uint64_t dt_bvar_probedesc(const dt_dctx_t *dctx, uint32_t idx)
 			return (uint64_t)dctx->strtab;
 
 		switch (idx) {
-		case DIF_VAR_PROBEPROV:
+		case DIF_VAR_PROBENAME:
 			s += DTRACE_FUNCNAMELEN;
-		case DIF_VAR_PROBEMOD:
-			s += DTRACE_MODNAMELEN;
 		case DIF_VAR_PROBEFUNC:
+			s += DTRACE_MODNAMELEN;
+		case DIF_VAR_PROBEMOD:
 			s += DTRACE_PROVNAMELEN;
-		case DIF_VAR_PROBENAME:
+		case DIF_VAR_PROBEPROV:
 		}
 
 		return (uint64_t)s;
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] Copy fprobes entry args with BPF helper function
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
  2025-03-19  6:32 ` [PATCH] Fix dt_bvar_probedesc() for late USDT processes eugene.loh
@ 2025-03-19  6:32 ` eugene.loh
  2025-03-19 18:58   ` Kris Van Hees
  2025-03-19  6:32 ` [PATCH] test: Expect USDT argmap to fail on ARM on older kernels eugene.loh
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: eugene.loh @ 2025-03-19  6:32 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

With commit a6b626a89 ("Fix fprobe/kprobe selection"), fprobes were
effectively turned on.  Unfortunately, with this fix, some tests like
test/unittest/stack/tst.stack_fbt.sh encountered problems on UEK7
since the BPF verifier would complain about the prototypes of some of
the probe arguments.  E.g., when loading arg3 in fprobe_trampoline()
from fbt::vfs_write:entry from %r8+24, the BPF verifier complains

        func 'vfs_write' arg3 type INT is not a struct
        invalid bpf_context access off=24 size=8

We can bypass this problem by using a BPF helper function to copy the
arguments onto the BPF stack and then load the arguments into mstate
from there.

There is also a BPF get_func_arg() helper function, but it is not
introduced until 5.17 -- that is, it appears after UEK7.  See Linux
commit f92c1e1 ("bpf: Add get_func_[arg|ret|arg_cnt] helpers").

While the already mentioned test signals the problem and the fix, we
also add an additional test that actually checks the correctness of
the arguments.

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 libdtrace/dt_prov_fbt.c                     |  14 ++-
 test/unittest/fbtprovider/tst.entryargs2.r  |  29 ++++++
 test/unittest/fbtprovider/tst.entryargs2.sh | 105 ++++++++++++++++++++
 3 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 test/unittest/fbtprovider/tst.entryargs2.r
 create mode 100755 test/unittest/fbtprovider/tst.entryargs2.sh

diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index 8aa53d643..50fa0d9dc 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -285,8 +285,20 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 	if (strcmp(pcb->pcb_probe->desc->prb, "entry") == 0) {
 		int	i;
 
+		/*
+		 * We want to copy entry args from %r8 to %r7 (plus offsets).
+		 * Unfortunately, for fprobes, the BPF verifier can reject
+		 * certain argument types.  We work around this by copying
+		 * the arguments onto the BPF stack and loading them from there.
+		 */
+		emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
+		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(prp->argc - 1)));
+		emit(dlp, BPF_MOV_IMM(BPF_REG_2, 8 * prp->argc));
+		emit(dlp, BPF_MOV_REG(BPF_REG_3, BPF_REG_8));
+		emit(dlp, BPF_CALL_HELPER(dtp->dt_bpfhelper[BPF_FUNC_probe_read_kernel]));
+
 		for (i = 0; i < prp->argc; i++) {
-			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, i * 8));
+			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_TRAMP_SP_SLOT(prp->argc - 1) + i * 8));
 			emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
 		}
 	} else {
diff --git a/test/unittest/fbtprovider/tst.entryargs2.r b/test/unittest/fbtprovider/tst.entryargs2.r
new file mode 100644
index 000000000..efc4685f9
--- /dev/null
+++ b/test/unittest/fbtprovider/tst.entryargs2.r
@@ -0,0 +1,29 @@
+mode READ           : no
+mode WRITE          : yes
+mode LSEEK          : yes
+mode PREAD          : yes
+mode PWRITE         : yes
+mode WRITER         : yes
+mode CAN_READ       : no
+mode CAN_WRITE      : yes
+mode OPENED         : yes
+buf: =========================
+count: 8
+pos: 20
+abcdefghijklmnopqrst========CDEFGHIJKLMNOPQRSTUVWXYZ0123456789
+
+mode READ           : yes
+mode WRITE          : yes
+mode LSEEK          : yes
+mode PREAD          : yes
+mode PWRITE         : yes
+mode WRITER         : yes
+mode CAN_READ       : yes
+mode CAN_WRITE      : yes
+mode OPENED         : yes
+buf: =========================
+count: 8
+pos: 20
+abcdefghijklmnopqrst========CDEFGHIJKLMNOPQRSTUVWXYZ0123456789
+
+success
diff --git a/test/unittest/fbtprovider/tst.entryargs2.sh b/test/unittest/fbtprovider/tst.entryargs2.sh
new file mode 100755
index 000000000..f5b435f56
--- /dev/null
+++ b/test/unittest/fbtprovider/tst.entryargs2.sh
@@ -0,0 +1,105 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2025, 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.
+#
+# Another test of entry args.
+#
+
+dtrace=$1
+CC=${CC:-/usr/bin/gcc}
+
+# Set up test directory.
+
+DIRNAME=$tmpdir/entryargs2.$$.$RANDOM
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+# Make the trigger.
+
+cat << EOF > main.c
+#include <stdio.h>
+#include <fcntl.h>  // open()
+#include <unistd.h> // lseek(), write(), close()
+
+int main(int c, char **v) {
+	int fd = open("tmp.txt", c == 1 ? O_WRONLY : O_RDWR);
+
+	if (fd == -1)
+		return 1;
+
+	/* Move the offset, then write to the file. */
+	/* (We will overwrite some "middle section" of the file with "========".) */
+	lseek(fd, 20, SEEK_SET);
+	write(fd, "=========================", 8);
+	close(fd);
+
+	return 0;
+}
+EOF
+
+# Build the trigger.           FIXME do consistent with Sam's changes.
+
+$CC $test_cppflags $test_ldflags main.c
+if [ $? -ne 0 ]; then
+	echo "failed to link final executable" >&2
+	exit 1
+fi
+
+# Prepare the D script.
+
+cat << EOF > D.d
+/* these definitions come from kernel header include/linux/fs.h */
+#define FMODE_READ	(1 << 0)
+#define FMODE_WRITE	(1 << 1)
+#define FMODE_LSEEK	(1 << 2)
+#define FMODE_PREAD	(1 << 3)
+#define FMODE_PWRITE	(1 << 4)
+
+#define FMODE_WRITER	(1 << 16)
+#define FMODE_CAN_READ	(1 << 17)
+#define FMODE_CAN_WRITE	(1 << 18)
+#define FMODE_OPENED	(1 << 19)
+
+fbt::vfs_write:entry
+/pid == \$target/
+{
+	mode = ((struct file *)arg0)->f_mode;
+	printf("mode READ           : %s\n", mode & FMODE_READ      ? "yes" : "no");
+	printf("mode WRITE          : %s\n", mode & FMODE_WRITE     ? "yes" : "no");
+	printf("mode LSEEK          : %s\n", mode & FMODE_LSEEK     ? "yes" : "no");
+	printf("mode PREAD          : %s\n", mode & FMODE_PREAD     ? "yes" : "no");
+	printf("mode PWRITE         : %s\n", mode & FMODE_PWRITE    ? "yes" : "no");
+	printf("mode WRITER         : %s\n", mode & FMODE_WRITER    ? "yes" : "no");
+	printf("mode CAN_READ       : %s\n", mode & FMODE_CAN_READ  ? "yes" : "no");
+	printf("mode CAN_WRITE      : %s\n", mode & FMODE_CAN_WRITE ? "yes" : "no");
+	printf("mode OPENED         : %s\n", mode & FMODE_OPENED    ? "yes" : "no");
+
+	printf("buf: %s\n", stringof(copyinstr(arg1)));
+	printf("count: %d\n", arg2);
+	printf("pos: %d", *((loff_t *)arg3));
+	exit(0);
+}
+EOF
+
+# Run the D script and trigger twice, once with O_WRONLY and then O_RDWR.
+
+for args in "" "dummy"; do
+
+	# Prepare the file to be (over)written.
+	rm -f tmp.txt
+	echo abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 > tmp.txt
+
+	# Run the D script and trigger.
+	$dtrace $dt_flags -c "./a.out $args" -Cqs D.d
+
+        # Report the output file.
+	cat tmp.txt
+	echo
+
+done
+
+echo success
+exit 0
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] test: Expect USDT argmap to fail on ARM on older kernels
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
  2025-03-19  6:32 ` [PATCH] Fix dt_bvar_probedesc() for late USDT processes eugene.loh
  2025-03-19  6:32 ` [PATCH] Copy fprobes entry args with BPF helper function eugene.loh
@ 2025-03-19  6:32 ` eugene.loh
  2025-03-19 19:04   ` [DTrace-devel] " Kris Van Hees
  2025-03-19  6:32 ` [PATCH] Get execargs from user space eugene.loh
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: eugene.loh @ 2025-03-19  6:32 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 test/unittest/usdt/skip_arm_uek6.x            | 25 +++++++++++++++++++
 .../usdt/tst.argmap-typed-partial.aarch64.x   |  1 +
 test/unittest/usdt/tst.argmap-typed.aarch64.x |  1 +
 .../tst.multiprov-dupprobe-fire.aarch64.x     |  1 +
 .../tst.multiprov-dupprobe-shlibs.aarch64.x   |  1 +
 .../usdt/tst.multiprovider-fire.aarch64.x     |  1 +
 6 files changed, 30 insertions(+)
 create mode 100755 test/unittest/usdt/skip_arm_uek6.x
 create mode 120000 test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
 create mode 120000 test/unittest/usdt/tst.argmap-typed.aarch64.x
 create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
 create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
 create mode 120000 test/unittest/usdt/tst.multiprovider-fire.aarch64.x

diff --git a/test/unittest/usdt/skip_arm_uek6.x b/test/unittest/usdt/skip_arm_uek6.x
new file mode 100755
index 000000000..252cbebb5
--- /dev/null
+++ b/test/unittest/usdt/skip_arm_uek6.x
@@ -0,0 +1,25 @@
+#!/bin/bash
+# Licensed under the Universal Permissive License v 1.0 as shown at
+# http://oss.oracle.com/licenses/upl.
+#
+# @@skip: not run directly by test harness
+#
+# Tests that depend on USDT argument translation fail on ARM for UEK6.
+# They're fine for UEK7.  It is unclear in exactly which kernel they
+# start working.
+
+if [[ `uname -m` != "aarch64" ]]; then
+	exit 0
+fi
+
+read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
+
+if [ $MAJOR -gt 5 ]; then
+	exit 0
+fi
+if [ $MAJOR -eq 5 -a $MINOR -ge 10 ]; then
+	exit 0
+fi
+
+echo "USDT argmap not working on ARM on older kernels"
+exit 1
diff --git a/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
new file mode 120000
index 000000000..8d462f98f
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
@@ -0,0 +1 @@
+skip_arm_uek6.x
\ No newline at end of file
diff --git a/test/unittest/usdt/tst.argmap-typed.aarch64.x b/test/unittest/usdt/tst.argmap-typed.aarch64.x
new file mode 120000
index 000000000..8d462f98f
--- /dev/null
+++ b/test/unittest/usdt/tst.argmap-typed.aarch64.x
@@ -0,0 +1 @@
+skip_arm_uek6.x
\ No newline at end of file
diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
new file mode 120000
index 000000000..8d462f98f
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
@@ -0,0 +1 @@
+skip_arm_uek6.x
\ No newline at end of file
diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
new file mode 120000
index 000000000..8d462f98f
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
@@ -0,0 +1 @@
+skip_arm_uek6.x
\ No newline at end of file
diff --git a/test/unittest/usdt/tst.multiprovider-fire.aarch64.x b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
new file mode 120000
index 000000000..8d462f98f
--- /dev/null
+++ b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
@@ -0,0 +1 @@
+skip_arm_uek6.x
\ No newline at end of file
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH] Get execargs from user space
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
                   ` (2 preceding siblings ...)
  2025-03-19  6:32 ` [PATCH] test: Expect USDT argmap to fail on ARM on older kernels eugene.loh
@ 2025-03-19  6:32 ` eugene.loh
  2025-03-19 19:07   ` [DTrace-devel] " Kris Van Hees
  2025-03-19 18:53 ` [PATCH] test: Account for pid:::entry ucaller being correct Kris Van Hees
  2025-04-14 23:33 ` [DTrace-devel] " Sam James
  5 siblings, 1 reply; 13+ messages in thread
From: eugene.loh @ 2025-03-19  6:32 UTC (permalink / raw)
  To: dtrace, dtrace-devel

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

Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
---
 bpf/bvar_execargs.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bpf/bvar_execargs.S b/bpf/bvar_execargs.S
index 1c47cafb2..08844f15f 100644
--- a/bpf/bvar_execargs.S
+++ b/bpf/bvar_execargs.S
@@ -65,7 +65,7 @@ dt_bvar_execargs:
 	mov	%r1, %r9
 	mov	%r2, %r8
 	mov	%r3, %r7
-	call	BPF_FUNC_probe_read		/* bpf_probe_read(&args, len + 1, arg_start) */
+	call	BPF_FUNC_probe_read_user	/* bpf_probe_read(&args, len + 1, arg_start) */
 	jne	%r0, 0, .Lerror
 
 	/* loop over args and replace '\0' with ' ' */
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] test: Account for pid:::entry ucaller being correct
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
                   ` (3 preceding siblings ...)
  2025-03-19  6:32 ` [PATCH] Get execargs from user space eugene.loh
@ 2025-03-19 18:53 ` Kris Van Hees
  2025-04-14 23:33 ` [DTrace-devel] " Sam James
  5 siblings, 0 replies; 13+ messages in thread
From: Kris Van Hees @ 2025-03-19 18:53 UTC (permalink / raw)
  To: eugene.loh; +Cc: dtrace, dtrace-devel

On Wed, Mar 19, 2025 at 02:32:26AM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> In commit f38bdf9ea ("test: Account for pid:::entry ustack() being correct")
> we accounted for x86-specific heuristics introduced in Linux 6.11 that
> dealt with pid:::entry uprobes firing so early in the function preamble
> that the frame pointer is not yet set and the caller is not (yet)
> correctly identified.
> 
> Update a related test to account for the same effect with ucaller.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

LGTM
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  test/unittest/vars/tst.ucaller.r.p | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100755 test/unittest/vars/tst.ucaller.r.p
> 
> diff --git a/test/unittest/vars/tst.ucaller.r.p b/test/unittest/vars/tst.ucaller.r.p
> new file mode 100755
> index 000000000..8e03f110d
> --- /dev/null
> +++ b/test/unittest/vars/tst.ucaller.r.p
> @@ -0,0 +1,28 @@
> +#!/bin/sh
> +
> +# A pid entry probe places a uprobe on the first instruction of a function.
> +# Unfortunately, this is so early in the function preamble that the function
> +# frame pointer has not yet been established and the actual caller of the
> +# traced function is missed.
> +#
> +# In Linux 6.11, x86-specific heuristics are introduced to fix this problem.
> +# See commit cfa7f3d
> +# ("perf,x86: avoid missing caller address in stack traces captured in uprobe")
> +# for both a description of the problem and an explanation of the heuristics.
> +#
> +# Add post processing to these test results to allow for both cases:
> +# caller frame is missing or not missing.
> +
> +if [ $(uname -m) == "x86_64" ]; then
> +        read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
> +
> +        if [ $MAJOR -ge 6 ]; then
> +                if [ $MAJOR -gt 6 -o $MINOR -ge 11 ]; then
> +                        awk '{ sub("myfunc_w", "myfunc_v"); print; }'
> +                        exit 0
> +                fi
> +        fi
> +fi
> +
> +# Otherwise, just pass the output through.
> +cat
> -- 
> 2.43.5
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Fix dt_bvar_probedesc() for late USDT processes
  2025-03-19  6:32 ` [PATCH] Fix dt_bvar_probedesc() for late USDT processes eugene.loh
@ 2025-03-19 18:54   ` Kris Van Hees
  0 siblings, 0 replies; 13+ messages in thread
From: Kris Van Hees @ 2025-03-19 18:54 UTC (permalink / raw)
  To: eugene.loh; +Cc: dtrace, dtrace-devel

On Wed, Mar 19, 2025 at 02:32:27AM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> With commit 8bd26415b
> ("bpf: separate bvar implementation into separate functions"),
> test/unittest/usdt/tst.nusdtprobes.sh started failing reproducibly
> on all platforms.
> 
> In that patch, the get_bvar() function is factored into separate
> functions.  It includes a change that looks basically like this:
> 
>     uint32_t    key = mst->prid;
> 
>     if (key < ((uint64_t)&NPROBES)) {
>         [...]
>     } else {
>         char *s = bpf_map_lookup_elem(&usdt_names, &key);
>         switch (idx) {
>   -     case DIF_VAR_PROBENAME: s += DTRACE_FUNCNAMELEN;
>   +     case DIF_VAR_PROBEPROV: s += DTRACE_FUNCNAMELEN;
>   -     case DIF_VAR_PROBEFUNC: s += DTRACE_MODNAMELEN;
>   +     case DIF_VAR_PROBEMOD : s += DTRACE_MODNAMELEN;
>   -     case DIF_VAR_PROBEMOD : s += DTRACE_PROVNAMELEN;
>   +     case DIF_VAR_PROBEFUNC: s += DTRACE_PROVNAMELEN;
>   -     case DIF_VAR_PROBEPROV:
>   +     case DIF_VAR_PROBENAME:
>         }
>         return (uint64_t)s;
>     }
> 
> That is, for the case of key>=NPROBES (that is, for USDT probes that
> were added after the dtrace session was started), the meanings of
> prov, mod, func, and name were exchanged.
> 
> Restore the correct meanings.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

My fault.  Thanks for the fix.
Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  bpf/get_bvar.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/bpf/get_bvar.c b/bpf/get_bvar.c
> index c760126da..fadb06c00 100644
> --- a/bpf/get_bvar.c
> +++ b/bpf/get_bvar.c
> @@ -185,13 +185,13 @@ noinline uint64_t dt_bvar_probedesc(const dt_dctx_t *dctx, uint32_t idx)
>  			return (uint64_t)dctx->strtab;
>  
>  		switch (idx) {
> -		case DIF_VAR_PROBEPROV:
> +		case DIF_VAR_PROBENAME:
>  			s += DTRACE_FUNCNAMELEN;
> -		case DIF_VAR_PROBEMOD:
> -			s += DTRACE_MODNAMELEN;
>  		case DIF_VAR_PROBEFUNC:
> +			s += DTRACE_MODNAMELEN;
> +		case DIF_VAR_PROBEMOD:
>  			s += DTRACE_PROVNAMELEN;
> -		case DIF_VAR_PROBENAME:
> +		case DIF_VAR_PROBEPROV:
>  		}
>  
>  		return (uint64_t)s;
> -- 
> 2.43.5
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] Copy fprobes entry args with BPF helper function
  2025-03-19  6:32 ` [PATCH] Copy fprobes entry args with BPF helper function eugene.loh
@ 2025-03-19 18:58   ` Kris Van Hees
  0 siblings, 0 replies; 13+ messages in thread
From: Kris Van Hees @ 2025-03-19 18:58 UTC (permalink / raw)
  To: eugene.loh; +Cc: dtrace, dtrace-devel

On Wed, Mar 19, 2025 at 02:32:28AM -0400, eugene.loh@oracle.com wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> With commit a6b626a89 ("Fix fprobe/kprobe selection"), fprobes were
> effectively turned on.  Unfortunately, with this fix, some tests like
> test/unittest/stack/tst.stack_fbt.sh encountered problems on UEK7
> since the BPF verifier would complain about the prototypes of some of
> the probe arguments.  E.g., when loading arg3 in fprobe_trampoline()
> from fbt::vfs_write:entry from %r8+24, the BPF verifier complains
> 
>         func 'vfs_write' arg3 type INT is not a struct
>         invalid bpf_context access off=24 size=8
> 
> We can bypass this problem by using a BPF helper function to copy the
> arguments onto the BPF stack and then load the arguments into mstate
> from there.
> 
> There is also a BPF get_func_arg() helper function, but it is not
> introduced until 5.17 -- that is, it appears after UEK7.  See Linux
> commit f92c1e1 ("bpf: Add get_func_[arg|ret|arg_cnt] helpers").

I'm OK with merging this (see r-b below) but I think that we should perhaps
see a follow-up patch soon that implements using bpf_get_func_arg() if
available, and otherwise use bpf_probe_read_kernel().  Not that it is (at
this point) necessary but it might be a good idea to adjust to newer ways
to access this data in case e.g. future kernels will hide access to the
argument data behind that bpf_get_fucn_arg() helper.

> While the already mentioned test signals the problem and the fix, we
> also add an additional test that actually checks the correctness of
> the arguments.
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

> ---
>  libdtrace/dt_prov_fbt.c                     |  14 ++-
>  test/unittest/fbtprovider/tst.entryargs2.r  |  29 ++++++
>  test/unittest/fbtprovider/tst.entryargs2.sh | 105 ++++++++++++++++++++
>  3 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 test/unittest/fbtprovider/tst.entryargs2.r
>  create mode 100755 test/unittest/fbtprovider/tst.entryargs2.sh
> 
> diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
> index 8aa53d643..50fa0d9dc 100644
> --- a/libdtrace/dt_prov_fbt.c
> +++ b/libdtrace/dt_prov_fbt.c
> @@ -285,8 +285,20 @@ static int fprobe_trampoline(dt_pcb_t *pcb, uint_t exitlbl)
>  	if (strcmp(pcb->pcb_probe->desc->prb, "entry") == 0) {
>  		int	i;
>  
> +		/*
> +		 * We want to copy entry args from %r8 to %r7 (plus offsets).
> +		 * Unfortunately, for fprobes, the BPF verifier can reject
> +		 * certain argument types.  We work around this by copying
> +		 * the arguments onto the BPF stack and loading them from there.
> +		 */
> +		emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_FP));
> +		emit(dlp, BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, DT_TRAMP_SP_SLOT(prp->argc - 1)));
> +		emit(dlp, BPF_MOV_IMM(BPF_REG_2, 8 * prp->argc));
> +		emit(dlp, BPF_MOV_REG(BPF_REG_3, BPF_REG_8));
> +		emit(dlp, BPF_CALL_HELPER(dtp->dt_bpfhelper[BPF_FUNC_probe_read_kernel]));
> +
>  		for (i = 0; i < prp->argc; i++) {
> -			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_8, i * 8));
> +			emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_TRAMP_SP_SLOT(prp->argc - 1) + i * 8));
>  			emit(dlp, BPF_STORE(BPF_DW, BPF_REG_7, DMST_ARG(i), BPF_REG_0));
>  		}
>  	} else {
> diff --git a/test/unittest/fbtprovider/tst.entryargs2.r b/test/unittest/fbtprovider/tst.entryargs2.r
> new file mode 100644
> index 000000000..efc4685f9
> --- /dev/null
> +++ b/test/unittest/fbtprovider/tst.entryargs2.r
> @@ -0,0 +1,29 @@
> +mode READ           : no
> +mode WRITE          : yes
> +mode LSEEK          : yes
> +mode PREAD          : yes
> +mode PWRITE         : yes
> +mode WRITER         : yes
> +mode CAN_READ       : no
> +mode CAN_WRITE      : yes
> +mode OPENED         : yes
> +buf: =========================
> +count: 8
> +pos: 20
> +abcdefghijklmnopqrst========CDEFGHIJKLMNOPQRSTUVWXYZ0123456789
> +
> +mode READ           : yes
> +mode WRITE          : yes
> +mode LSEEK          : yes
> +mode PREAD          : yes
> +mode PWRITE         : yes
> +mode WRITER         : yes
> +mode CAN_READ       : yes
> +mode CAN_WRITE      : yes
> +mode OPENED         : yes
> +buf: =========================
> +count: 8
> +pos: 20
> +abcdefghijklmnopqrst========CDEFGHIJKLMNOPQRSTUVWXYZ0123456789
> +
> +success
> diff --git a/test/unittest/fbtprovider/tst.entryargs2.sh b/test/unittest/fbtprovider/tst.entryargs2.sh
> new file mode 100755
> index 000000000..f5b435f56
> --- /dev/null
> +++ b/test/unittest/fbtprovider/tst.entryargs2.sh
> @@ -0,0 +1,105 @@
> +#!/bin/bash
> +#
> +# Oracle Linux DTrace.
> +# Copyright (c) 2025, 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.
> +#
> +# Another test of entry args.
> +#
> +
> +dtrace=$1
> +CC=${CC:-/usr/bin/gcc}
> +
> +# Set up test directory.
> +
> +DIRNAME=$tmpdir/entryargs2.$$.$RANDOM
> +mkdir -p $DIRNAME
> +cd $DIRNAME
> +
> +# Make the trigger.
> +
> +cat << EOF > main.c
> +#include <stdio.h>
> +#include <fcntl.h>  // open()
> +#include <unistd.h> // lseek(), write(), close()
> +
> +int main(int c, char **v) {
> +	int fd = open("tmp.txt", c == 1 ? O_WRONLY : O_RDWR);
> +
> +	if (fd == -1)
> +		return 1;
> +
> +	/* Move the offset, then write to the file. */
> +	/* (We will overwrite some "middle section" of the file with "========".) */
> +	lseek(fd, 20, SEEK_SET);
> +	write(fd, "=========================", 8);
> +	close(fd);
> +
> +	return 0;
> +}
> +EOF
> +
> +# Build the trigger.           FIXME do consistent with Sam's changes.
> +
> +$CC $test_cppflags $test_ldflags main.c
> +if [ $? -ne 0 ]; then
> +	echo "failed to link final executable" >&2
> +	exit 1
> +fi
> +
> +# Prepare the D script.
> +
> +cat << EOF > D.d
> +/* these definitions come from kernel header include/linux/fs.h */
> +#define FMODE_READ	(1 << 0)
> +#define FMODE_WRITE	(1 << 1)
> +#define FMODE_LSEEK	(1 << 2)
> +#define FMODE_PREAD	(1 << 3)
> +#define FMODE_PWRITE	(1 << 4)
> +
> +#define FMODE_WRITER	(1 << 16)
> +#define FMODE_CAN_READ	(1 << 17)
> +#define FMODE_CAN_WRITE	(1 << 18)
> +#define FMODE_OPENED	(1 << 19)
> +
> +fbt::vfs_write:entry
> +/pid == \$target/
> +{
> +	mode = ((struct file *)arg0)->f_mode;
> +	printf("mode READ           : %s\n", mode & FMODE_READ      ? "yes" : "no");
> +	printf("mode WRITE          : %s\n", mode & FMODE_WRITE     ? "yes" : "no");
> +	printf("mode LSEEK          : %s\n", mode & FMODE_LSEEK     ? "yes" : "no");
> +	printf("mode PREAD          : %s\n", mode & FMODE_PREAD     ? "yes" : "no");
> +	printf("mode PWRITE         : %s\n", mode & FMODE_PWRITE    ? "yes" : "no");
> +	printf("mode WRITER         : %s\n", mode & FMODE_WRITER    ? "yes" : "no");
> +	printf("mode CAN_READ       : %s\n", mode & FMODE_CAN_READ  ? "yes" : "no");
> +	printf("mode CAN_WRITE      : %s\n", mode & FMODE_CAN_WRITE ? "yes" : "no");
> +	printf("mode OPENED         : %s\n", mode & FMODE_OPENED    ? "yes" : "no");
> +
> +	printf("buf: %s\n", stringof(copyinstr(arg1)));
> +	printf("count: %d\n", arg2);
> +	printf("pos: %d", *((loff_t *)arg3));
> +	exit(0);
> +}
> +EOF
> +
> +# Run the D script and trigger twice, once with O_WRONLY and then O_RDWR.
> +
> +for args in "" "dummy"; do
> +
> +	# Prepare the file to be (over)written.
> +	rm -f tmp.txt
> +	echo abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 > tmp.txt
> +
> +	# Run the D script and trigger.
> +	$dtrace $dt_flags -c "./a.out $args" -Cqs D.d
> +
> +        # Report the output file.
> +	cat tmp.txt
> +	echo
> +
> +done
> +
> +echo success
> +exit 0
> -- 
> 2.43.5
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [DTrace-devel] [PATCH] test: Expect USDT argmap to fail on ARM on older kernels
  2025-03-19  6:32 ` [PATCH] test: Expect USDT argmap to fail on ARM on older kernels eugene.loh
@ 2025-03-19 19:04   ` Kris Van Hees
  2025-04-11 20:37     ` Kris Van Hees
  0 siblings, 1 reply; 13+ messages in thread
From: Kris Van Hees @ 2025-03-19 19:04 UTC (permalink / raw)
  To: eugene.loh; +Cc: dtrace, dtrace-devel

I am holding off on this patch for the moment, just to get a closer look at
the potential cause (and some info on how old the kernel needs to be for this
to fail).  I would expect arg access to be the problem rather than the mapping
because the mapping is simply something that happens in BPF code.  That should
not be kernel-dependent.

On Wed, Mar 19, 2025 at 02:32:29AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  test/unittest/usdt/skip_arm_uek6.x            | 25 +++++++++++++++++++
>  .../usdt/tst.argmap-typed-partial.aarch64.x   |  1 +
>  test/unittest/usdt/tst.argmap-typed.aarch64.x |  1 +
>  .../tst.multiprov-dupprobe-fire.aarch64.x     |  1 +
>  .../tst.multiprov-dupprobe-shlibs.aarch64.x   |  1 +
>  .../usdt/tst.multiprovider-fire.aarch64.x     |  1 +
>  6 files changed, 30 insertions(+)
>  create mode 100755 test/unittest/usdt/skip_arm_uek6.x
>  create mode 120000 test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
>  create mode 120000 test/unittest/usdt/tst.argmap-typed.aarch64.x
>  create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
>  create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
>  create mode 120000 test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> 
> diff --git a/test/unittest/usdt/skip_arm_uek6.x b/test/unittest/usdt/skip_arm_uek6.x
> new file mode 100755
> index 000000000..252cbebb5
> --- /dev/null
> +++ b/test/unittest/usdt/skip_arm_uek6.x
> @@ -0,0 +1,25 @@
> +#!/bin/bash
> +# Licensed under the Universal Permissive License v 1.0 as shown at
> +# http://oss.oracle.com/licenses/upl.
> +#
> +# @@skip: not run directly by test harness
> +#
> +# Tests that depend on USDT argument translation fail on ARM for UEK6.
> +# They're fine for UEK7.  It is unclear in exactly which kernel they
> +# start working.
> +
> +if [[ `uname -m` != "aarch64" ]]; then
> +	exit 0
> +fi
> +
> +read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
> +
> +if [ $MAJOR -gt 5 ]; then
> +	exit 0
> +fi
> +if [ $MAJOR -eq 5 -a $MINOR -ge 10 ]; then
> +	exit 0
> +fi
> +
> +echo "USDT argmap not working on ARM on older kernels"
> +exit 1
> diff --git a/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
> new file mode 120000
> index 000000000..8d462f98f
> --- /dev/null
> +++ b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
> @@ -0,0 +1 @@
> +skip_arm_uek6.x
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.argmap-typed.aarch64.x b/test/unittest/usdt/tst.argmap-typed.aarch64.x
> new file mode 120000
> index 000000000..8d462f98f
> --- /dev/null
> +++ b/test/unittest/usdt/tst.argmap-typed.aarch64.x
> @@ -0,0 +1 @@
> +skip_arm_uek6.x
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
> new file mode 120000
> index 000000000..8d462f98f
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
> @@ -0,0 +1 @@
> +skip_arm_uek6.x
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
> new file mode 120000
> index 000000000..8d462f98f
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
> @@ -0,0 +1 @@
> +skip_arm_uek6.x
> \ No newline at end of file
> diff --git a/test/unittest/usdt/tst.multiprovider-fire.aarch64.x b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> new file mode 120000
> index 000000000..8d462f98f
> --- /dev/null
> +++ b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> @@ -0,0 +1 @@
> +skip_arm_uek6.x
> \ No newline at end of file
> -- 
> 2.43.5
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [DTrace-devel] [PATCH] Get execargs from user space
  2025-03-19  6:32 ` [PATCH] Get execargs from user space eugene.loh
@ 2025-03-19 19:07   ` Kris Van Hees
  2025-03-19 19:19     ` Eugene Loh
  0 siblings, 1 reply; 13+ messages in thread
From: Kris Van Hees @ 2025-03-19 19:07 UTC (permalink / raw)
  To: eugene.loh; +Cc: dtrace, dtrace-devel

On Wed, Mar 19, 2025 at 02:32:30AM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh@oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

I expect this is a failure on arm64 only?  That would make sense since it is
the only arch we currently work with for DTrace that has kernels where the
specific probe_read_kernel/user separation is enforced. 

I'll fix the comment as indicated below while merging.

> ---
>  bpf/bvar_execargs.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/bpf/bvar_execargs.S b/bpf/bvar_execargs.S
> index 1c47cafb2..08844f15f 100644
> --- a/bpf/bvar_execargs.S
> +++ b/bpf/bvar_execargs.S
> @@ -65,7 +65,7 @@ dt_bvar_execargs:
>  	mov	%r1, %r9
>  	mov	%r2, %r8
>  	mov	%r3, %r7
> -	call	BPF_FUNC_probe_read		/* bpf_probe_read(&args, len + 1, arg_start) */
> +	call	BPF_FUNC_probe_read_user	/* bpf_probe_read(&args, len + 1, arg_start) */

comment should also mention bpf_probe_read_user

>  	jne	%r0, 0, .Lerror
>  
>  	/* loop over args and replace '\0' with ' ' */
> -- 
> 2.43.5
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel@oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [DTrace-devel] [PATCH] Get execargs from user space
  2025-03-19 19:07   ` [DTrace-devel] " Kris Van Hees
@ 2025-03-19 19:19     ` Eugene Loh
  0 siblings, 0 replies; 13+ messages in thread
From: Eugene Loh @ 2025-03-19 19:19 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: dtrace, dtrace-devel

On 3/19/25 15:07, Kris Van Hees wrote:

> On Wed, Mar 19, 2025 at 02:32:30AM -0400, eugene.loh--- via DTrace-devel wrote:
>> From: Eugene Loh <eugene.loh@oracle.com>
>>
>> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>
>
> I expect this is a failure on arm64 only?
Right.
> That would make sense since it is
> the only arch we currently work with for DTrace that has kernels where the
> specific probe_read_kernel/user separation is enforced.
>
> I'll fix the comment as indicated below while merging.
Thanks.
>> ---
>>   bpf/bvar_execargs.S | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/bpf/bvar_execargs.S b/bpf/bvar_execargs.S
>> index 1c47cafb2..08844f15f 100644
>> --- a/bpf/bvar_execargs.S
>> +++ b/bpf/bvar_execargs.S
>> @@ -65,7 +65,7 @@ dt_bvar_execargs:
>>   	mov	%r1, %r9
>>   	mov	%r2, %r8
>>   	mov	%r3, %r7
>> -	call	BPF_FUNC_probe_read		/* bpf_probe_read(&args, len + 1, arg_start) */
>> +	call	BPF_FUNC_probe_read_user	/* bpf_probe_read(&args, len + 1, arg_start) */
> comment should also mention bpf_probe_read_user
>
>>   	jne	%r0, 0, .Lerror
>>   
>>   	/* loop over args and replace '\0' with ' ' */
>> -- 
>> 2.43.5
>>
>>
>> _______________________________________________
>> DTrace-devel mailing list
>> DTrace-devel@oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [DTrace-devel] [PATCH] test: Expect USDT argmap to fail on ARM on older kernels
  2025-03-19 19:04   ` [DTrace-devel] " Kris Van Hees
@ 2025-04-11 20:37     ` Kris Van Hees
  0 siblings, 0 replies; 13+ messages in thread
From: Kris Van Hees @ 2025-04-11 20:37 UTC (permalink / raw)
  To: Kris Van Hees; +Cc: eugene.loh, dtrace, dtrace-devel

On Wed, Mar 19, 2025 at 03:04:55PM -0400, Kris Van Hees wrote:
> I am holding off on this patch for the moment, just to get a closer look at
> the potential cause (and some info on how old the kernel needs to be for this
> to fail).  I would expect arg access to be the problem rather than the mapping
> because the mapping is simply something that happens in BPF code.  That should
> not be kernel-dependent.

Reviewed-by: Kris Van Hees <kris.van.hees@oracle.com>

... because the affected kernels are old enough that it does not really matter
    much and I do not want to hold up a fix for this test always showing up as
    failing on those systems.  We'll keep looking at the why in the meantime
    as well.

> 
> On Wed, Mar 19, 2025 at 02:32:29AM -0400, eugene.loh--- via DTrace-devel wrote:
> > From: Eugene Loh <eugene.loh@oracle.com>
> > 
> > Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> > ---
> >  test/unittest/usdt/skip_arm_uek6.x            | 25 +++++++++++++++++++
> >  .../usdt/tst.argmap-typed-partial.aarch64.x   |  1 +
> >  test/unittest/usdt/tst.argmap-typed.aarch64.x |  1 +
> >  .../tst.multiprov-dupprobe-fire.aarch64.x     |  1 +
> >  .../tst.multiprov-dupprobe-shlibs.aarch64.x   |  1 +
> >  .../usdt/tst.multiprovider-fire.aarch64.x     |  1 +
> >  6 files changed, 30 insertions(+)
> >  create mode 100755 test/unittest/usdt/skip_arm_uek6.x
> >  create mode 120000 test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
> >  create mode 120000 test/unittest/usdt/tst.argmap-typed.aarch64.x
> >  create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
> >  create mode 120000 test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
> >  create mode 120000 test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> > 
> > diff --git a/test/unittest/usdt/skip_arm_uek6.x b/test/unittest/usdt/skip_arm_uek6.x
> > new file mode 100755
> > index 000000000..252cbebb5
> > --- /dev/null
> > +++ b/test/unittest/usdt/skip_arm_uek6.x
> > @@ -0,0 +1,25 @@
> > +#!/bin/bash
> > +# Licensed under the Universal Permissive License v 1.0 as shown at
> > +# http://oss.oracle.com/licenses/upl.
> > +#
> > +# @@skip: not run directly by test harness
> > +#
> > +# Tests that depend on USDT argument translation fail on ARM for UEK6.
> > +# They're fine for UEK7.  It is unclear in exactly which kernel they
> > +# start working.
> > +
> > +if [[ `uname -m` != "aarch64" ]]; then
> > +	exit 0
> > +fi
> > +
> > +read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
> > +
> > +if [ $MAJOR -gt 5 ]; then
> > +	exit 0
> > +fi
> > +if [ $MAJOR -eq 5 -a $MINOR -ge 10 ]; then
> > +	exit 0
> > +fi
> > +
> > +echo "USDT argmap not working on ARM on older kernels"
> > +exit 1
> > diff --git a/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
> > new file mode 120000
> > index 000000000..8d462f98f
> > --- /dev/null
> > +++ b/test/unittest/usdt/tst.argmap-typed-partial.aarch64.x
> > @@ -0,0 +1 @@
> > +skip_arm_uek6.x
> > \ No newline at end of file
> > diff --git a/test/unittest/usdt/tst.argmap-typed.aarch64.x b/test/unittest/usdt/tst.argmap-typed.aarch64.x
> > new file mode 120000
> > index 000000000..8d462f98f
> > --- /dev/null
> > +++ b/test/unittest/usdt/tst.argmap-typed.aarch64.x
> > @@ -0,0 +1 @@
> > +skip_arm_uek6.x
> > \ No newline at end of file
> > diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
> > new file mode 120000
> > index 000000000..8d462f98f
> > --- /dev/null
> > +++ b/test/unittest/usdt/tst.multiprov-dupprobe-fire.aarch64.x
> > @@ -0,0 +1 @@
> > +skip_arm_uek6.x
> > \ No newline at end of file
> > diff --git a/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
> > new file mode 120000
> > index 000000000..8d462f98f
> > --- /dev/null
> > +++ b/test/unittest/usdt/tst.multiprov-dupprobe-shlibs.aarch64.x
> > @@ -0,0 +1 @@
> > +skip_arm_uek6.x
> > \ No newline at end of file
> > diff --git a/test/unittest/usdt/tst.multiprovider-fire.aarch64.x b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> > new file mode 120000
> > index 000000000..8d462f98f
> > --- /dev/null
> > +++ b/test/unittest/usdt/tst.multiprovider-fire.aarch64.x
> > @@ -0,0 +1 @@
> > +skip_arm_uek6.x
> > \ No newline at end of file
> > -- 
> > 2.43.5
> > 
> > 
> > _______________________________________________
> > DTrace-devel mailing list
> > DTrace-devel@oss.oracle.com
> > https://oss.oracle.com/mailman/listinfo/dtrace-devel

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [DTrace-devel] [PATCH] test: Account for pid:::entry ucaller being correct
  2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
                   ` (4 preceding siblings ...)
  2025-03-19 18:53 ` [PATCH] test: Account for pid:::entry ucaller being correct Kris Van Hees
@ 2025-04-14 23:33 ` Sam James
  5 siblings, 0 replies; 13+ messages in thread
From: Sam James @ 2025-04-14 23:33 UTC (permalink / raw)
  To: eugene.loh--- via DTrace-devel; +Cc: dtrace, eugene.loh

"eugene.loh--- via DTrace-devel" <dtrace-devel@oss.oracle.com> writes:

> From: Eugene Loh <eugene.loh@oracle.com>
>
> In commit f38bdf9ea ("test: Account for pid:::entry ustack() being correct")
> we accounted for x86-specific heuristics introduced in Linux 6.11 that
> dealt with pid:::entry uprobes firing so early in the function preamble
> that the frame pointer is not yet set and the caller is not (yet)
> correctly identified.
>
> Update a related test to account for the same effect with ucaller.
>
> Signed-off-by: Eugene Loh <eugene.loh@oracle.com>
> ---
>  test/unittest/vars/tst.ucaller.r.p | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100755 test/unittest/vars/tst.ucaller.r.p
>
> diff --git a/test/unittest/vars/tst.ucaller.r.p b/test/unittest/vars/tst.ucaller.r.p
> new file mode 100755
> index 000000000..8e03f110d
> --- /dev/null
> +++ b/test/unittest/vars/tst.ucaller.r.p
> @@ -0,0 +1,28 @@
> +#!/bin/sh
> +
> +# A pid entry probe places a uprobe on the first instruction of a function.
> +# Unfortunately, this is so early in the function preamble that the function
> +# frame pointer has not yet been established and the actual caller of the
> +# traced function is missed.
> +#
> +# In Linux 6.11, x86-specific heuristics are introduced to fix this problem.
> +# See commit cfa7f3d
> +# ("perf,x86: avoid missing caller address in stack traces captured in uprobe")
> +# for both a description of the problem and an explanation of the heuristics.
> +#
> +# Add post processing to these test results to allow for both cases:
> +# caller frame is missing or not missing.
> +
> +if [ $(uname -m) == "x86_64" ]; then
> +        read MAJOR MINOR <<< `uname -r | grep -Eo '^[0-9]+\.[0-9]+' | tr '.' ' '`
> +

<<< is a bashism, but the shebang here is POSIX shell. Please just
change it to bash IMO.

> +        if [ $MAJOR -ge 6 ]; then
> +                if [ $MAJOR -gt 6 -o $MINOR -ge 11 ]; then
> +                        awk '{ sub("myfunc_w", "myfunc_v"); print; }'

Please use gawk instead (see previous commits involving that).

> +                        exit 0
> +                fi
> +        fi
> +fi
> +
> +# Otherwise, just pass the output through.
> +cat

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-04-14 23:33 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-19  6:32 [PATCH] test: Account for pid:::entry ucaller being correct eugene.loh
2025-03-19  6:32 ` [PATCH] Fix dt_bvar_probedesc() for late USDT processes eugene.loh
2025-03-19 18:54   ` Kris Van Hees
2025-03-19  6:32 ` [PATCH] Copy fprobes entry args with BPF helper function eugene.loh
2025-03-19 18:58   ` Kris Van Hees
2025-03-19  6:32 ` [PATCH] test: Expect USDT argmap to fail on ARM on older kernels eugene.loh
2025-03-19 19:04   ` [DTrace-devel] " Kris Van Hees
2025-04-11 20:37     ` Kris Van Hees
2025-03-19  6:32 ` [PATCH] Get execargs from user space eugene.loh
2025-03-19 19:07   ` [DTrace-devel] " Kris Van Hees
2025-03-19 19:19     ` Eugene Loh
2025-03-19 18:53 ` [PATCH] test: Account for pid:::entry ucaller being correct Kris Van Hees
2025-04-14 23:33 ` [DTrace-devel] " Sam James

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox