[PATCH] Change stack_skip to 3

From: Eugene Loh <[email protected]>

For most probes, bpf_get_stack() strips off artificial BPF JIT stack
frames.  In a few cases -- like fprobes and rawtp -- we have to skip
the first few frames manually.  Typically, we should skip 3 frames
(trampoline, clause, and the bpf_get_stack call itself).  Further,
our BPF functions like
    get_bvar_caller()
    get_bvar_stackdepth()
    get_bvar_ucaller()
    get_bvar_ustackdepth()
skip an additional frame to account for that call.

Change stack_skip to 3 for the dt_fbt_fprobe and dt_rawtp providers.

This raises the thorny question of testing.  Until recently, the
stack_skip value we had been using -- 4, not 3 -- worked since it was
chosen empirically:  there was a bug in the kernel that added an extra
frame.  But now, the kernel situation is muddled.  The bug has been
fixed in recent kernels, but only for x86.  See

https://github.com/torvalds/linux/commit/6d08340d1e354787d6c65a8c3cdd4d41ffb8a5ed
Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"

A broader fix, for non-x86, is also expected.  See the thread that
starts at:
https://lore.kernel.org/all/[email protected]/

So, for now, XFAIL tests on "unfixed" (older) kernels.

Orabug: 38776929
Signed-off-by: Eugene Loh <[email protected]>
---
 libdtrace/dt_prov_fbt.c                   |  2 +-
 libdtrace/dt_prov_rawtp.c                 |  2 +-
 test/unittest/funcs/stack/tst.asgn_dvar.x |  1 +
 test/unittest/funcs/stack/tst.asgn_gvar.x |  1 +
 test/unittest/funcs/stack/tst.asgn_lvar.x |  1 +
 test/unittest/funcs/stack/tst.asgn_tvar.x |  1 +
 test/unittest/funcs/stack/tst.ref_addrs.x |  1 +
 test/unittest/printf/tst.stack.x          |  1 +
 test/unittest/stack/tst.stack.x           |  1 +
 test/utils/stack_skip_test.sh             | 33 +++++++++++++++++++++++
 10 files changed, 42 insertions(+), 2 deletions(-)
 create mode 120000 test/unittest/funcs/stack/tst.asgn_dvar.x
 create mode 120000 test/unittest/funcs/stack/tst.asgn_gvar.x
 create mode 120000 test/unittest/funcs/stack/tst.asgn_lvar.x
 create mode 120000 test/unittest/funcs/stack/tst.asgn_tvar.x
 create mode 120000 test/unittest/funcs/stack/tst.ref_addrs.x
 create mode 120000 test/unittest/printf/tst.stack.x
 create mode 120000 test/unittest/stack/tst.stack.x
 create mode 100755 test/utils/stack_skip_test.sh

