Merge remote-tracking branch 'origin/topic/vern/script-opt-maint.Dec23'

* origin/topic/vern/script-opt-maint.Dec23:
  recent BTests that should be skipped when using -O gen-C++
  expanded ZAM maintenance notes & support scripts
  script optimization tracking of functions called by event engine or indirectly
  memory-handling fixes for information associated with low-level ZAM instructions
  fix for -O C++ lambda functions reporting errors/warnings
  revert problems with profiling attributes introduced by recent script-opt PR
  script optimization fixes for pattern tables
  regularized (some) types of pointers used in script optimization
  splitting off script optimization CSE into its own source files
  some very minor tidying of script optimization code/documentation
  fix for Trigger's whose termination leads to deleting other Trigger's
  bug fix for delayed logging

CHANGES

@@ -1,3 +1,29 @@
+6.2.0-dev.275 | 2023-12-12 09:52:00 +0100
+
+  * recent BTests that should be skipped when using -O gen-C++ (Vern Paxson, Corelight)
+
+  * expanded ZAM maintenance notes & support scripts (Vern Paxson, Corelight)
+
+  * script optimization tracking of functions called by event engine or indirectly (Vern Paxson, Corelight)
+
+  * memory-handling fixes for information associated with low-level ZAM instructions (Vern Paxson, Corelight)
+
+  * fix for -O C++ lambda functions reporting errors/warnings (Vern Paxson, Corelight)
+
+  * revert problems with profiling attributes introduced by recent script-opt PR (Vern Paxson, Corelight)
+
+  * script optimization fixes for pattern tables (Vern Paxson, Corelight)
+
+  * regularized (some) types of pointers used in script optimization (Vern Paxson, Corelight)
+
+  * splitting off script optimization CSE into its own source files (Vern Paxson, Corelight)
+
+  * some very minor tidying of script optimization code/documentation (Vern Paxson, Corelight)
+
+  * fix for Trigger's whose termination leads to deleting other Trigger's (Vern Paxson, Corelight)
+
+  * bug fix for delayed logging (Vern Paxson, Corelight)
+
 6.2.0-dev.262 | 2023-12-11 13:11:09 +0100
 
   * Bump auxil/spicy to latest development snapshot (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-6.2.0-dev.262
+6.2.0-dev.275

src/CMakeLists.txt

@@ -394,6 +394,7 @@ set(MAIN_SRCS
     script_opt/CPP/Util.cc
     script_opt/CPP/Vars.cc
     ${_gen_zeek_script_cpp}
+    script_opt/CSE.cc
     script_opt/Expr.cc
     script_opt/FuncInfo.cc
     script_opt/GenIDDefs.cc

src/Notifier.cc

@@ -58,13 +58,10 @@ void Registry::Modified(Modifiable* m) {
 }
 
 void Registry::Terminate() {
-    std::set<Receiver*> receivers;
-
-    for ( auto& r : registrations )
-        receivers.emplace(r.second);
-
-    for ( auto& r : receivers )
-        r->Terminate();
+    while ( ! registrations.empty() ) {
+        const auto& it = registrations.begin();
+        it->second->Terminate();
+    }
 }
 
 Modifiable::~Modifiable() {

src/Trigger.h

@@ -69,7 +69,7 @@ class Trigger final : public Obj, public notifier::detail::Receiver {
     ~Trigger() override;
 
     // Evaluates the condition. If true, executes the body and deletes
-    // the object deleted.
+    // the object.
     //
     // Returns the state of condition.
     bool Eval();

src/logging/Manager.cc

@@ -1194,7 +1194,7 @@ ValPtr Manager::Delay(const EnumValPtr& id, const RecordValPtr record, FuncPtr p
     }
 
     const auto& active_write_ctx = active_writes.back();
-    if ( active_write_ctx.id != id || active_write_ctx.record != record ) {
+    if ( active_write_ctx.id->Get() != id->Get() || active_write_ctx.record != record ) {
         reporter->Error("invalid Log::delay() call: argument mismatch with active Log::write()");
         return make_intrusive<detail::LogDelayTokenVal>();
     }

src/script_opt/CPP/Attrs.cc

@@ -16,7 +16,7 @@ shared_ptr<CPP_InitInfo> CPPCompile::RegisterAttributes(const AttributesPtr& att
     if ( pa != processed_attrs.end() )
         return pa->second;
 
-    attributes.AddKey(attrs, pfs.HashAttrs(attrs));
+    attributes.AddKey(attrs, pfs->HashAttrs(attrs));
 
     // The cast is just so we can make an IntrusivePtr.
     auto a_rep = const_cast<Attributes*>(attributes.GetRep(attrs));

src/script_opt/CPP/Compile.h

@@ -124,8 +124,8 @@ namespace zeek::detail {
 
 class CPPCompile {
 public:
-    CPPCompile(std::vector<FuncInfo>& _funcs, ProfileFuncs& pfs, const std::string& gen_name, bool _standalone,
-               bool report_uncompilable);
+    CPPCompile(std::vector<FuncInfo>& _funcs, std::shared_ptr<ProfileFuncs> pfs, const std::string& gen_name,
+               bool _standalone, bool report_uncompilable);
     ~CPPCompile();
 
     // Constructing a CPPCompile object does all of the compilation.
@@ -191,7 +191,7 @@ class CPPCompile {
     // However, we can't generate that code when first encountering
     // the attribute, because doing so will need to refer to the names
     // of types, and initially those are unavailable (because the type's
-    // representatives, per pfs.RepTypes(), might not have yet been
+    // representatives, per pfs->RepTypes(), might not have yet been
     // tracked).  So instead we track the associated CallExprInitInfo
     // objects, and after all types have been tracked, then spin
     // through them to generate the code.
@@ -314,7 +314,7 @@ class CPPCompile {
     std::vector<FuncInfo>& funcs;
 
     // The global profile of all of the functions.
-    ProfileFuncs& pfs;
+    std::shared_ptr<ProfileFuncs> pfs;
 
     // Script functions that we are able to compile.  We compute
     // these ahead of time so that when compiling script function A
@@ -894,7 +894,7 @@ class CPPCompile {
     // Returns the "representative" for a given type, used to ensure
     // that we re-use the C++ variable corresponding to a type and
     // don't instantiate redundant instances.
-    const Type* TypeRep(const Type* t) { return pfs.TypeRep(t); }
+    const Type* TypeRep(const Type* t) { return pfs->TypeRep(t); }
     const Type* TypeRep(const TypePtr& t) { return TypeRep(t.get()); }
 
     // Low-level C++ representations for types, of various flavors.

src/script_opt/CPP/DeclFunc.cc

@@ -144,6 +144,7 @@ void CPPCompile::DeclareSubclass(const FuncTypePtr& ft, const ProfileFunc* pf, c
     StartBlock();
 
     Emit("flow = FLOW_RETURN;");
+    Emit("f->SetOnlyCall(ce.get());");
 
     if ( in_hook ) {
         Emit("if ( ! %s(%s) )", fname, args);

src/script_opt/CPP/Driver.cc

@@ -13,9 +13,9 @@ namespace zeek::detail {
 
 using namespace std;
 
-CPPCompile::CPPCompile(vector<FuncInfo>& _funcs, ProfileFuncs& _pfs, const string& gen_name, bool _standalone,
-                       bool report_uncompilable)
-    : funcs(_funcs), pfs(_pfs), standalone(_standalone) {
+CPPCompile::CPPCompile(vector<FuncInfo>& _funcs, std::shared_ptr<ProfileFuncs> _pfs, const string& gen_name,
+                       bool _standalone, bool report_uncompilable)
+    : funcs(_funcs), pfs(std::move(_pfs)), standalone(_standalone) {
     auto target_name = gen_name.c_str();
 
     write_file = fopen(target_name, "w");
@@ -99,21 +99,21 @@ void CPPCompile::Compile(bool report_uncompilable) {
     GenProlog();
 
     // Track all of the types we'll be using.
-    for ( const auto& t : pfs.RepTypes() ) {
+    for ( const auto& t : pfs->RepTypes() ) {
         TypePtr tp{NewRef{}, (Type*)(t)};
-        types.AddKey(tp, pfs.HashType(t));
+        types.AddKey(tp, pfs->HashType(t));
     }
 
     NL();
 
-    for ( auto& g : pfs.AllGlobals() )
+    for ( auto& g : pfs->AllGlobals() )
         CreateGlobal(g);
 
-    for ( const auto& e : pfs.Events() )
+    for ( const auto& e : pfs->Events() )
         if ( AddGlobal(e, "gl") )
             Emit("EventHandlerPtr %s_ev;", globals[string(e)]);
 
-    for ( const auto& t : pfs.RepTypes() ) {
+    for ( const auto& t : pfs->RepTypes() ) {
         ASSERT(types.HasKey(t));
         TypePtr tp{NewRef{}, (Type*)(t)};
         RegisterType(tp);
@@ -131,14 +131,14 @@ void CPPCompile::Compile(bool report_uncompilable) {
     // be identical.  In that case, we don't want to generate the lambda
     // twice, but we do want to map the second one to the same body name.
     unordered_map<string, const Stmt*> lambda_ASTs;
-    for ( const auto& l : pfs.Lambdas() ) {
+    for ( const auto& l : pfs->Lambdas() ) {
         const auto& n = l->Name();
         const auto body = l->Ingredients()->Body().get();
         if ( lambda_ASTs.count(n) > 0 )
             // Reuse previous body.
             body_names[body] = body_names[lambda_ASTs[n]];
         else {
-            DeclareLambda(l, pfs.ExprProf(l).get());
+            DeclareLambda(l, pfs->ExprProf(l).get());
             lambda_ASTs[n] = body;
         }
     }
@@ -151,12 +151,12 @@ void CPPCompile::Compile(bool report_uncompilable) {
             CompileFunc(func);
 
     lambda_ASTs.clear();
-    for ( const auto& l : pfs.Lambdas() ) {
+    for ( const auto& l : pfs->Lambdas() ) {
         const auto& n = l->Name();
         if ( lambda_ASTs.count(n) > 0 )
             continue;
 
-        CompileLambda(l, pfs.ExprProf(l).get());
+        CompileLambda(l, pfs->ExprProf(l).get());
         lambda_ASTs[n] = l->Ingredients()->Body().get();
     }
 

src/script_opt/CPP/Exprs.cc

@@ -151,7 +151,7 @@ string CPPCompile::GenNameExpr(const NameExpr* ne, GenType gt) {
 
     if ( t->Tag() == TYPE_FUNC && ! is_global_var ) {
         auto func = n->Name();
-        if ( globals.count(func) > 0 && pfs.BiFGlobals().count(n) == 0 )
+        if ( globals.count(func) > 0 && pfs->BiFGlobals().count(n) == 0 )
             return GenericValPtrToGT(IDNameStr(n), t, gt);
     }
 
@@ -202,9 +202,9 @@ string CPPCompile::GenIncrExpr(const Expr* e, GenType gt, bool is_incr, bool top
 
     // Make sure any newly created types are known to
     // the profiler.
-    (void)pfs.HashType(one_e->GetType());
-    (void)pfs.HashType(rhs->GetType());
-    (void)pfs.HashType(assign->GetType());
+    (void)pfs->HashType(one_e->GetType());
+    (void)pfs->HashType(rhs->GetType());
+    (void)pfs->HashType(assign->GetType());
 
     auto gen = GenExpr(assign, GEN_DONT_CARE, top_level);
 
@@ -269,10 +269,10 @@ string CPPCompile::GenCallExpr(const CallExpr* c, GenType gt, bool top_level) {
         //
         // If it is a BiF *that's also a global variable*, then
         // we need to look up the BiF version of the global.
-        if ( pfs.BiFGlobals().count(f_id) == 0 )
+        if ( pfs->BiFGlobals().count(f_id) == 0 )
             gen += +"->AsFunc()";
 
-        else if ( pfs.Globals().count(f_id) > 0 )
+        else if ( pfs->Globals().count(f_id) > 0 )
             // The BiF version has an extra "_", per
             // AddBiF(..., true).
             gen = globals[string(id_name) + "_"];
@@ -318,20 +318,25 @@ string CPPCompile::GenInExpr(const Expr* e, GenType gt) {
     auto t1 = op1->GetType();
     auto t2 = op2->GetType();
 
+    auto tag1 = t1->Tag();
+    auto tag2 = t2->Tag();
+
     string gen;
 
-    if ( t1->Tag() == TYPE_PATTERN )
+    if ( tag1 == TYPE_STRING && tag2 == TYPE_TABLE && t2->AsTableType()->IsPatternIndex() )
+        gen = GenExpr(op2, GEN_DONT_CARE) + "->MatchPattern(" + GenExpr(op1, GEN_NATIVE) + ")";
+    else if ( tag1 == TYPE_PATTERN )
         gen = string("(") + GenExpr(op1, GEN_DONT_CARE) + ")->MatchAnywhere(" + GenExpr(op2, GEN_DONT_CARE) +
               "->AsString())";
 
-    else if ( t2->Tag() == TYPE_STRING )
+    else if ( tag2 == TYPE_STRING )
         gen = string("str_in__CPP(") + GenExpr(op1, GEN_DONT_CARE) + "->AsString(), " + GenExpr(op2, GEN_DONT_CARE) +
               "->AsString())";
 
-    else if ( t1->Tag() == TYPE_ADDR && t2->Tag() == TYPE_SUBNET )
+    else if ( tag1 == TYPE_ADDR && tag2 == TYPE_SUBNET )
         gen = string("(") + GenExpr(op2, GEN_DONT_CARE) + ")->Contains(" + GenExpr(op1, GEN_VAL_PTR) + "->Get())";
 
-    else if ( t2->Tag() == TYPE_VECTOR )
+    else if ( tag2 == TYPE_VECTOR )
         gen = GenExpr(op2, GEN_DONT_CARE) + "->Has(" + GenExpr(op1, GEN_NATIVE) + ")";
 
     else
@@ -511,8 +516,8 @@ string CPPCompile::GenAddToExpr(const Expr* e, GenType gt, bool top_level) {
 
         // Make sure any newly created types are known to
         // the profiler.
-        (void)pfs.HashType(rhs->GetType());
-        (void)pfs.HashType(assign->GetType());
+        (void)pfs->HashType(rhs->GetType());
+        (void)pfs->HashType(assign->GetType());
 
         return GenExpr(assign, gt, top_level);
     }
@@ -542,8 +547,8 @@ string CPPCompile::GenRemoveFromExpr(const Expr* e, GenType gt, bool top_level)
 
         // Make sure any newly created types are known to
         // the profiler.
-        (void)pfs.HashType(rhs->GetType());
-        (void)pfs.HashType(assign->GetType());
+        (void)pfs->HashType(rhs->GetType());
+        (void)pfs->HashType(assign->GetType());
 
         return GenExpr(assign, gt, top_level);
     }

src/script_opt/CPP/Inits.cc

@@ -180,7 +180,7 @@ void CPPCompile::InitializeGlobals() {
 
     for ( const auto& ginit : IDOptInfo::GetGlobalInitExprs() ) {
         auto g = ginit.Id();
-        if ( pfs.Globals().count(g) == 0 )
+        if ( pfs->Globals().count(g) == 0 )
             continue;
 
         auto ic = ginit.IC();

src/script_opt/CPP/InitsInfo.cc

@@ -460,12 +460,12 @@ FuncTypeInfo::FuncTypeInfo(CPPCompile* _c, TypePtr _t) : AbstractTypeInfo(_c, st
     params = f->Params();
     yield = f->Yield();
 
-    auto gi = c->RegisterType(f->Params());
+    auto gi = c->RegisterType(params);
     if ( gi )
         init_cohort = gi->InitCohort();
 
     if ( yield ) {
-        gi = c->RegisterType(f->Yield());
+        auto gi = c->RegisterType(f->Yield());
         if ( gi )
             init_cohort = max(init_cohort, gi->InitCohort());
     }

src/script_opt/CPP/InitsInfo.h

@@ -594,7 +594,7 @@ class FuncTypeInfo : public AbstractTypeInfo {
 
 private:
     FunctionFlavor flavor;
-    TypePtr params;
+    RecordTypePtr params;
     TypePtr yield;
 };
 

src/script_opt/CPP/Vars.cc

@@ -9,9 +9,9 @@ using namespace std;
 
 void CPPCompile::CreateGlobal(const ID* g) {
     auto gn = string(g->Name());
-    bool is_bif = pfs.BiFGlobals().count(g) > 0;
+    bool is_bif = pfs->BiFGlobals().count(g) > 0;
 
-    if ( pfs.Globals().count(g) == 0 ) {
+    if ( pfs->Globals().count(g) == 0 ) {
         // Only used in the context of calls.  If it's compilable,
         // then we'll call it directly.
         if ( compilable_funcs.count(gn) > 0 ) {
@@ -28,7 +28,7 @@ void CPPCompile::CreateGlobal(const ID* g) {
     if ( AddGlobal(gn, "gl") ) { // We'll be creating this global.
         Emit("IDPtr %s;", globals[gn]);
 
-        if ( pfs.Events().count(gn) > 0 )
+        if ( pfs->Events().count(gn) > 0 )
             // This is an event that's also used as a variable.
             Emit("EventHandlerPtr %s_ev;", globals[gn]);
 

src/script_opt/CPP/maint/README

@@ -13,15 +13,15 @@ The maintenance workflow:
 
 1.  Make sure the compiler can compile and execute the base scripts:
 
-	echo | src/zeek -O gen-C++
+	src/zeek -O gen-C++ /dev/null
 	ninja
 	src/zeek -O use-C++ -r some.pcap
 
     and that it can compile them standalone:
 
 	rm CPP-gen.cc
 	ninja
-	echo | src/zeek -O gen-standalone-C++
+	src/zeek -O gen-standalone-C++ /dev/null
 	ninja
 	rm CPP-gen.cc
 	ninja