Index | Thread | Search

From:
Christian Ludwig <christian_ludwig@genua.de>
Subject:
Re: btrace: Implement else-branch in bt(5)
To:
<tech@openbsd.org>
Date:
Tue, 26 Mar 2024 21:53:02 +0100

Download raw body.

Thread
  • Christian Ludwig:

    btrace: Implement else-branch in bt(5)

Hi,

this is an updated diff to [1]. It implements the else branching logic
in btrace(8), including 'else if'. The statement lists for if-conditions
and else-conditions are wrapped in a 'struct bt_cond' now. Handling
B_AC_TEST statements moves into stmt_eval(), they are not special
anymore. That makes nested conditional statements easier to deal with.

Please test.

[1] https://marc.info/?l=openbsd-tech&m=171033395608513&w=2

So long,


 - Christian

---
 regress/usr.sbin/btrace/if.bt | 29 +++++++++++++++++++++++++++++
 regress/usr.sbin/btrace/if.ok |  4 ++++
 usr.sbin/btrace/bt_parse.y    | 18 ++++++++++++++----
 usr.sbin/btrace/bt_parser.h   |  8 ++++++++
 usr.sbin/btrace/btrace.c      | 32 +++++++++++++++++---------------
 5 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/regress/usr.sbin/btrace/if.bt b/regress/usr.sbin/btrace/if.bt
index 053801cac54..41f30815ba5 100644
--- a/regress/usr.sbin/btrace/if.bt
+++ b/regress/usr.sbin/btrace/if.bt
@@ -9,6 +9,20 @@ BEGIN {
 	if (1) {
 		printf("printed!\n");
 	}
+
+	if (0)
+		printf("simple if\n");
+	else
+		printf("simple else\n");
+
+	if (0) {
+		printf("disabled if\n");
+	} else if (1) {
+		printf("multiple statements in ");
+		printf("else-if branch\n");
+	} else {
+		printf("no else\n");
+	}
 }
 
 END {
@@ -18,4 +32,19 @@ END {
 		printf("(%d) ", @var);
 		printf("statements\n");
 	}
+
+	if (0) printf("single-line if\n"); else printf("single-line else\n");
+
+	if (0) {
+		printf("not printed\n");
+	} else {
+		if (0) {
+			printf("nested not printed\n");
+		} else {
+			printf("nested printed\n");
+			exit();
+			printf("nested not printed\n");
+		}
+		printf("also not printed\n");
+	}
 }
diff --git a/regress/usr.sbin/btrace/if.ok b/regress/usr.sbin/btrace/if.ok
index c2d5a91bc90..cfac256dac6 100644
--- a/regress/usr.sbin/btrace/if.ok
+++ b/regress/usr.sbin/btrace/if.ok
@@ -1,2 +1,6 @@
 printed!
+simple else
+multiple statements in else-if branch
 multiple (4) statements
+single-line else
+nested printed
diff --git a/usr.sbin/btrace/bt_parse.y b/usr.sbin/btrace/bt_parse.y
index 075eaa5b7f8..16b623530fb 100644
--- a/usr.sbin/btrace/bt_parse.y
+++ b/usr.sbin/btrace/bt_parse.y
@@ -119,7 +119,7 @@ static int 	 beflag = 0;		/* BEGIN/END parsing context flag */
 %token	<v.i>		ERROR ENDFILT
 %token	<v.i>		OP_EQ OP_NE OP_LE OP_LT OP_GE OP_GT OP_LAND OP_LOR
 /* Builtins */
-%token	<v.i>		BUILTIN BEGIN END HZ IF STR
+%token	<v.i>		BUILTIN BEGIN ELSE END HZ IF STR
 /* Functions and Map operators */
 %token  <v.i>		F_DELETE F_PRINT
 %token	<v.i>		MFUNC FUNC0 FUNC1 FUNCN OP1 OP2 OP4 MOP0 MOP1
@@ -248,7 +248,9 @@ stmt	: ';' NL			{ $$ = NULL; }
 	| GVAR '=' OP4 '(' expr ',' vargs ')'	{ $$ = bh_inc($1, $5, $7); }
 	;
 
-stmtblck: IF '(' expr ')' block			{ $$ = bt_new($3, $5); }
+stmtblck: IF '(' expr ')' block			{ $$ = bt_new($3, $5, NULL); }
+	| IF '(' expr ')' block ELSE block	{ $$ = bt_new($3, $5, $7); }
+	| IF '(' expr ')' block ELSE stmtblck	{ $$ = bt_new($3, $5, $7); }
 	;
 
 stmtlist: stmtlist stmtblck		{ $$ = bs_append($1, $2); }
@@ -340,15 +342,22 @@ bc_new(struct bt_arg *term, enum bt_argtype op, struct bt_arg *ba)
 
 /* Create a new if/else test */
 struct bt_stmt *
