On Sat, Oct 21, 2023 at 07:17:05PM +0200, Martin Pieuchot wrote:
> On 18/10/23(Wed) 12:56, Scott Cheloha wrote:
> > Hi,
> > 
> > A bt(5) exit() statement causes the btrace(8) interpreter to exit(3)
> > immediately.
> > 
> > A BPFtrace exit() statement is more nuanced: the END probe is executed
> > and the contents of all maps are printed before the interpreter exits.
> > 
> > This patch adds a halting check after the execution of each bt(5)
> > statement.  If a statement causes the program to halt, the halt
> > bubbles up to the top-level rule evaluation loop and terminates
> > execution.  rules_teardown() then runs, just as if the program had
> > received SIGTERM.
> > 
> > Two edge-like cases:
> > 
> > 1. You can exit() from BEGIN.  rules_setup() returns non-zero if this
> >    happens so the main loop knows to halt immediately.
> > 
> > 2. You can exit() from END.  This is just an early-return: the END probe
> >    doesn't run again.
> > 
> > Thoughts?
> 
> Makes sense to ease the transition from bpftrace scripts.  Ok with me if
> you make sure the regression tests still pass.  Some outputs might
> depend on the actual behavior and would need to be updated.

Agh, my mistake, there are two tests that depend on the current
behavior.  I have updated them below.

ok with the test fixes?

Index: usr.sbin/btrace/btrace.c
===================================================================
RCS file: /cvs/src/usr.sbin/btrace/btrace.c,v
retrieving revision 1.79
diff -u -p -r1.79 btrace.c
--- usr.sbin/btrace/btrace.c    12 Oct 2023 15:16:44 -0000      1.79
+++ usr.sbin/btrace/btrace.c    22 Oct 2023 01:21:21 -0000
@@ -71,10 +71,10 @@ struct dtioc_probe_info     *dtpi_get_by_val
  * Main loop and rule evaluation.
  */
 void                    rules_do(int);
-void                    rules_setup(int);
-void                    rules_apply(int, struct dt_evt *);
+int                     rules_setup(int);
+int                     rules_apply(int, struct dt_evt *);
 void                    rules_teardown(int);
-void                    rule_eval(struct bt_rule *, struct dt_evt *);
+int                     rule_eval(struct bt_rule *, struct dt_evt *);
 void                    rule_printmaps(struct bt_rule *);
 
 /*
@@ -84,7 +84,7 @@ uint64_t               builtin_nsecs(struct dt_evt *
 const char             *builtin_kstack(struct dt_evt *);
 const char             *builtin_arg(struct dt_evt *, enum bt_argtype);
 struct bt_arg          *fn_str(struct bt_arg *, struct dt_evt *, char *);
-void                    stmt_eval(struct bt_stmt *, struct dt_evt *);
+int                     stmt_eval(struct bt_stmt *, struct dt_evt *);
 void                    stmt_bucketize(struct bt_stmt *, struct dt_evt *);
 void                    stmt_clear(struct bt_stmt *);
 void                    stmt_delete(struct bt_stmt *, struct dt_evt *);
@@ -405,6 +405,7 @@ void
 rules_do(int fd)
 {
        struct sigaction sa;
+       int halt = 0;
 
        memset(&sa, 0, sizeof(sa));
        sigemptyset(&sa.sa_mask);
@@ -415,9 +416,9 @@ rules_do(int fd)
        if (sigaction(SIGTERM, &sa, NULL))
                err(1, "sigaction");
 
-       rules_setup(fd);
+       halt = rules_setup(fd);
 
-       while (!quit_pending && g_nprobes > 0) {
+       while (!quit_pending && !halt && g_nprobes > 0) {
                static struct dt_evt devtbuf[64];
                ssize_t rlen;
                size_t i;
@@ -434,8 +435,11 @@ rules_do(int fd)
                if ((rlen % sizeof(struct dt_evt)) != 0)
                        err(1, "incorrect read");
 
-               for (i = 0; i < rlen / sizeof(struct dt_evt); i++)
-                       rules_apply(fd, &devtbuf[i]);
+               for (i = 0; i < rlen / sizeof(struct dt_evt); i++) {
+                       halt = rules_apply(fd, &devtbuf[i]);
+                       if (halt)
+                               break;
+               }
        }
 
        rules_teardown(fd);
@@ -484,7 +488,7 @@ rules_action_scan(struct bt_stmt *bs)
        return evtflags;
 }
 
-void
+int
 rules_setup(int fd)
 {
        struct dtioc_probe_info *dtpi;
@@ -493,7 +497,7 @@ rules_setup(int fd)
        struct bt_probe *bp;
        struct bt_stmt *bs;
        struct bt_arg *ba;
-       int dokstack = 0, on = 1;
+       int dokstack = 0, halt = 0, on = 1;
        uint64_t evtflags;
 
        TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -553,7 +557,7 @@ rules_setup(int fd)
        clock_gettime(CLOCK_REALTIME, &bt_devt.dtev_tsp);
 
        if (rbegin)
-               rule_eval(rbegin, &bt_devt);
+               halt = rule_eval(rbegin, &bt_devt);
 
        /* Enable all probes */
        TAILQ_FOREACH(r, &g_rules, br_next) {
@@ -571,9 +575,14 @@ rules_setup(int fd)
                if (ioctl(fd, DTIOCRECORD, &on))
                        err(1, "DTIOCRECORD");
        }
