diff --git a/doc/scripts/builtins.rst b/doc/scripts/builtins.rst index ba786ba0d2..cf62299783 100644 --- a/doc/scripts/builtins.rst +++ b/doc/scripts/builtins.rst @@ -505,15 +505,14 @@ The Bro scripting language supports the following built-in types. A hook is another flavor of function that shares characteristics of both a :bro:type:`function` and a :bro:type:`event`. They are like events in that many handler bodies can be defined for the same hook - identifier, they have no return vale, and the order of execution - can be enforced with :bro:attr:`&priority`. They are more like - functions in the way they are invoked/called, because, unlike - events, their execution is immediate and they do not get scheduled - through an event queue. Also, a unique feature of a hook is that - a given hook handler body can short-circuit the execution of - remaining hook handlers simply by exiting from the body as a result - of a ``break`` statement (as opposed to a ``return`` or just - reaching the end of the body). + identifier and the order of execution can be enforced with + :bro:attr:`&priority`. They are more like functions in the way they + are invoked/called, because, unlike events, their execution is + immediate and they do not get scheduled through an event queue. + Also, a unique feature of a hook is that a given hook handler body + can short-circuit the execution of remaining hook handlers simply by + exiting from the body as a result of a ``break`` statement (as + opposed to a ``return`` or just reaching the end of the body). A hook type is declared like:: @@ -553,12 +552,12 @@ The Bro scripting language supports the following built-in types. a hook type isn't strictly required, when it is provided, the argument types must match. - To invoke immediate execution of all hook handler bodies, a ``hook`` - statement must be used: + To invoke immediate execution of all hook handler bodies, they + can be called just like a function: .. code:: bro - hook myhook("hi"); + myhook("hi"); And the output would like like:: @@ -568,6 +567,12 @@ The Bro scripting language supports the following built-in types. Note how the modification to arguments can be seen by remaining hook handlers. + The return value of a hook call is an implicit :bro:type:`bool` + value with ``T`` meaning that all handlers for the hook were + executed and ``F`` meaning that only some of the handlers may have + executed due to one handler body exiting as a result of a ``break`` + statement. + Attributes ---------- diff --git a/src/Expr.cc b/src/Expr.cc index 733e0fe9a5..a7075566aa 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4374,7 +4374,7 @@ bool InExpr::DoUnserialize(UnserialInfo* info) return true; } -CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args, bool in_hook) +CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args) : Expr(EXPR_CALL) { func = arg_func; @@ -4415,13 +4415,8 @@ CallExpr::CallExpr(Expr* arg_func, ListExpr* arg_args, bool in_hook) break; case FUNC_FLAVOR_HOOK: - // It's fine to not have a yield if it's known that the call - // is being done from a hook statement. - if ( ! in_hook ) - { - Error("hook called in expression, use hook statement instead"); - SetError(); - } + Error("hook has no yield type"); + SetError(); break; default: diff --git a/src/Expr.h b/src/Expr.h index bd4824f8ee..c16cf86612 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -959,7 +959,7 @@ protected: class CallExpr : public Expr { public: - CallExpr(Expr* func, ListExpr* args, bool in_hook = false); + CallExpr(Expr* func, ListExpr* args); ~CallExpr(); Expr* Func() const { return func; } diff --git a/src/Func.cc b/src/Func.cc index 27acce4f04..e3675ebc2c 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -349,16 +349,31 @@ Val* BroFunc::Call(val_list* args, Frame* parent) const break; } - if ( flow == FLOW_BREAK && Flavor() == FUNC_FLAVOR_HOOK ) + if ( Flavor() == FUNC_FLAVOR_HOOK ) { - // short-circuit execution of remaining hook handler bodies - break; + // Ignore any return values of hook bodies, final return value + // depends on whether a body returns as a result of break statement + Unref(result); + result = 0; + + if ( flow == FLOW_BREAK ) + { + // short-circuit execution of remaining hook handler bodies + result = new Val(false, TYPE_BOOL); + break; + } } } + if ( Flavor() == FUNC_FLAVOR_HOOK ) + { + if ( ! result ) + result = new Val(true, TYPE_BOOL); + } + // Warn if the function returns something, but we returned from // the function without an explicit return, or without a value. - if ( FType()->YieldType() && FType()->YieldType()->Tag() != TYPE_VOID && + else if ( FType()->YieldType() && FType()->YieldType()->Tag() != TYPE_VOID && (flow != FLOW_RETURN /* we fell off the end */ || ! result /* explicit return with no result */) && ! f->HasDelayed() ) diff --git a/src/SerialTypes.h b/src/SerialTypes.h index a18c9bcc65..c47ff19298 100644 --- a/src/SerialTypes.h +++ b/src/SerialTypes.h @@ -165,7 +165,6 @@ SERIAL_STMT(EVENT_BODY_LIST, 16) SERIAL_STMT(INIT_STMT, 17) SERIAL_STMT(NULL_STMT, 18) SERIAL_STMT(WHEN_STMT, 19) -SERIAL_STMT(HOOK_STMT, 20) #define SERIAL_TYPE(name, val) SERIAL_CONST(name, val, BRO_TYPE) SERIAL_TYPE(BRO_TYPE, 1) diff --git a/src/Stmt.cc b/src/Stmt.cc index 0a5ae16ef6..c65b44a9bd 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -23,7 +23,7 @@ const char* stmt_name(BroStmtTag t) "print", "event", "expr", "if", "when", "switch", "for", "next", "break", "return", "add", "delete", "list", "bodylist", - "", "hook", + "", "null", }; @@ -933,52 +933,6 @@ bool EventStmt::DoUnserialize(UnserialInfo* info) return event_expr != 0; } -HookStmt::HookStmt(CallExpr* arg_e) : ExprStmt(STMT_HOOK, arg_e) - { - call_expr = arg_e; - } - -Val* HookStmt::Exec(Frame* f, stmt_flow_type& flow) const - { - RegisterAccess(); - - Val* ret = call_expr->Eval(f); - Unref(ret); - - flow = FLOW_NEXT; - - return 0; - } - -TraversalCode HookStmt::Traverse(TraversalCallback* cb) const - { - TraversalCode tc = cb->PreStmt(this); - HANDLE_TC_STMT_PRE(tc); - - // call expr is stored in base class's "e" field. - tc = e->Traverse(cb); - HANDLE_TC_STMT_PRE(tc); - - tc = cb->PostStmt(this); - HANDLE_TC_STMT_POST(tc); - } - -IMPLEMENT_SERIAL(HookStmt, SER_HOOK_STMT); - -bool HookStmt::DoSerialize(SerialInfo* info) const - { - DO_SERIALIZE(SER_HOOK_STMT, ExprStmt); - return call_expr->Serialize(info); - } - -bool HookStmt::DoUnserialize(UnserialInfo* info) - { - DO_UNSERIALIZE(ExprStmt); - - call_expr = (CallExpr*) Expr::Unserialize(info, EXPR_CALL); - return call_expr != 0; - } - ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr) : ExprStmt(STMT_FOR, loop_expr) { @@ -1378,7 +1332,10 @@ ReturnStmt::ReturnStmt(Expr* arg_e) : ExprStmt(STMT_RETURN, arg_e) } else if ( ! e ) - Error("return statement needs expression"); + { + if ( ft->Flavor() != FUNC_FLAVOR_HOOK ) + Error("return statement needs expression"); + } else (void) check_and_promote_expr(e, yt); @@ -1990,7 +1947,6 @@ int same_stmt(const Stmt* s1, const Stmt* s2) case STMT_RETURN: case STMT_EXPR: case STMT_EVENT: - case STMT_HOOK: { const ExprStmt* e1 = (const ExprStmt*) s1; const ExprStmt* e2 = (const ExprStmt*) s2; diff --git a/src/Stmt.h b/src/Stmt.h index 68bb8d6425..7c3b42609b 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -286,24 +286,6 @@ protected: EventExpr* event_expr; }; -class HookStmt : public ExprStmt { -public: - HookStmt(CallExpr* e); - - Val* Exec(Frame* f, stmt_flow_type& flow) const; - - TraversalCode Traverse(TraversalCallback* cb) const; - -protected: - friend class Stmt; - - HookStmt() { call_expr = 0; } - - DECLARE_SERIAL(HookStmt); - - CallExpr* call_expr; -}; - class ForStmt : public ExprStmt { public: ForStmt(id_list* loop_vars, Expr* loop_expr); diff --git a/src/StmtEnums.h b/src/StmtEnums.h index fa5b70389d..f431e3fea1 100644 --- a/src/StmtEnums.h +++ b/src/StmtEnums.h @@ -15,7 +15,7 @@ typedef enum { STMT_RETURN, STMT_ADD, STMT_DELETE, STMT_LIST, STMT_EVENT_BODY_LIST, - STMT_INIT, STMT_HOOK, + STMT_INIT, STMT_NULL #define NUM_STMTS (int(STMT_NULL) + 1) } BroStmtTag; diff --git a/src/Var.cc b/src/Var.cc index 9c4fb5b978..68be18b55e 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -294,12 +294,12 @@ void add_type(ID* id, BroType* t, attr_list* attr, int /* is_event */) void begin_func(ID* id, const char* module_name, function_flavor flavor, int is_redef, FuncType* t) { - if ( flavor == FUNC_FLAVOR_EVENT || flavor == FUNC_FLAVOR_HOOK ) + if ( flavor == FUNC_FLAVOR_EVENT ) { const BroType* yt = t->YieldType(); if ( yt && yt->Tag() != TYPE_VOID ) - id->Error("event/hook cannot yield a value", t); + id->Error("event cannot yield a value", t); t->ClearYieldType(flavor); } diff --git a/src/parse.y b/src/parse.y index b4eee1a56c..39673cacc2 100644 --- a/src/parse.y +++ b/src/parse.y @@ -2,7 +2,7 @@ // See the file "COPYING" in the main distribution directory for copyright. %} -%expect 88 +%expect 87 %token TOK_ADD TOK_ADD_TO TOK_ADDR TOK_ANY %token TOK_ATENDIF TOK_ATELSE TOK_ATIF TOK_ATIFDEF TOK_ATIFNDEF @@ -56,7 +56,6 @@ %type pattern %type expr init anonymous_function %type event -%type hook %type stmt stmt_list func_body for_head %type type opt_type enum_body %type func_hdr func_params @@ -212,7 +211,6 @@ static std::list* concat_opt_docs (std::list* pre, Val* val; RE_Matcher* re; Expr* expr; - CallExpr* call_expr; EventExpr* event_expr; Stmt* stmt; ListExpr* list; @@ -875,7 +873,7 @@ type: | TOK_HOOK '(' formal_args ')' { set_location(@1, @3); - $$ = new FuncType($3, 0, FUNC_FLAVOR_HOOK); + $$ = new FuncType($3, base_type(TYPE_BOOL), FUNC_FLAVOR_HOOK); } | TOK_FILE TOK_OF type @@ -1209,6 +1207,8 @@ func_hdr: } | TOK_HOOK def_global_id func_params { + $3->ClearYieldType(FUNC_FLAVOR_HOOK); + $3->SetYieldType(base_type(TYPE_BOOL)); begin_func($2, current_module.c_str(), FUNC_FLAVOR_HOOK, 0, $3); $$ = $3; @@ -1372,14 +1372,6 @@ stmt: brofiler.AddStmt($$); } - | TOK_HOOK hook ';' opt_no_test - { - set_location(@1, @4); - $$ = new HookStmt($2); - if ( ! $4 ) - brofiler.AddStmt($$); - } - | TOK_IF '(' expr ')' stmt { set_location(@1, @4); @@ -1533,14 +1525,6 @@ event: } ; -hook: - expr '(' opt_expr_list ')' - { - set_location(@1, @4); - $$ = new CallExpr($1, $3, true); - } - ; - case_list: case_list case { $1->append($2); } diff --git a/testing/btest/Baseline/language.hook/out b/testing/btest/Baseline/language.hook/out index 10688acc3b..18828ad15f 100644 --- a/testing/btest/Baseline/language.hook/out +++ b/testing/btest/Baseline/language.hook/out @@ -1,7 +1,15 @@ myhook, &priority=10, [a=1156, b=hello world] +myhook return F +myhook return T myhook, &priority=5, [a=37, b=goobye world] +F myhook3, 8 +T myhook4, 2 myhook4, 1 +T myhook, &priority=10, [a=2, b=it works] +myhook return F +myhook return T myhook, &priority=5, [a=37, b=goobye world] +F diff --git a/testing/btest/Baseline/language.invalid_hook/out b/testing/btest/Baseline/language.invalid_hook/out deleted file mode 100644 index 167d62ccc8..0000000000 --- a/testing/btest/Baseline/language.invalid_hook/out +++ /dev/null @@ -1 +0,0 @@ -error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.invalid_hook/invalid_hook.bro, line 15: hook called in expression, use hook statement instead (myhook(nope)) diff --git a/testing/btest/core/leaks/hook.bro b/testing/btest/core/leaks/hook.bro index eadb406e71..d9dcbff369 100644 --- a/testing/btest/core/leaks/hook.bro +++ b/testing/btest/core/leaks/hook.bro @@ -39,7 +39,25 @@ hook myhook(r: rec) &priority=10 r$b = "goobye world"; # returning from the handler early, is fine, remaining handlers still run. return; - print "ERROR: break statement should return from hook handler body"; + print "ERROR: return statement should return from hook handler body"; + } + +hook myhook(r: rec) &priority=9 + { + print "myhook return F"; + # return value is ignored, remaining handlers still run, final return + # value is whether any hook body returned via break statement + return F; + print "ERROR: return statement should return from hook handler body"; + } + +hook myhook(r: rec) &priority=8 + { + print "myhook return T"; + # return value is ignored, remaining handlers still run, final return + # value is whether any hook body returned via break statement + return T; + print "ERROR: return statement should return from hook handler body"; } # hook function doesn't need a declaration, we can go straight to defining @@ -63,16 +81,16 @@ event new_connection(c: connection) { print "new_connection", c$id; - hook myhook([$a=1156, $b="hello world"]); + print myhook([$a=1156, $b="hello world"]); # A hook with no handlers is fine, it's just a no-op. - hook myhook2("nope"); + print myhook2("nope"); - hook myhook3(8); - hook myhook4(); + print myhook3(8); + print myhook4(); # A hook can be treated like other data types and doesn't have to be # invoked directly by name. local h = myhook; - hook h([$a=2, $b="it works"]); + print h([$a=2, $b="it works"]); } diff --git a/testing/btest/language/hook.bro b/testing/btest/language/hook.bro index 8f7a85ce95..eedd2ff056 100644 --- a/testing/btest/language/hook.bro +++ b/testing/btest/language/hook.bro @@ -34,7 +34,25 @@ hook myhook(r: rec) &priority=10 r$b = "goobye world"; # returning from the handler early, is fine, remaining handlers still run. return; - print "ERROR: break statement should return from hook handler body"; + print "ERROR: return statement should return from hook handler body"; + } + +hook myhook(r: rec) &priority=9 + { + print "myhook return F"; + # return value is ignored, remaining handlers still run, final return + # value is whether any hook body returned via break statement + return F; + print "ERROR: return statement should return from hook handler body"; + } + +hook myhook(r: rec) &priority=8 + { + print "myhook return T"; + # return value is ignored, remaining handlers still run, final return + # value is whether any hook body returned via break statement + return T; + print "ERROR: return statement should return from hook handler body"; } # hook function doesn't need a declaration, we can go straight to defining @@ -56,16 +74,16 @@ hook myhook4() &priority=2 event bro_init() { - hook myhook([$a=1156, $b="hello world"]); + print myhook([$a=1156, $b="hello world"]); # A hook with no handlers is fine, it's just a no-op. - hook myhook2("nope"); + print myhook2("nope"); - hook myhook3(8); - hook myhook4(); + print myhook3(8); + print myhook4(); # A hook can be treated like other data types and doesn't have to be # invoked directly by name. local h = myhook; - hook h([$a=2, $b="it works"]); + print h([$a=2, $b="it works"]); } diff --git a/testing/btest/language/invalid_hook.bro b/testing/btest/language/invalid_hook.bro deleted file mode 100644 index 0dbbfd1b6f..0000000000 --- a/testing/btest/language/invalid_hook.bro +++ /dev/null @@ -1,16 +0,0 @@ -# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1 -# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out - -global myhook: hook(s: string); - -hook myhook(s: string) - { - print "myhook", s; - } - -event bro_init() - { - # hooks must be invoked with a "hook", statement. They have no return - # value and don't make sense to evaluate as arbitrary expressions. - local r = myhook("nope"); - }