-bt_new(struct bt_arg *ba, struct bt_stmt *condbs)
+bt_new(struct bt_arg *ba, struct bt_stmt *condbs, struct bt_stmt *elsebs)
 {
 	struct bt_arg *bop;
+	struct bt_cond *bc;
 
 	bop = ba_op(B_AT_OP_NE, NULL, ba);
 
-	return bs_new(B_AC_TEST, bop, (struct bt_var *)condbs);
+	bc = calloc(1, sizeof(*bc));
+	if (bc == NULL)
+		err(1, "bt_cond: calloc");
+	bc->bc_condbs = condbs;
+	bc->bc_elsebs = elsebs;
 
+	return bs_new(B_AC_TEST, bop, (struct bt_var *)bc);
 }
+
 /* Create a new probe */
 struct bt_probe *
 bp_new(const char *prov, const char *func, const char *name, int32_t rate)
@@ -714,6 +723,7 @@ lookup(char *s)
 		{ "count",	MOP0, 		B_AT_MF_COUNT },
 		{ "cpu",	BUILTIN,	B_AT_BI_CPU },
 		{ "delete",	F_DELETE,	B_AC_DELETE },
+		{ "else",	ELSE,		0 },
 		{ "exit",	FUNC0,		B_AC_EXIT },
 		{ "hist",	OP1,		0 },
 		{ "hz",		HZ,		0 },
diff --git a/usr.sbin/btrace/bt_parser.h b/usr.sbin/btrace/bt_parser.h
index 051bd72815d..119a3ea1eeb 100644
--- a/usr.sbin/btrace/bt_parser.h
+++ b/usr.sbin/btrace/bt_parser.h
@@ -180,6 +180,14 @@ struct bt_arg {
 
 #define BA_INITIALIZER(v, t)	{ { NULL }, (void *)(v), NULL, (t) }
 
+/*
+ * Represents branches of an if-else statement.
+ */
+struct bt_cond {
+	struct bt_stmt		*bc_condbs;
+	struct bt_stmt		*bc_elsebs;
+};
+
 /*
  * Each action associated with a given probe is made of at least one
  * statement.
diff --git a/usr.sbin/btrace/btrace.c b/usr.sbin/btrace/btrace.c
index ce32989be91..defb7addf01 100644
--- a/usr.sbin/btrace/btrace.c
+++ b/usr.sbin/btrace/btrace.c
@@ -460,6 +460,7 @@ static uint64_t
 rules_action_scan(struct bt_stmt *bs)
 {
 	struct bt_arg *ba;
+	struct bt_cond *bc;
 	uint64_t evtflags = 0;
 
 	while (bs != NULL) {
@@ -474,8 +475,9 @@ rules_action_scan(struct bt_stmt *bs)
 			evtflags |= ba2dtflags(ba);
 			break;
 		case B_AC_TEST:
-			evtflags |= rules_action_scan(
-			    (struct bt_stmt *)bs->bs_var);
+			bc = (struct bt_cond *)bs->bs_var;
+			evtflags |= rules_action_scan(bc->bc_condbs);
+			evtflags |= rules_action_scan(bc->bc_elsebs);
 			break;
 		default:
 			break;
@@ -669,18 +671,6 @@ rule_eval(struct bt_rule *r, struct dt_evt *dtev)
 	}
 
 	SLIST_FOREACH(bs, &r->br_action, bs_next) {
-		if ((bs->bs_act == B_AC_TEST) && stmt_test(bs, dtev) == true) {
-			struct bt_stmt *bbs = (struct bt_stmt *)bs->bs_var;
-
-			while (bbs != NULL) {
-				if (stmt_eval(bbs, dtev))
-					return 1;
-				bbs = SLIST_NEXT(bbs, bs_next);
-			}
-
-			continue;
-		}
-
 		if (stmt_eval(bs, dtev))
 			return 1;
 	}
@@ -830,6 +820,8 @@ builtin_arg(struct dt_evt *dtev, enum bt_argtype dat)
 int
 stmt_eval(struct bt_stmt *bs, struct dt_evt *dtev)
 {
+	struct bt_stmt *bbs;
+	struct bt_cond *bc;
 	int halt = 0;
 
 	switch (bs->bs_act) {
@@ -858,7 +850,17 @@ stmt_eval(struct bt_stmt *bs, struct dt_evt *dtev)
 		stmt_store(bs, dtev);
 		break;
 	case B_AC_TEST:
-		/* done before */
+		bc = (struct bt_cond *)bs->bs_var;
+		if (stmt_test(bs, dtev) == true)
+			bbs = bc->bc_condbs;
+		else
+			bbs = bc->bc_elsebs;
+
+		while (bbs != NULL) {
+			if (stmt_eval(bbs, dtev))
+				return 1;
+			bbs = SLIST_NEXT(bbs, bs_next);
+		}
 		break;
 	case B_AC_TIME:
 		stmt_time(bs, dtev);
-- 
2.34.1