From a5c941139c8b623eb7dc0e9ccdc4133e1c45048f Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 6 Feb 2024 14:17:39 +0100 Subject: [PATCH 1/3] Stmt: Introduce STMT_EXTERN It's currently possible for plugin's to implement their own statement subclasses and override the Exec() implementation. This has been leveraged by ZeekJS [1] and zeek-perf-support [2] as well as a private WASM plugin. All of these used STMT_ANY as the tag of their own statement subclasses. With STMT_EXTERN, we make the possibility to add external code into the AST somewhat more supported. It's all in detail space and plugin authors have no guarantee for stability, but it seems such a powerful extension point that IMO we should keep it. I'm conscious there's the broader topic how this interacts with ZAM optimization like in-lining or rewriting of statements. However, this already applies to the STMT_ANY usage of the mentioned plugins. [1] https://github.com/corelight/zeekjs [2] https://github.com/zeek/zeek-perf-support --- src/Stmt.cc | 1 + src/StmtEnums.h | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Stmt.cc b/src/Stmt.cc index cf455c44ed..6041e16e8a 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -52,6 +52,7 @@ const char* stmt_name(StmtTag t) { "ZAM", "null", "assert", + "extern", }; return stmt_names[int(t)]; diff --git a/src/StmtEnums.h b/src/StmtEnums.h index 3eb463eab9..7a237cb043 100644 --- a/src/StmtEnums.h +++ b/src/StmtEnums.h @@ -31,7 +31,8 @@ enum StmtTag { STMT_ZAM, // a ZAM function body STMT_NULL, STMT_ASSERT, -#define NUM_STMTS (int(STMT_ASSERT) + 1) + STMT_EXTERN, // for custom Stmt subclasses provided by plugins +#define NUM_STMTS (int(STMT_EXTERN) + 1) }; enum StmtFlowType { From 4019e9917ab72e4134e99dfa69ae40bf744d2724 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 6 Feb 2024 14:11:21 +0100 Subject: [PATCH 2/3] Stmt: Deprecate STMT_ANY This isn't used in-tree and has been misused by some external plugins of mine (zeekjs and zeek-perf-support) for their own Stmt subclasses. These plugins should be updated to use the new STMT_EXTERN statement. Handle STMT_ANY explicitly in stmt_name() for the time being to fix #3529 until we remove STMT_ANY for good. --- src/Stmt.cc | 6 ++++++ src/StmtEnums.h | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Stmt.cc b/src/Stmt.cc index 6041e16e8a..9c2a593b91 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -55,6 +55,12 @@ const char* stmt_name(StmtTag t) { "extern", }; +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + if ( int(t) == STMT_ANY ) + return "any"; +#pragma GCC diagnostic pop + return stmt_names[int(t)]; } diff --git a/src/StmtEnums.h b/src/StmtEnums.h index 7a237cb043..7cac90deae 100644 --- a/src/StmtEnums.h +++ b/src/StmtEnums.h @@ -6,7 +6,7 @@ namespace zeek::detail { // These are in a separate file to break circular dependences enum StmtTag { - STMT_ANY = -1, + STMT_ANY [[deprecated("Remove in v7.1 - Unused and plugins should use STMT_EXTERN.")]] = -1, STMT_ALARM, // Does no longer exist but kept to create enums consistent. STMT_PRINT, STMT_EVENT, From 8ad954cfd45b48ffe9797880df054776282a7588 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 6 Feb 2024 15:09:08 +0100 Subject: [PATCH 3/3] Bump zeekjs for STMT_ANY deprecation --- auxil/zeekjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/auxil/zeekjs b/auxil/zeekjs index 70d88f4b10..db66f7bc5a 160000 --- a/auxil/zeekjs +++ b/auxil/zeekjs @@ -1 +1 @@ -Subproject commit 70d88f4b10e36f8715eb0f2a25eb0fc0eb4fce37 +Subproject commit db66f7bc5a94cebf6476ff3600ae1a5402faa7a3