diff --git a/libdtrace/dt_prov_fbt.c b/libdtrace/dt_prov_fbt.c
index 8dbf9740a..4df621657 100644
--- a/libdtrace/dt_prov_fbt.c
+++ b/libdtrace/dt_prov_fbt.c
@@ -736,7 +736,7 @@ reject:
 dt_provimpl_t	dt_fbt_fprobe = {
 	.name		= prvname,
 	.prog_type	= BPF_PROG_TYPE_TRACING,
-	.stack_skip	= 4,
+	.stack_skip	= 3,
 	.populate	= &populate,
 	.provide	= &provide,
 	.load_prog	= &fprobe_prog_load,
diff --git a/libdtrace/dt_prov_rawtp.c b/libdtrace/dt_prov_rawtp.c
index e3269f4b3..27a992d2e 100644
--- a/libdtrace/dt_prov_rawtp.c
+++ b/libdtrace/dt_prov_rawtp.c
@@ -296,7 +296,7 @@ use_alt:
 dt_provimpl_t	dt_rawtp = {
 	.name		= prvname,
 	.prog_type	= BPF_PROG_TYPE_RAW_TRACEPOINT,
-	.stack_skip	= 4,
+	.stack_skip	= 3,
 	.populate	= &populate,
 	.load_prog	= &dt_bpf_prog_load,
 	.trampoline	= &trampoline,
diff --git a/test/unittest/funcs/stack/tst.asgn_dvar.x b/test/unittest/funcs/stack/tst.asgn_dvar.x
new file mode 120000
index 000000000..0edb412f0
--- /dev/null
+++ b/test/unittest/funcs/stack/tst.asgn_dvar.x
@@ -0,0 +1 @@
+../../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/funcs/stack/tst.asgn_gvar.x b/test/unittest/funcs/stack/tst.asgn_gvar.x
new file mode 120000
index 000000000..0edb412f0
--- /dev/null
+++ b/test/unittest/funcs/stack/tst.asgn_gvar.x
@@ -0,0 +1 @@
+../../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/funcs/stack/tst.asgn_lvar.x b/test/unittest/funcs/stack/tst.asgn_lvar.x
new file mode 120000
index 000000000..0edb412f0
--- /dev/null
+++ b/test/unittest/funcs/stack/tst.asgn_lvar.x
@@ -0,0 +1 @@
+../../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/funcs/stack/tst.asgn_tvar.x b/test/unittest/funcs/stack/tst.asgn_tvar.x
new file mode 120000
index 000000000..0edb412f0
--- /dev/null
+++ b/test/unittest/funcs/stack/tst.asgn_tvar.x
@@ -0,0 +1 @@
+../../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/funcs/stack/tst.ref_addrs.x b/test/unittest/funcs/stack/tst.ref_addrs.x
new file mode 120000
index 000000000..0edb412f0
--- /dev/null
+++ b/test/unittest/funcs/stack/tst.ref_addrs.x
@@ -0,0 +1 @@
+../../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/printf/tst.stack.x b/test/unittest/printf/tst.stack.x
new file mode 120000
index 000000000..b5ac46b84
--- /dev/null
+++ b/test/unittest/printf/tst.stack.x
@@ -0,0 +1 @@
+../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/unittest/stack/tst.stack.x b/test/unittest/stack/tst.stack.x
new file mode 120000
index 000000000..b5ac46b84
--- /dev/null
+++ b/test/unittest/stack/tst.stack.x
@@ -0,0 +1 @@
+../../utils/stack_skip_test.sh
\ No newline at end of file
diff --git a/test/utils/stack_skip_test.sh b/test/utils/stack_skip_test.sh
new file mode 100755
index 000000000..8ebfd29ff
--- /dev/null
+++ b/test/utils/stack_skip_test.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+# Older kernels had a bug in which the leaf frame was replicated.
+#
+# This bug was fixed on x86_64.  See
+#   https://github.com/torvalds/linux/commit/6d08340d1e354787d6c65a8c3cdd4d41ffb8a5ed
+#   Revert "perf/x86: Always store regs->ip in perf_callchain_kernel()"
+#
+# A broader fix, for non-x86, is also expected.  See the thread that starts at:
+#   https://lore.kernel.org/all/[email protected]/
+#
+# Use this as a .x file to mark tests as "XFAIL" for older kernels with the
+# superfluous frame (and therefore exposing a BPF JIT frame, appearing as an
+# untranslated 0xffff address).  On older kernels, it is also possible for fbt
+# to be implemented with kprobes rather than fprobes;  in this case, the .x
+# issues no concerns and the tests are expected to PASS.
+
+$dtrace -c test/triggers/periodic_output -qn '
+fbt::hrtimer_nanosleep:entry
+{
+        stack(1);
+        exit(0);
+}
+
+ERROR
+{
+        exit(1);
+}' | grep -q 0xffff
+if [ $? -eq 0 ]; then
+	echo stack appears to have superfluous frame
+	exit 1
+fi
+exit 0
-- 
2.47.3