+
+       return halt;
 }
 
-void
+/*
+ * Returns non-zero if the program should halt.
+ */
+int
 rules_apply(int fd, struct dt_evt *dtev)
 {
        struct bt_rule *r;
@@ -586,9 +595,11 @@ rules_apply(int fd, struct dt_evt *dtev)
                                continue;
 
                        dtai_cache(fd, &dt_dtpis[dtev->dtev_pbn - 1]);
-                       rule_eval(r, dtev);
+                       if (rule_eval(r, dtev))
+                               return 1;
                }
        }
+       return 0;
 }
 
 void
@@ -637,7 +648,10 @@ rules_teardown(int fd)
                rule_printmaps(r);
 }
 
-void
+/*
+ * Returns non-zero if the program should halt.
+ */
+int
 rule_eval(struct bt_rule *r, struct dt_evt *dtev)
 {
        struct bt_stmt *bs;
@@ -651,7 +665,7 @@ rule_eval(struct bt_rule *r, struct dt_e
        if (r->br_filter != NULL && r->br_filter->bf_condition != NULL) {
                if (stmt_test(r->br_filter->bf_condition, dtev) == false) {
                        bt_filtered++;
-                       return;
+                       return 0;
                }
        }
 
@@ -660,15 +674,19 @@ rule_eval(struct bt_rule *r, struct dt_e
                        struct bt_stmt *bbs = (struct bt_stmt *)bs->bs_var;
 
                        while (bbs != NULL) {
-                               stmt_eval(bbs, dtev);
+                               if (stmt_eval(bbs, dtev))
+                                       return 1;
                                bbs = SLIST_NEXT(bbs, bs_next);
                        }
 
                        continue;
                }
 
-               stmt_eval(bs, dtev);
+               if (stmt_eval(bs, dtev))
+                       return 1;
        }
+
+       return 0;
 }
 
 void
@@ -802,9 +820,14 @@ builtin_arg(struct dt_evt *dtev, enum bt
        return buf;
 }
 
-void
+/*
+ * Returns non-zero if the program should halt.
+ */
+int
 stmt_eval(struct bt_stmt *bs, struct dt_evt *dtev)
 {
+       int halt = 0;
+
        switch (bs->bs_act) {
        case B_AC_BUCKETIZE:
                stmt_bucketize(bs, dtev);
@@ -816,7 +839,7 @@ stmt_eval(struct bt_stmt *bs, struct dt_
                stmt_delete(bs, dtev);
                break;
        case B_AC_EXIT:
-               exit(0);
+               halt = 1;
                break;
        case B_AC_INSERT:
                stmt_insert(bs, dtev);
@@ -842,6 +865,7 @@ stmt_eval(struct bt_stmt *bs, struct dt_
        default:
                xabort("no handler for action type %d", bs->bs_act);
        }
+       return halt;
 }
 
 /*
Index: usr.sbin/btrace/bt.5
===================================================================
RCS file: /cvs/src/usr.sbin/btrace/bt.5,v
retrieving revision 1.16
diff -u -p -r1.16 bt.5
--- usr.sbin/btrace/bt.5        30 Jun 2023 13:31:37 -0000      1.16
+++ usr.sbin/btrace/bt.5        22 Oct 2023 01:21:21 -0000
@@ -127,6 +127,11 @@ from
 .Va @map .
 .It Fn exit
 Terminate execution with exit code 0.
+The
+.Ic END
+probe,
+if any,
+is executed and the contents of all maps are printed.
 .It Fn hist "value"
 Increment the bucket corresponding to
 .Va value
Index: regress/usr.sbin/btrace/exit.bt
===================================================================
RCS file: /cvs/src/regress/usr.sbin/btrace/exit.bt,v
retrieving revision 1.1
diff -u -p -r1.1 exit.bt
--- regress/usr.sbin/btrace/exit.bt     21 Jan 2020 16:27:17 -0000      1.1
+++ regress/usr.sbin/btrace/exit.bt     22 Oct 2023 01:21:21 -0000
@@ -6,5 +6,7 @@ BEGIN
 
 END
 {
+       printf("executed\n");
+       exit();
        printf("not executed\n");
 }
Index: regress/usr.sbin/btrace/exit.ok
===================================================================
RCS file: /cvs/src/regress/usr.sbin/btrace/exit.ok,v
retrieving revision 1.1
diff -u -p -r1.1 exit.ok
--- regress/usr.sbin/btrace/exit.ok     21 Jan 2020 16:27:17 -0000      1.1
+++ regress/usr.sbin/btrace/exit.ok     22 Oct 2023 01:21:21 -0000
@@ -1 +1,2 @@
 exiting now...
+executed
Index: regress/usr.sbin/btrace/mapoverwrite.ok
===================================================================
RCS file: /cvs/src/regress/usr.sbin/btrace/mapoverwrite.ok,v
retrieving revision 1.1
diff -u -p -r1.1 mapoverwrite.ok
--- regress/usr.sbin/btrace/mapoverwrite.ok     13 Aug 2023 09:49:47 -0000      
1.1
+++ regress/usr.sbin/btrace/mapoverwrite.ok     22 Oct 2023 01:21:21 -0000
@@ -0,0 +1,3 @@
+@map[3]: 33
+@map[1]: 22
+@map[0]: 11

Reply via email to