[DTrace-devel] [PATCH 2/2] Use default action ONLY if the clause is empty
eugene.loh at oracle.com
eugene.loh at oracle.com
Tue Sep 15 19:26:36 PDT 2020
From: Eugene Loh <eugene.loh at oracle.com>
If a clause is empty, use the default clause. This was working.
On the other hand, if a clause is not empty, do not use the default
clause -- even if the clause traces no data! This was not working
in DTv2. Arguably, the new behavior was more sensible, but it
breaks precedent, including in some very important and common
cases, such as clauses that simply manipulate D variables.
Restore the legacy behavior.
Do not modify the prologue. A few instructions in the prologue could
be eliminated for clauses that will not write the output buffer, but
the instructions are inconsequential and would be difficult to eliminate.
The test test/unittest/funcs/tst.default.d was too mild to catch
this problem. Add test/unittest/actions/default/tst.default.d .
Orabug: 31772287
Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
---
libdtrace/dt_cg.c | 104 +++++++++++---------
test/unittest/actions/default/tst.default.d | 40 ++++++++
test/unittest/actions/default/tst.default.r | 7 ++
3 files changed, 105 insertions(+), 46 deletions(-)
create mode 100644 test/unittest/actions/default/tst.default.d
create mode 100644 test/unittest/actions/default/tst.default.r
diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
index 30dd7e1a..48b5b289 100644
--- a/libdtrace/dt_cg.c
+++ b/libdtrace/dt_cg.c
@@ -270,67 +270,79 @@ static void
dt_cg_epilogue(dt_pcb_t *pcb)
{
dt_irlist_t *dlp = &pcb->pcb_ir;
- dt_ident_t *buffers = dt_dlib_get_map(pcb->pcb_hdl, "buffers");
struct bpf_insn instr;
- assert(buffers != NULL);
-
/*
- * rc = dctx->mst->fault; // lddw %r0, [%fp + DT_STK_DCTX]
- * // lddw %r0, [%r0 + DCTX_MST]
- * // lddw %r0, [%r0 + DMST_FAULT]
- * if (rc != 0)
- * goto exit; // jne %r0, 0, pcb->pcb_exitlbl
+ * Output the buffer if:
+ * - data-recording action, or
+ * - default action (no clause specified)
*/
- TRACE_REGSET("Epilogue: Begin");
- instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_STK_DCTX);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DCTX_MST);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DMST_FAULT);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, 0, pcb->pcb_exitlbl);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ if (pcb->pcb_stmt->dtsd_clauseflags & DT_CLSFLAG_DATAREC ||
+ pcb->pcb_dret->dn_acts == NULL) {
+ dt_ident_t *buffers = dt_dlib_get_map(pcb->pcb_hdl, "buffers");
+
+ assert(buffers != NULL);
+
+ /*
+ * rc = dctx->mst->fault; // lddw %r0, [%fp + DT_STK_DCTX]
+ * // lddw %r0, [%r0 + DCTX_MST]
+ * // lddw %r0, [%r0 + DMST_FAULT]
+ * if (rc != 0)
+ * goto exit; // jne %r0, 0, pcb->pcb_exitlbl
+ */
+ TRACE_REGSET("Epilogue: Begin");
+ instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_FP, DT_STK_DCTX);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DCTX_MST);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_LOAD(BPF_DW, BPF_REG_0, BPF_REG_0, DMST_FAULT);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, 0, pcb->pcb_exitlbl);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+
+ /*
+ * bpf_perf_event_output(dctx->ctx, &buffers, BPF_F_CURRENT_CPU,
+ * buf - 4, bufoff + 4);
+ * // lddw %r1, [%fp + DT_STK_DCTX]
+ * // lddw %r1, [%r1 + DCTX_CTX]
+ * // lddw %r2, &buffers
+ * // lddw %r3, BPF_F_CURRENT_CPU
+ * // mov %r4, %r9
+ * // add %r4, -4
+ * // mov %r5, pcb->pcb_bufoff
+ * // add %r4, 4
+ * // call bpf_perf_event_output
+ *
+ */
+ instr = BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_1, DCTX_CTX);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ dt_cg_xsetx(dlp, buffers, DT_LBL_NONE, BPF_REG_2, buffers->di_id);
+ dt_cg_xsetx(dlp, NULL, DT_LBL_NONE, BPF_REG_3, BPF_F_CURRENT_CPU);
+ instr = BPF_MOV_REG(BPF_REG_4, BPF_REG_9);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -4);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_MOV_IMM(BPF_REG_5, pcb->pcb_bufoff);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, 4);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ instr = BPF_CALL_HELPER(BPF_FUNC_perf_event_output);
+ dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
+ TRACE_REGSET("Epilogue: End ");
+ }
/*
- * bpf_perf_event_output(dctx->ctx, &buffers, BPF_F_CURRENT_CPU,
- * buf - 4, bufoff + 4);
- * // lddw %r1, [%fp + DT_STK_DCTX]
- * // lddw %r1, [%r1 + DCTX_CTX]
- * // lddw %r2, &buffers
- * // lddw %r3, BPF_F_CURRENT_CPU
- * // mov %r4, %r9
- * // add %r4, -4
- * // mov %r5, pcb->pcb_bufoff
- * // add %r4, 4
- * // call bpf_perf_event_output
- *
* exit:
* return 0; // mov %r0, 0
* // exit
* }
*/
- instr = BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_1, DCTX_CTX);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- dt_cg_xsetx(dlp, buffers, DT_LBL_NONE, BPF_REG_2, buffers->di_id);
- dt_cg_xsetx(dlp, NULL, DT_LBL_NONE, BPF_REG_3, BPF_F_CURRENT_CPU);
- instr = BPF_MOV_REG(BPF_REG_4, BPF_REG_9);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, -4);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_MOV_IMM(BPF_REG_5, pcb->pcb_bufoff);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, 4);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- instr = BPF_CALL_HELPER(BPF_FUNC_perf_event_output);
- dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
instr = BPF_MOV_IMM(BPF_REG_0, 0);
dt_irlist_append(dlp, dt_cg_node_alloc(pcb->pcb_exitlbl, instr));
instr = BPF_RETURN();
dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
- TRACE_REGSET("Epilogue: End ");
}
/*
diff --git a/test/unittest/actions/default/tst.default.d b/test/unittest/actions/default/tst.default.d
new file mode 100644
index 00000000..211465f3
--- /dev/null
+++ b/test/unittest/actions/default/tst.default.d
@@ -0,0 +1,40 @@
+/*
+ * Oracle Linux DTrace.
+ * Copyright (c) 2020, 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 default action is used for the empty clause but not
+ * for a non-empty clause, even if it does not trace anything.
+ * A special case is a printf() with no attributes, which is
+ * a trace record with no extra data in the tracing record;
+ * make sure the printf() occurs.
+ *
+ * SECTION: Actions and Subroutines/default
+ */
+
+/* should not trace, but should update n */
+BEGIN
+{ n = 1; }
+
+/* should trace, "hello world" should appear */
+tick-14
+{ printf("hello world"); }
+
+/* should not trace, but should update n */
+tick-13
+{ n += 2; }
+
+/* should trace */
+tick-12
+{ }
+
+/* should not trace, but should update n */
+tick-11
+{ n += 4; }
+
+/* should trace, and n should be 1+2+4=7 */
+tick-10
+{ trace(n); exit(0) }
diff --git a/test/unittest/actions/default/tst.default.r b/test/unittest/actions/default/tst.default.r
new file mode 100644
index 00000000..ef0e5257
--- /dev/null
+++ b/test/unittest/actions/default/tst.default.r
@@ -0,0 +1,7 @@
+ FUNCTION:NAME
+ :tick-14 hello world
+ :tick-12
+ :tick-10 7
+
+-- @@stderr --
+dtrace: script 'test/unittest/actions/default/tst.default.d' matched 6 probes
--
2.18.4
More information about the DTrace-devel
mailing list