[DTrace-devel] [PATCH] Fix ERROR-in-BEGIN probe handling

[Kris Van Hees]

The handling of ERROR probe invocations during the BEGIN probe execution
was flawed.  It failed to continue processing BEGIN clauses once an
ERROR invocation took place, and it didn't provide (as required) for
processing the CPU probe data buffer twice.

The continuation of processing has been corrected by ensuring that the
correct DTrace workstatus is reported from the probe callback functions.
The double buffer processing is now supported by passing a flag to
st_consume_cpu() to indicate whether the tail of the ring buffer ought
to be updated or not.

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_consume.c                        | 23 ++++++++++---------
 .../error/tst.clause_scope-begin-ended.d      |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/libdtrace/dt_consume.c b/libdtrace/dt_consume.c
index 1db05255..db0e1dea 100644
--- a/libdtrace/dt_consume.c
+++ b/libdtrace/dt_consume.c
@@ -2178,7 +2178,7 @@ dt_consume_one(dtrace_hdl_t *dtp, FILE *fp, char *buf,
 int
 dt_consume_cpu(dtrace_hdl_t *dtp, FILE *fp, dt_peb_t *peb,
 	       dtrace_consume_probe_f *efunc, dtrace_consume_rec_f *rfunc,
-	       void *arg)
+	       int peek_only, void *arg)
 {
 	struct perf_event_mmap_page	*rb_page = (void *)peb->base;
 	struct perf_event_header	*hdr;
@@ -2209,7 +2209,7 @@ dt_consume_cpu(dtrace_hdl_t *dtp, FILE *fp, dt_peb_t *peb,
 	 */
 	base = peb->base + pebset->page_size;
 
-	for (;;) {
+	do {
 		head = ring_buffer_read_head(rb_page);
 		tail = rb_page->data_tail;
 
@@ -2253,8 +2253,9 @@ dt_consume_cpu(dtrace_hdl_t *dtp, FILE *fp, dt_peb_t *peb,
 			tail += hdr->size;
 		} while (tail != head);
 
-		ring_buffer_write_tail(rb_page, tail);
-	}
+		if (!peek_only)
+			ring_buffer_write_tail(rb_page, tail);
+	} while (!peek_only);
 
 	return DTRACE_WORKSTATUS_OKAY;
 }
@@ -2278,7 +2279,7 @@ dt_consume_begin_probe(const dtrace_probedata_t *data, void *arg)
 
 	if (begin->dtbgn_beginonly) {
 		if (!(r1 && r2))
-			return DTRACE_CONSUME_DONE;
+			return DTRACE_CONSUME_NEXT;
 	} else {
 		if (r1 && r2)
 			return DTRACE_CONSUME_NEXT;
@@ -2384,7 +2385,7 @@ dt_consume_begin(dtrace_hdl_t *dtp, FILE *fp, struct epoll_event *events,
 	 * and return.
 	 */
 	if (!dtp->dt_stopped || cpu != dtp->dt_endedon)
-		return dt_consume_cpu(dtp, fp, bpeb, pf, rf, arg);
+		return dt_consume_cpu(dtp, fp, bpeb, pf, rf, 0, arg);
 
 	begin.dtbgn_probefunc = pf;
 	begin.dtbgn_recfunc = rf;
@@ -2401,7 +2402,7 @@ dt_consume_begin(dtrace_hdl_t *dtp, FILE *fp, struct epoll_event *events,
 	dtp->dt_errarg = &begin;
 
 	rval = dt_consume_cpu(dtp, fp, bpeb, dt_consume_begin_probe,
-			      dt_consume_begin_record, &begin);
+			      dt_consume_begin_record, 1, &begin);
 
 	dtp->dt_errhdlr = begin.dtbgn_errhdlr;
 	dtp->dt_errarg = begin.dtbgn_errarg;
@@ -2415,7 +2416,7 @@ dt_consume_begin(dtrace_hdl_t *dtp, FILE *fp, struct epoll_event *events,
 		if (peb == NULL || peb == bpeb)
 			continue;
 
-		rval = dt_consume_cpu(dtp, fp, peb, pf, rf, arg);
+		rval = dt_consume_cpu(dtp, fp, peb, pf, rf, 0, arg);
 		if (rval != 0)
 			return rval;
 	}
@@ -2435,7 +2436,7 @@ dt_consume_begin(dtrace_hdl_t *dtp, FILE *fp, struct epoll_event *events,
 	dtp->dt_errarg = &begin;
 
 	rval = dt_consume_cpu(dtp, fp, bpeb, dt_consume_begin_probe,
-			      dt_consume_begin_record, &begin);
+			      dt_consume_begin_record, 0, &begin);
 
 	dtp->dt_errhdlr = begin.dtbgn_errhdlr;
 	dtp->dt_errarg = begin.dtbgn_errarg;
@@ -2574,7 +2575,7 @@ dtrace_consume(dtrace_hdl_t *dtp, FILE *fp, dtrace_consume_probe_f *pf,
 		if (dtp->dt_stopped && peb->cpu == dtp->dt_endedon)
 			continue;
 
-		rval = dt_consume_cpu(dtp, fp, peb, pf, rf, arg);
+		rval = dt_consume_cpu(dtp, fp, peb, pf, rf, 0, arg);
 		if (rval != 0)
 			return rval;
 	}
@@ -2597,7 +2598,7 @@ dtrace_consume(dtrace_hdl_t *dtp, FILE *fp, dtrace_consume_probe_f *pf,
 		if (peb->cpu != dtp->dt_endedon)
 			continue;
 
-		return dt_consume_cpu(dtp, fp, peb, pf, rf, arg);
+		return dt_consume_cpu(dtp, fp, peb, pf, rf, 0, arg);
 	}
 
 	/*
diff --git a/test/unittest/error/tst.clause_scope-begin-ended.d b/test/unittest/error/tst.clause_scope-begin-ended.d
index 29dd0aef..6ba9f22a 100644
--- a/test/unittest/error/tst.clause_scope-begin-ended.d
+++ b/test/unittest/error/tst.clause_scope-begin-ended.d
@@ -4,7 +4,7 @@
  * Licensed under the Universal Permissive License v 1.0 as shown at
  * http://oss.oracle.com/licenses/upl.
  */
-/* @@xfail: dtv2 */
+
 /*
  * ASSERTION: A fault that triggers the ERROR probe terminates the execution of
  *            the current clause, but other clauses for the same probe should
-- 
2.33.0