[DTrace-devel] [PATCH] Fix flowindent for "return" probes with multiple statements
eugene.loh at oracle.com
eugene.loh at oracle.com
Sat Feb 21 03:46:18 UTC 2026
From: Eugene Loh <eugene.loh at oracle.com>
The flowindent algorithm should change indentation based only on the
number of levels of entry and return. So, if an entry or return probe
has multiple statements, the indentation should change only for the
first statement.
Such logic is used for entry probes. Similar logic should be used for
return probes.
Add a test to cover the case of multiple statements per entry and return
probes. Also have it check a greater number of indentations. Since
the new test supersedes the old ones, remove the old tests, which
were fickle anyhow. The new test can still fail -- the flowindent
algorithm is not completely robust -- so allow reinvocation in case of
failure. The intermittent failures of flowindent tests have long been a
testing irritant. Now, with reinvocation, testing shows no failures in
100 iterations on over a dozen systems.
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_consume.c | 11 +-
test/unittest/options/tst.F.aarch64.r | 1 -
test/unittest/options/tst.F.d | 36 ----
test/unittest/options/tst.F.r.p | 1 -
test/unittest/options/tst.F.x86_64.r | 1 -
.../unittest/options/tst.flowindent.aarch64.r | 160 ----------------
test/unittest/options/tst.flowindent.d | 36 ----
test/unittest/options/tst.flowindent.r | 1 +
test/unittest/options/tst.flowindent.r.p | 7 -
test/unittest/options/tst.flowindent.sh | 171 ++++++++++++++++++
test/unittest/options/tst.flowindent.x86_64.r | 160 ----------------
11 files changed, 176 insertions(+), 409 deletions(-)
delete mode 120000 test/unittest/options/tst.F.aarch64.r
delete mode 100644 test/unittest/options/tst.F.d
delete mode 120000 test/unittest/options/tst.F.r.p
delete mode 120000 test/unittest/options/tst.F.x86_64.r
delete mode 100644 test/unittest/options/tst.flowindent.aarch64.r
delete mode 100644 test/unittest/options/tst.flowindent.d
create mode 100644 test/unittest/options/tst.flowindent.r
delete mode 100755 test/unittest/options/tst.flowindent.r.p
create mode 100755 test/unittest/options/tst.flowindent.sh
delete mode 100644 test/unittest/options/tst.flowindent.x86_64.r
diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index 8e9b01cb6..d3cdcc2fc 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -475,17 +475,14 @@ dt_flowindent(dtrace_hdl_t *dtp, dtrace_probedata_t *data, dtrace_id_t lastprid,
}
/*
- * If we're going to indent this, we need to check the ID of our last
- * call. If we're looking at the same probe ID but a different STID,
- * we _don't_ want to indent. (Yes, there are some minor holes in
- * this scheme -- it's a heuristic.)
+ * If we're looking at the same probe ID but a different STID,
+ * we do NOT want to change indentation. (This heuristic could fail.)
*/
- if (flow == DTRACEFLOW_ENTRY) {
+ if (flow != DTRACEFLOW_NONE)
if (stid != laststid && pd->id == lastprid)
flow = DTRACEFLOW_NONE;
- }
- if (flow == DTRACEFLOW_ENTRY || flow == DTRACEFLOW_RETURN)
+ if (flow != DTRACEFLOW_NONE)
data->dtpda_prefix = str;
else
data->dtpda_prefix = "| ";
diff --git a/test/unittest/options/tst.F.aarch64.r b/test/unittest/options/tst.F.aarch64.r
deleted file mode 120000
index 802f9b829..000000000
--- a/test/unittest/options/tst.F.aarch64.r
+++ /dev/null
@@ -1 +0,0 @@
-tst.flowindent.aarch64.r
\ No newline at end of file
diff --git a/test/unittest/options/tst.F.d b/test/unittest/options/tst.F.d
deleted file mode 100644
index 90430868e..000000000
--- a/test/unittest/options/tst.F.d
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Oracle Linux DTrace.
- * 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: The -F option enables entry/return matching output.
- *
- * SECTION: Options and Tunables/Consumer Options
- */
-
-/* @@runtest-opts: -FZ -xswitchrate=1s */
-/* @@timeout: 15 */
-/* @@trigger: readwholedir */
-
-BEGIN
-{
- i = 0;
- j = 0;
-}
-
-syscall::read:
-/pid == $target/
-{
- printf("syscall: %d\n", i++);
-}
-
-fbt:vmlinux:SyS_read:,
-fbt:vmlinux:__arm64_sys_read:,
-fbt:vmlinux:__x64_sys_read:
-/pid == $target/
-{
- printf("fbt: %d\n", j++);
-}
diff --git a/test/unittest/options/tst.F.r.p b/test/unittest/options/tst.F.r.p
deleted file mode 120000
index 6260a34c1..000000000
--- a/test/unittest/options/tst.F.r.p
+++ /dev/null
@@ -1 +0,0 @@
-tst.flowindent.r.p
\ No newline at end of file
diff --git a/test/unittest/options/tst.F.x86_64.r b/test/unittest/options/tst.F.x86_64.r
deleted file mode 120000
index 509d38a69..000000000
--- a/test/unittest/options/tst.F.x86_64.r
+++ /dev/null
@@ -1 +0,0 @@
-tst.flowindent.x86_64.r
\ No newline at end of file
diff --git a/test/unittest/options/tst.flowindent.aarch64.r b/test/unittest/options/tst.flowindent.aarch64.r
deleted file mode 100644
index 7e745280e..000000000
--- a/test/unittest/options/tst.flowindent.aarch64.r
+++ /dev/null
@@ -1,160 +0,0 @@
- => read syscall: 40
-
- -> __arm64_sys_read fbt: 40
-
- <- __arm64_sys_read fbt: 41
-
- <= read syscall: 41
-
- => read syscall: 42
-
- -> __arm64_sys_read fbt: 42
-
- <- __arm64_sys_read fbt: 43
-
- <= read syscall: 43
-
- => read syscall: 44
-
- -> __arm64_sys_read fbt: 44
-
- <- __arm64_sys_read fbt: 45
-
- <= read syscall: 45
-
- => read syscall: 46
-
- -> __arm64_sys_read fbt: 46
-
- <- __arm64_sys_read fbt: 47
-
- <= read syscall: 47
-
- => read syscall: 48
-
- -> __arm64_sys_read fbt: 48
-
- <- __arm64_sys_read fbt: 49
-
- <= read syscall: 49
-
- => read syscall: 50
-
- -> __arm64_sys_read fbt: 50
-
- <- __arm64_sys_read fbt: 51
-
- <= read syscall: 51
-
- => read syscall: 52
-
- -> __arm64_sys_read fbt: 52
-
- <- __arm64_sys_read fbt: 53
-
- <= read syscall: 53
-
- => read syscall: 54
-
- -> __arm64_sys_read fbt: 54
-
- <- __arm64_sys_read fbt: 55
-
- <= read syscall: 55
-
- => read syscall: 56
-
- -> __arm64_sys_read fbt: 56
-
- <- __arm64_sys_read fbt: 57
-
- <= read syscall: 57
-
- => read syscall: 58
-
- -> __arm64_sys_read fbt: 58
-
- <- __arm64_sys_read fbt: 59
-
- <= read syscall: 59
-
- => read syscall: 60
-
- -> __arm64_sys_read fbt: 60
-
- <- __arm64_sys_read fbt: 61
-
- <= read syscall: 61
-
- => read syscall: 62
-
- -> __arm64_sys_read fbt: 62
-
- <- __arm64_sys_read fbt: 63
-
- <= read syscall: 63
-
- => read syscall: 64
-
- -> __arm64_sys_read fbt: 64
-
- <- __arm64_sys_read fbt: 65
-
- <= read syscall: 65
-
- => read syscall: 66
-
- -> __arm64_sys_read fbt: 66
-
- <- __arm64_sys_read fbt: 67
-
- <= read syscall: 67
-
- => read syscall: 68
-
- -> __arm64_sys_read fbt: 68
-
- <- __arm64_sys_read fbt: 69
-
- <= read syscall: 69
-
- => read syscall: 70
-
- -> __arm64_sys_read fbt: 70
-
- <- __arm64_sys_read fbt: 71
-
- <= read syscall: 71
-
- => read syscall: 72
-
- -> __arm64_sys_read fbt: 72
-
- <- __arm64_sys_read fbt: 73
-
- <= read syscall: 73
-
- => read syscall: 74
-
- -> __arm64_sys_read fbt: 74
-
- <- __arm64_sys_read fbt: 75
-
- <= read syscall: 75
-
- => read syscall: 76
-
- -> __arm64_sys_read fbt: 76
-
- <- __arm64_sys_read fbt: 77
-
- <= read syscall: 77
-
- => read syscall: 78
-
- -> __arm64_sys_read fbt: 78
-
- <- __arm64_sys_read fbt: 79
-
- <= read syscall: 79
-
diff --git a/test/unittest/options/tst.flowindent.d b/test/unittest/options/tst.flowindent.d
deleted file mode 100644
index 1ce1e26f7..000000000
--- a/test/unittest/options/tst.flowindent.d
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Oracle Linux DTrace.
- * 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: The -xflowindent option enables entry/return matching output.
- *
- * SECTION: Options and Tunables/Consumer Options
- */
-
-/* @@runtest-opts: -xflowindent -Z -xswitchrate=1s */
-/* @@timeout: 15 */
-/* @@trigger: readwholedir */
-
-BEGIN
-{
- i = 0;
- j = 0;
-}
-
-syscall::read:
-/pid == $target/
-{
- printf("syscall: %d\n", i++);
-}
-
-fbt:vmlinux:SyS_read:,
-fbt:vmlinux:__arm64_sys_read:,
-fbt:vmlinux:__x64_sys_read:
-/pid == $target/
-{
- printf("fbt: %d\n", j++);
-}
diff --git a/test/unittest/options/tst.flowindent.r b/test/unittest/options/tst.flowindent.r
new file mode 100644
index 000000000..2e9ba477f
--- /dev/null
+++ b/test/unittest/options/tst.flowindent.r
@@ -0,0 +1 @@
+success
diff --git a/test/unittest/options/tst.flowindent.r.p b/test/unittest/options/tst.flowindent.r.p
deleted file mode 100755
index 1c6489608..000000000
--- a/test/unittest/options/tst.flowindent.r.p
+++ /dev/null
@@ -1,7 +0,0 @@
-#!/usr/bin/gawk -f
-
-# Report only from 40 to 80.
-BEGIN { report = 0 }
-$NF == 40 { report = 1 }
-$NF == 80 { report = 0 }
-report == 1 { print }
diff --git a/test/unittest/options/tst.flowindent.sh b/test/unittest/options/tst.flowindent.sh
new file mode 100755
index 000000000..bcd5900bf
--- /dev/null
+++ b/test/unittest/options/tst.flowindent.sh
@@ -0,0 +1,171 @@
+#!/bin/bash
+#
+# Oracle Linux DTrace.
+# Copyright (c) 2026, 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.
+#
+
+# @@reinvoke-failure: 1
+
+dtrace=$1
+
+DIRNAME="$tmpdir/flowindent.$$.$RANDOM"
+mkdir -p $DIRNAME
+cd $DIRNAME
+
+# Create the trigger. Produce the following call stack 3 times:
+# main()
+# -> foo1()
+# -> foo2()
+# -> foo3()
+# -> foo4()
+# => syscall::write
+# -> fbt:vmlinux:__*_sys_write
+
+cat << EOF > main.c
+#include <stdio.h>
+
+int foo4(int x) {
+ printf("%d", x);
+ fflush(stdout); /* prevent buffering the printf */
+ return x + 1;
+}
+
+int foo3(int x) { return foo4(x) + 1; }
+int foo2(int x) { return foo3(x) + 1; }
+int foo1(int x) { return foo2(x) + 1; }
+
+int main(int c, char **v) {
+ int iter, value = 0;
+ for (iter = 0; iter < 3; iter++) {
+ value = foo1(value);
+ }
+ return 0;
+}
+EOF
+
+$CC main.c
+if [ $? -ne 0 ]; then
+ echo ERROR: compilation
+ exit 1
+fi
+
+# Run with flowindent, equivalently specifying either -F or -xflowindent.
+# Ignore the trigger (a.out) output.
+# Use a long switchrate to reduce the chances of interruption from dt_consume_cpu(),
+# which resets the flowindent algorithm.
+
+for my_opt in "-F" "-xflowindent" ; do
+ $dtrace $dt_flags -c "./a.out > /dev/null" $my_opt -xswitchrate=1s -Zqn '
+ /*
+ * Probe on each foo* entry and return.
+ * Report the cpu for more debugging in case of failure.
+ */
+ pid$target::foo*:entry,
+ pid$target::foo*:return { printf("%d foo\n", cpu); }
+
+ /*
+ * Add a second statement for those foo* probes.
+ */
+ pid$target::foo*:entry,
+ pid$target::foo*:return { printf("%d foo again\n", cpu); }
+
+ /*
+ * And add some syscall and kernel probes.
+ */
+ syscall::write:,
+ fbt:vmlinux:__arm64_sys_write:,
+ fbt:vmlinux:__x64_sys_write:
+ /pid == $target/
+ {
+ printf("%d write\n", cpu);
+ }' >> D.out 2>&1
+
+ if [ $? -ne 0 ]; then
+ echo ERROR: D script
+ exit 1
+ fi
+done
+
+# Construct the file to compare D output. Note:
+# - Each entry/return changes indentation only once,
+# even if there are multiple statements per probe.
+# - The syscall entry/return uses => and <=.
+# Each iteration happens to have 20 lines of output.
+# There are 2 runs (-F and -xflowindent).
+# Each run has 3 iterations, but we will discard the first one.
+# So the cmp file has 2*(3-1) = 4 iterations in total.
+
+for iter in `seq 4`; do
+cat << EOF >> D.cmp
+ -> foo1 foo
+ | foo1:entry foo again
+ -> foo2 foo
+ | foo2:entry foo again
+ -> foo3 foo
+ | foo3:entry foo again
+ -> foo4 foo
+ | foo4:entry foo again
+ => write write
+ -> write write
+ <- write write
+ <= write write
+ <- foo4 foo
+ | foo4:return foo again
+ <- foo3 foo
+ | foo3:return foo again
+ <- foo2 foo
+ | foo2:return foo again
+ <- foo1 foo
+| foo1:return foo again
+EOF
+done
+
+# Postprocess the D output.
+
+awk '
+# If there is a break in the output, restart the line number.
+NF == 0 || /FUNCTION/ { lineno = 0; next }
+
+# Increment the line number.
+{ lineno++; }
+
+# We do not want to be interrupted by dt_consume_cpu() calls.
+# Therefore we use a long switchrate to throttle the consumer.
+# Since throttling does not start immediately, however, we also
+# discard the first iteration of output in each run. We saw
+# earlier there are 20 lines of output per iteration.
+lineno <= 20 { next }
+
+# Then print. The cpu IDs are removed for the sake of comparison.
+# The cpu IDs were printed in the first place since it is possible
+# that the trigger migrates from one cpu to another, disrupting the
+# flowindent algorithm. That is unlikely for such a short-running
+# program. Still, if we get unlucky, at least there is more data
+# for debugging. If migration proves to become an issue, consider
+# using taskset to bind the trigger to a cpu.
+{
+ sub(/__arm64_sys_write/, "write "); # scrub arm64 label
+ sub( /__x64_sys_write/, "write " ); # scrub x64 label
+ sub( /[0-9]+ foo/, "foo"); # remove cpu ID
+ sub( /[0-9]+ write/, "write"); # remove cpu ID
+ print;
+}' D.out > D.post
+if [ $? -ne 0 ]; then
+ echo ERROR: awk
+ exit 1
+fi
+
+# Report.
+
+if diff -q D.cmp D.post ; then
+ echo success
+ exit 0
+fi
+cat D.out
+echo ====
+diff D.cmp D.post
+echo ====
+
+exit 1
diff --git a/test/unittest/options/tst.flowindent.x86_64.r b/test/unittest/options/tst.flowindent.x86_64.r
deleted file mode 100644
index 53800726a..000000000
--- a/test/unittest/options/tst.flowindent.x86_64.r
+++ /dev/null
@@ -1,160 +0,0 @@
- => read syscall: 40
-
- -> __x64_sys_read fbt: 40
-
- <- __x64_sys_read fbt: 41
-
- <= read syscall: 41
-
- => read syscall: 42
-
- -> __x64_sys_read fbt: 42
-
- <- __x64_sys_read fbt: 43
-
- <= read syscall: 43
-
- => read syscall: 44
-
- -> __x64_sys_read fbt: 44
-
- <- __x64_sys_read fbt: 45
-
- <= read syscall: 45
-
- => read syscall: 46
-
- -> __x64_sys_read fbt: 46
-
- <- __x64_sys_read fbt: 47
-
- <= read syscall: 47
-
- => read syscall: 48
-
- -> __x64_sys_read fbt: 48
-
- <- __x64_sys_read fbt: 49
-
- <= read syscall: 49
-
- => read syscall: 50
-
- -> __x64_sys_read fbt: 50
-
- <- __x64_sys_read fbt: 51
-
- <= read syscall: 51
-
- => read syscall: 52
-
- -> __x64_sys_read fbt: 52
-
- <- __x64_sys_read fbt: 53
-
- <= read syscall: 53
-
- => read syscall: 54
-
- -> __x64_sys_read fbt: 54
-
- <- __x64_sys_read fbt: 55
-
- <= read syscall: 55
-
- => read syscall: 56
-
- -> __x64_sys_read fbt: 56
-
- <- __x64_sys_read fbt: 57
-
- <= read syscall: 57
-
- => read syscall: 58
-
- -> __x64_sys_read fbt: 58
-
- <- __x64_sys_read fbt: 59
-
- <= read syscall: 59
-
- => read syscall: 60
-
- -> __x64_sys_read fbt: 60
-
- <- __x64_sys_read fbt: 61
-
- <= read syscall: 61
-
- => read syscall: 62
-
- -> __x64_sys_read fbt: 62
-
- <- __x64_sys_read fbt: 63
-
- <= read syscall: 63
-
- => read syscall: 64
-
- -> __x64_sys_read fbt: 64
-
- <- __x64_sys_read fbt: 65
-
- <= read syscall: 65
-
- => read syscall: 66
-
- -> __x64_sys_read fbt: 66
-
- <- __x64_sys_read fbt: 67
-
- <= read syscall: 67
-
- => read syscall: 68
-
- -> __x64_sys_read fbt: 68
-
- <- __x64_sys_read fbt: 69
-
- <= read syscall: 69
-
- => read syscall: 70
-
- -> __x64_sys_read fbt: 70
-
- <- __x64_sys_read fbt: 71
-
- <= read syscall: 71
-
- => read syscall: 72
-
- -> __x64_sys_read fbt: 72
-
- <- __x64_sys_read fbt: 73
-
- <= read syscall: 73
-
- => read syscall: 74
-
- -> __x64_sys_read fbt: 74
-
- <- __x64_sys_read fbt: 75
-
- <= read syscall: 75
-
- => read syscall: 76
-
- -> __x64_sys_read fbt: 76
-
- <- __x64_sys_read fbt: 77
-
- <= read syscall: 77
-
- => read syscall: 78
-
- -> __x64_sys_read fbt: 78
-
- <- __x64_sys_read fbt: 79
-
- <= read syscall: 79
-
--
2.47.3
More information about the DTrace-devel
mailing list