diff --git a/scripts/policy/protocols/ssl/ssl-log-ext.zeek b/scripts/policy/protocols/ssl/ssl-log-ext.zeek index 844111e7ca..6fed64169a 100644 --- a/scripts/policy/protocols/ssl/ssl-log-ext.zeek +++ b/scripts/policy/protocols/ssl/ssl-log-ext.zeek @@ -137,7 +137,7 @@ event ssl_extension_supported_versions(c: connection, is_client: bool, versions: c$ssl$server_supported_version = versions[0]; } - event ssl_extension_psk_key_exchange_modes(c: connection, is_client: bool, modes: index_vec) +event ssl_extension_psk_key_exchange_modes(c: connection, is_client: bool, modes: index_vec) { if ( ! c?$ssl || ! is_client ) return; diff --git a/src/Stmt.h b/src/Stmt.h index d79a8631fb..2a439ccaa8 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -122,6 +122,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; protected: ValPtr DoExec(Frame* f, Val* v, StmtFlowType& flow) override; @@ -182,6 +183,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; protected: friend class ZAMCompiler; @@ -301,6 +303,8 @@ public: // Note, no need for a NoFlowAfter method because the loop might // execute zero times, so it's always the default of "false". + // However, we do need to check for potential returns. + bool CouldReturn(bool ignore_break) const override; protected: ValPtr Exec(Frame* f, StmtFlowType& flow) override; @@ -350,6 +354,8 @@ public: // Note, no need for a NoFlowAfter method because the loop might // execute zero times, so it's always the default of "false". + // However, we do need to check for potential returns. + bool CouldReturn(bool ignore_break) const override; protected: ValPtr DoExec(Frame* f, Val* v, StmtFlowType& flow) override; @@ -395,6 +401,7 @@ public: StmtPtr Duplicate() override { return SetSucc(new BreakStmt()); } bool NoFlowAfter(bool ignore_break) const override { return ! ignore_break; } + bool CouldReturn(bool ignore_break) const override { return ! ignore_break; } protected: }; @@ -436,6 +443,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override { return true; } + bool CouldReturn(bool ignore_break) const override { return true; } }; class StmtList : public Stmt { @@ -460,6 +468,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; // Idioms commonly used in reduction. StmtList(StmtPtr s1, StmtPtr s2); @@ -725,7 +734,7 @@ public: // Note, no need for a NoFlowAfter() method because anything that // has "NoFlowAfter" inside the body still gets caught and we - // continue afterwards. + // continue afterwards. Same goes for CouldReturn(). StmtPtr Duplicate() override; diff --git a/src/StmtBase.h b/src/StmtBase.h index 46ff718d07..db47926c6f 100644 --- a/src/StmtBase.h +++ b/src/StmtBase.h @@ -138,6 +138,12 @@ public: // in that case, they do lead to flow reaching the end. virtual bool NoFlowAfter(bool ignore_break) const { return false; } + // True if the statement could potentially execute a return. This is + // used by script optimization to track confluence inside catch-return + // blocks. Here, ignore_break is false if we're analyzing a coalesced + // hook body. + virtual bool CouldReturn(bool ignore_break) const { return false; } + // Access to the original statement from which this one is derived, // or this one if we don't have an original. Returns a bare pointer // rather than a StmtPtr to emphasize that the access is read-only. diff --git a/src/script_opt/GenIDDefs.cc b/src/script_opt/GenIDDefs.cc index e4b24059d3..d0e6dee13b 100644 --- a/src/script_opt/GenIDDefs.cc +++ b/src/script_opt/GenIDDefs.cc @@ -62,17 +62,31 @@ TraversalCode GenIDDefs::PreStmt(const Stmt* s) { auto block = cr->Block(); cr_active.push_back(confluence_blocks.size()); - cr_return_seen.push_back(false); - block->Traverse(this); - if ( cr_return_seen.back() ) - // We encountered a return along the way. curr_stmt is - // now set to the last statement in the block, so mark - // it with a return. - ReturnAt(curr_stmt); + bool did_confluence = false; + + if ( block->Tag() == STMT_LIST ) + { + auto prev_stmt = s; + auto& stmts = block->AsStmtList()->Stmts(); + for ( auto& st : stmts ) + { + if ( ! did_confluence && st->CouldReturn(false) ) + { + StartConfluenceBlock(prev_stmt); + did_confluence = true; + } + + st->Traverse(this); + } + } + else + block->Traverse(this); + + if ( did_confluence ) + EndConfluenceBlock(); cr_active.pop_back(); - cr_return_seen.pop_back(); auto retvar = cr->RetVar(); if ( retvar ) @@ -445,11 +459,9 @@ void GenIDDefs::ReturnAt(const Stmt* s) { // If we're right at a catch-return then we don't want to make the // identifier as encountering a scope-ending "return" here. By avoiding // that, we're able to do optimization across catch-return blocks. - if ( cr_active.empty() || cr_active.back() != confluence_blocks.size() || cr_return_seen.back() ) + if ( cr_active.empty() || cr_active.back() != confluence_blocks.size() ) for ( auto id : modified_IDs.back() ) id->GetOptInfo()->ReturnAt(s); - if ( ! cr_active.empty() ) - cr_return_seen.back() = true; } void GenIDDefs::TrackID(const ID* id, const ExprPtr& e) { diff --git a/src/script_opt/GenIDDefs.h b/src/script_opt/GenIDDefs.h index 1088278c2c..a8b05864cb 100644 --- a/src/script_opt/GenIDDefs.h +++ b/src/script_opt/GenIDDefs.h @@ -99,9 +99,6 @@ private: // (new) confluence blocks. std::vector cr_active; - // Parallel array that tracks whether a return has occurred. - std::vector cr_return_seen; - // The following is parallel to confluence_blocks except // the front entry tracks identifiers at the outermost // (non-confluence) scope. Thus, to index it for a given diff --git a/src/script_opt/Stmt.cc b/src/script_opt/Stmt.cc index 9fc0de06f5..132f074423 100644 --- a/src/script_opt/Stmt.cc +++ b/src/script_opt/Stmt.cc @@ -273,6 +273,10 @@ bool IfStmt::NoFlowAfter(bool ignore_break) const { return false; } +bool IfStmt::CouldReturn(bool ignore_break) const { + return (s1 && s1->CouldReturn(ignore_break)) || (s2 && s2->CouldReturn(ignore_break)); +} + IntrusivePtr Case::Duplicate() { if ( expr_cases ) { auto new_exprs = expr_cases->Duplicate()->AsListExprPtr(); @@ -404,6 +408,14 @@ bool SwitchStmt::NoFlowAfter(bool ignore_break) const { return default_seen_with_no_flow_after; } +bool SwitchStmt::CouldReturn(bool ignore_break) const { + for ( const auto& c : *Cases() ) + if ( c->Body()->CouldReturn(true) ) + return true; + + return false; +} + bool AddDelStmt::IsReduced(Reducer* c) const { return e->HasReducedOps(c); } StmtPtr AddDelStmt::DoReduce(Reducer* c) { @@ -499,6 +511,10 @@ StmtPtr WhileStmt::DoReduce(Reducer* c) { return ThisPtr(); } +bool WhileStmt::CouldReturn(bool ignore_break) const { + return body->CouldReturn(false); +} + StmtPtr ForStmt::Duplicate() { auto expr_copy = e->Duplicate(); @@ -568,6 +584,10 @@ StmtPtr ForStmt::DoReduce(Reducer* c) { return ThisPtr(); } +bool ForStmt::CouldReturn(bool ignore_break) const { + return body->CouldReturn(false); +} + StmtPtr ReturnStmt::Duplicate() { return SetSucc(new ReturnStmt(e ? e->Duplicate() : nullptr, true)); } ReturnStmt::ReturnStmt(ExprPtr arg_e, bool ignored) : ExprStmt(STMT_RETURN, std::move(arg_e)) {} @@ -784,6 +804,14 @@ bool StmtList::NoFlowAfter(bool ignore_break) const { return false; } +bool StmtList::CouldReturn(bool ignore_break) const { + for ( auto& s : stmts ) + if ( s->CouldReturn(ignore_break) ) + return true; + + return false; +} + StmtPtr InitStmt::Duplicate() { // Need to duplicate the initializer list since later reductions // can modify it in place.