[DTrace-devel] [PATCH v4] dtrace provider: add a predicate against the current tgid

[Nick Alcock]

If we don't put a predicate on, BEGIN/END probes fire in every running
dtrace at the same time, messing up the activity state of all but the one it
was meant to fire in and often causing the others to fail to exit on exit()
(they hang until ended by some other means, like an interrupt or -c
termination).

Thankfully the -xcpu run-BEGIN/END-in-a-thread complexities can be ignored
because we can match on DTrace's tgid instead of its PID (thread ID),
which will always catch exactly our BEGIN/END firings and no-one else's.

(In theory this might cause trouble if you run multiple consumers in
different threads in the same process, but that's not going to work as it
is, and has never been considered a sane thing to do.)

One test is perturbed by the new changing-every-invocation value this
adds to the BEGIN prologue, and is suitably adjusted.

Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
---
 libdtrace/dt_prov_dtrace.c                 | 19 ++++++++-
 test/unittest/dtrace-util/tst.DisOption.sh | 45 +++++++++++++++++++---
 2 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/libdtrace/dt_prov_dtrace.c b/libdtrace/dt_prov_dtrace.c
index 307ad558874f3..3e6ae88a37848 100644
--- a/libdtrace/dt_prov_dtrace.c
+++ b/libdtrace/dt_prov_dtrace.c
@@ -93,7 +93,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 	}
 
 	/*
-	 * The BEGIN probe should only run when the activity state is INACTIVE.
+	 * The BEGIN probe should only run when the activity state is INACTIVE,
+	 * for this process's PID (TGID).
 	 * At the end of the trampoline (after executing any clauses), the
 	 * state must be advanced to the next state (INACTIVE -> ACTIVE, or if
 	 * there was an exit() action in the clause, DRAINING -> STOPPED).
@@ -102,7 +103,8 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 	 * on in state[DT_STATE_BEGANON] to ensure that we know which trace
 	 * data buffer to process first.
 	 *
-	 * The END probe should only run when the activity state is DRAINING.
+	 * The END probe should only run when the activity state is DRAINING,
+	 * for this process's PID (TGID).
 	 * At the end of the trampoline (after executing any clauses), the
 	 * state must be advanced to the next state (DRAINING -> STOPPED).
 	 *
@@ -118,6 +120,19 @@ static int trampoline(dt_pcb_t *pcb, uint_t exitlbl)
 		key = DT_STATE_ENDEDON;
 	}
 
+	/*
+	 * Retrieve the PID (TGID) of the process that caused the probe to fire,
+	 * and check it against the PID we're meant to be using.  Do it before
+	 * the trampoline to minimize the cost of pointless firings in other
+	 * tracers, even though this means preserving the context in %r1 around
+	 * the call.
+	 */
+	emit(dlp, BPF_MOV_REG(BPF_REG_6, BPF_REG_1));
+	emit(dlp, BPF_CALL_HELPER(BPF_FUNC_get_current_pid_tgid));
+	emit(dlp, BPF_ALU64_IMM(BPF_RSH, BPF_REG_0, 32));
+	emit(dlp, BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, getpid(), pcb->pcb_exitlbl));
+	emit(dlp, BPF_MOV_REG(BPF_REG_1, BPF_REG_6));
+
 	dt_cg_tramp_prologue_act(pcb, act);
 
 	/*
diff --git a/test/unittest/dtrace-util/tst.DisOption.sh b/test/unittest/dtrace-util/tst.DisOption.sh
index 1ad3f35f4fb8e..1f09a0869f664 100755
--- a/test/unittest/dtrace-util/tst.DisOption.sh
+++ b/test/unittest/dtrace-util/tst.DisOption.sh
@@ -1,13 +1,37 @@
 #!/bin/bash
 #
 # Oracle Linux DTrace.
-# Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+# Copyright (c) 2022, 2024, 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: That the dtrace -xdisasm option can control the listings.
 
+# We create simple D scripts (as well as their expected output).
+#
+# We run with different settings of -xdisasm:
+#
+#                   # default
+#     -xdisasm=1    # listing mode 0
+#     -xdisasm=2    # listing mode 1
+#     -xdisasm=4    # listing mode 2
+#     -xdisasm=8    # listing mode 3
+#     -xdisasm=15   # all
+#
+# We sanity check stdout and stderr.
+#
+# We check that the default is equivalent to -xdisasm=1.
+#
+# We split the -xdisasm=15 output into different pieces, and check
+# that the pieces correspond to the -xdisasm=1, 2, 4, and 8 output.
+#
+# One problem is that, since the checks are between different invocations
+# of dtrace, some of the output may change -- specifically:
+#     * the BOOTTM value
+#     * the tgid value that the predicate checks
+# So, we filter those run-dependent items out.
+
 dtrace=$1
 
 DIRNAME="$tmpdir/DisOption.$$.$RANDOM"
@@ -36,13 +60,14 @@ EOF
 # run dtrace for various xdisasm settings
 
 function run_dtrace() {
-    $dtrace $dt_flags $2 -Sq -s D1.d -s D2.d > $1.out 2> $1.tmp
+    $dtrace $dt_flags $2 -Sq -s D1.d -s D2.d > $1.out 2> $1.err
     if [ $? -ne 0 ]; then
         echo DTrace failed $2
         cat $1.out $1.err
         exit 1
     fi
-    # Avoid differenced due to different BOOTTM values.
+
+    # Avoid differences due to different BOOTTM values.
     awk '/: 18 [0-9] 0 / && /lddw/ {
 	    sub(/0x[0-9a-f]+/, 0x0);
 	    sub(/[0-9a-f]{8}/, "00000000");
@@ -58,7 +83,17 @@ function run_dtrace() {
 	    print;
 	    next;
 	}
-	{ print; }' $1.tmp > $1.err
+	{ print; }' $1.err > $1.tmp
+    mv $1.tmp $1.err
+
+    # Avoid differences due to different tgid values in predicates.
+    # If we see bpf_get_current_pid_tgid, omit the 3rd line if it's
+    # "jne %r0, ..." since the check value will change from run to run.
+    awk '/call bpf_get_current_pid_tgid/ { ncount = 0 }
+    	 { ncount++ }
+	 ncount == 3 && /^[ :0-9a-f]* jne *%r0, / { next }
+	{ print; }' $1.err > $1.tmp
+    mv $1.tmp $1.err
 }
 
 run_dtrace def               # default
@@ -68,7 +103,7 @@ run_dtrace   2 -xdisasm=4    # listing mode 2
 run_dtrace   3 -xdisasm=8    # listing mode 3
 run_dtrace all -xdisasm=15   # all
 
-# check individual D stdout and stderr files
+# sanity check individual D stdout and stderr files
 
 for x in def 0 1 2 3 all; do
     if [ `cat $x.err | wc -l` -lt 10 ]; then
-- 
2.43.0.272.gce700b77fd