From 5eb37e4c7839f709a9c45ee22f36528394b97904 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Thu, 12 May 2022 13:54:46 -0700 Subject: [PATCH] switch cached Trigger values to be opaque-and-generic, rather than assuming CallExpr's --- src/Expr.cc | 4 ++-- src/Frame.cc | 1 + src/Frame.h | 11 +++++++++-- src/Func.cc | 2 +- src/Trigger.cc | 18 +++++++++--------- src/Trigger.h | 11 ++++++----- src/broker/Store.h | 13 +++++++------ src/broker/store.bif | 10 +++++----- src/zeek.bif | 20 ++++++++++---------- 9 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/Expr.cc b/src/Expr.cc index ae46abf213..390d6f713c 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4605,7 +4605,7 @@ ValPtr CallExpr::Eval(Frame* f) const if ( func_val && v ) { const zeek::Func* funcv = func_val->AsFunc(); - const CallExpr* current_call = f ? f->GetCall() : nullptr; + auto current_assoc = f ? f->GetTriggerAssoc() : nullptr; if ( f ) f->SetCall(this); @@ -4614,7 +4614,7 @@ ValPtr CallExpr::Eval(Frame* f) const ret = funcv->Invoke(&args, f); if ( f ) - f->SetCall(current_call); + f->SetTriggerAssoc(current_assoc); } return ret; diff --git a/src/Frame.cc b/src/Frame.cc index e0165d0b52..a3539157e9 100644 --- a/src/Frame.cc +++ b/src/Frame.cc @@ -201,6 +201,7 @@ Frame* Frame::Clone() const other->CaptureClosure(closure, outer_ids); other->call = call; + other->assoc = assoc; other->trigger = trigger; for ( int i = 0; i < size; i++ ) diff --git a/src/Frame.h b/src/Frame.h index d67929fb2b..64944b7e3e 100644 --- a/src/Frame.h +++ b/src/Frame.h @@ -258,10 +258,16 @@ public: void ClearTrigger(); trigger::Trigger* GetTrigger() const { return trigger.get(); } - void SetCall(const CallExpr* arg_call) { call = arg_call; } - void ClearCall() { call = nullptr; } + void SetCall(const CallExpr* arg_call) + { + call = arg_call; + SetTriggerAssoc((void*)call); + } const CallExpr* GetCall() const { return call; } + void SetTriggerAssoc(const void* arg_assoc) { assoc = arg_assoc; } + const void* GetTriggerAssoc() const { return assoc; } + void SetCallLoc(const Location* loc) { call_loc = loc; } const detail::Location* GetCallLocation() const; @@ -388,6 +394,7 @@ private: trigger::TriggerPtr trigger; const CallExpr* call = nullptr; + const void* assoc = nullptr; const Location* call_loc = nullptr; // only needed if call is nil std::unique_ptr> functions_with_closure_frame_reference; diff --git a/src/Func.cc b/src/Func.cc index 3c44d247c5..432c1b5b70 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -365,7 +365,7 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const if ( parent ) { f->SetTrigger({NewRef{}, parent->GetTrigger()}); - f->SetCall(parent->GetCall()); + f->SetTriggerAssoc(parent->GetTriggerAssoc()); } g_frame_stack.push_back(f.get()); // used for backtracing diff --git a/src/Trigger.cc b/src/Trigger.cc index 9be2bed8d0..91aaff5003 100644 --- a/src/Trigger.cc +++ b/src/Trigger.cc @@ -350,7 +350,7 @@ bool Trigger::Eval() { Trigger* trigger = frame->GetTrigger(); assert(trigger); - assert(frame->GetCall()); + assert(frame->GetTriggerAssoc()); assert(trigger->attached == this); #ifdef DEBUG @@ -359,7 +359,7 @@ bool Trigger::Eval() delete[] pname; #endif - auto queued = trigger->Cache(frame->GetCall(), v.get()); + auto queued = trigger->Cache(frame->GetTriggerAssoc(), v.get()); trigger->Release(); frame->ClearTrigger(); @@ -404,7 +404,7 @@ void Trigger::Timeout() { Trigger* trigger = frame->GetTrigger(); assert(trigger); - assert(frame->GetCall()); + assert(frame->GetTriggerAssoc()); assert(trigger->attached == this); #ifdef DEBUG @@ -413,7 +413,7 @@ void Trigger::Timeout() pname); delete[] pname; #endif - auto queued = trigger->Cache(frame->GetCall(), v.get()); + auto queued = trigger->Cache(frame->GetTriggerAssoc(), v.get()); trigger->Release(); frame->ClearTrigger(); @@ -480,12 +480,12 @@ void Trigger::Attach(Trigger* trigger) Hold(); } -bool Trigger::Cache(const CallExpr* expr, Val* v) +bool Trigger::Cache(const void* obj, Val* v) { if ( disabled || ! v ) return false; - ValCache::iterator i = cache.find(expr); + ValCache::iterator i = cache.find(obj); if ( i != cache.end() ) { @@ -494,7 +494,7 @@ bool Trigger::Cache(const CallExpr* expr, Val* v) } else - cache.insert(ValCache::value_type(expr, v)); + cache.insert(ValCache::value_type(obj, v)); Ref(v); @@ -502,11 +502,11 @@ bool Trigger::Cache(const CallExpr* expr, Val* v) return true; } -Val* Trigger::Lookup(const CallExpr* expr) +Val* Trigger::Lookup(const void* obj) { assert(! disabled); - ValCache::iterator i = cache.find(expr); + ValCache::iterator i = cache.find(obj); return (i != cache.end()) ? i->second : 0; } diff --git a/src/Trigger.h b/src/Trigger.h index 833b3f05bc..5b8f84f770 100644 --- a/src/Trigger.h +++ b/src/Trigger.h @@ -25,7 +25,6 @@ namespace detail class Frame; class Stmt; class Expr; -class CallExpr; class ID; class WhenInfo; @@ -86,9 +85,11 @@ public: // Cache for return values of delayed function calls. Returns whether // the trigger is queued for later evaluation -- it may not be queued - // if the Val is null or it's disabled. - bool Cache(const CallExpr* expr, Val* val); - Val* Lookup(const CallExpr*); + // if the Val is null or it's disabled. The cache is managed using + // void*'s so that the value can be associated with either a CallExpr + // (for interpreted execution) or a C++ function (for compiled-to-C++). + bool Cache(const void* obj, Val* val); + Val* Lookup(const void*); // Disable this trigger completely. Needed because Unref'ing the trigger // may not immediately delete it as other references may still exist. @@ -153,7 +154,7 @@ private: std::vector> objs; - using ValCache = std::map; + using ValCache = std::map; ValCache cache; }; diff --git a/src/broker/Store.h b/src/broker/Store.h index 10ebad95e5..dd6f2c14af 100644 --- a/src/broker/Store.h +++ b/src/broker/Store.h @@ -5,6 +5,7 @@ #include #include +#include "zeek/Expr.h" #include "zeek/OpaqueVal.h" #include "zeek/Trigger.h" #include "zeek/broker/data.bif.h" @@ -72,9 +73,9 @@ static std::optional convert_expiry(double e) class StoreQueryCallback { public: - StoreQueryCallback(zeek::detail::trigger::Trigger* arg_trigger, - const zeek::detail::CallExpr* arg_call, broker::store store) - : trigger(arg_trigger), call(arg_call), store(std::move(store)) + StoreQueryCallback(zeek::detail::trigger::Trigger* arg_trigger, const void* arg_assoc, + broker::store store) + : trigger(arg_trigger), assoc(arg_assoc), store(std::move(store)) { Ref(trigger); } @@ -83,14 +84,14 @@ public: void Result(const RecordValPtr& result) { - trigger->Cache(call, result.get()); + trigger->Cache(assoc, result.get()); trigger->Release(); } void Abort() { auto result = query_result(); - trigger->Cache(call, result.get()); + trigger->Cache(assoc, result.get()); trigger->Release(); } @@ -100,7 +101,7 @@ public: private: zeek::detail::trigger::Trigger* trigger; - const zeek::detail::CallExpr* call; + const void* assoc; broker::store store; }; diff --git a/src/broker/store.bif b/src/broker/store.bif index d29a44191d..8d4bf79dc3 100644 --- a/src/broker/store.bif +++ b/src/broker/store.bif @@ -156,7 +156,7 @@ function Broker::__exists%(h: opaque of Broker::Store, frame->SetDelayed(); trigger->Hold(); - auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetCall(), + auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetTriggerAssoc(), handle->store); auto req_id = handle->proxy.exists(std::move(*key)); broker_mgr->TrackStoreQuery(handle, req_id, cb); @@ -202,7 +202,7 @@ function Broker::__get%(h: opaque of Broker::Store, frame->SetDelayed(); trigger->Hold(); - auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetCall(), + auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetTriggerAssoc(), handle->store); auto req_id = handle->proxy.get(std::move(*key)); broker_mgr->TrackStoreQuery(handle, req_id, cb); @@ -255,7 +255,7 @@ function Broker::__put_unique%(h: opaque of Broker::Store, frame->SetDelayed(); trigger->Hold(); - auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetCall(), + auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetTriggerAssoc(), handle->store); auto req_id = handle->proxy.put_unique(std::move(*key), std::move(*val), @@ -311,7 +311,7 @@ function Broker::__get_index_from_value%(h: opaque of Broker::Store, frame->SetDelayed(); trigger->Hold(); - auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetCall(), + auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetTriggerAssoc(), handle->store); auto req_id = handle->proxy.get_index_from_value(std::move(*key), std::move(*index)); @@ -355,7 +355,7 @@ function Broker::__keys%(h: opaque of Broker::Store%): Broker::QueryResult frame->SetDelayed(); trigger->Hold(); - auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetCall(), + auto cb = new zeek::Broker::detail::StoreQueryCallback(trigger, frame->GetTriggerAssoc(), handle->store); auto req_id = handle->proxy.keys(); broker_mgr->TrackStoreQuery(handle, req_id, cb); diff --git a/src/zeek.bif b/src/zeek.bif index a1fc14b26c..785f606973 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -3704,11 +3704,11 @@ function dump_packet%(pkt: pcap_packet, file_name: string%) : bool class LookupHostCallback : public zeek::detail::DNS_Mgr::LookupCallback { public: LookupHostCallback(zeek::detail::trigger::Trigger* arg_trigger, - const zeek::detail::CallExpr* arg_call, bool arg_lookup_name) + const void* arg_assoc, bool arg_lookup_name) { Ref(arg_trigger); trigger = arg_trigger; - call = arg_call; + assoc = arg_assoc; lookup_name = arg_lookup_name; } @@ -3721,7 +3721,7 @@ public: void Resolved(const std::string& name) override { zeek::Val* result = new zeek::StringVal(name); - trigger->Cache(call, result); + trigger->Cache(assoc, result); Unref(result); trigger->Release(); } @@ -3729,7 +3729,7 @@ public: void Resolved(zeek::TableValPtr addrs) override { // No Ref() for addrs. - trigger->Cache(call, addrs.get()); + trigger->Cache(assoc, addrs.get()); trigger->Release(); } @@ -3738,7 +3738,7 @@ public: if ( lookup_name ) { zeek::Val* result = new zeek::StringVal("<\?\?\?>"); - trigger->Cache(call, result); + trigger->Cache(assoc, result); Unref(result); } @@ -3747,7 +3747,7 @@ public: auto* lv = new zeek::ListVal(zeek::TYPE_ADDR); lv->Append(zeek::make_intrusive("0.0.0.0")); auto result = lv->ToSetVal(); - trigger->Cache(call, result.get()); + trigger->Cache(assoc, result.get()); Unref(lv); } @@ -3756,7 +3756,7 @@ public: private: zeek::detail::trigger::Trigger* trigger; - const zeek::detail::CallExpr* call; + const void* assoc; bool lookup_name; }; %%} @@ -3786,7 +3786,7 @@ function lookup_addr%(host: addr%) : string trigger->Hold(); zeek::detail::dns_mgr->LookupAddr(host->AsAddr(), - new LookupHostCallback(trigger, frame->GetCall(), true)); + new LookupHostCallback(trigger, frame->GetTriggerAssoc(), true)); return nullptr; %} @@ -3815,7 +3815,7 @@ function lookup_hostname_txt%(host: string%) : string trigger->Hold(); zeek::detail::dns_mgr->Lookup(host->CheckString(), T_TXT, - new LookupHostCallback(trigger, frame->GetCall(), true)); + new LookupHostCallback(trigger, frame->GetTriggerAssoc(), true)); return nullptr; %} @@ -3844,7 +3844,7 @@ function lookup_hostname%(host: string%) : addr_set trigger->Hold(); zeek::detail::dns_mgr->LookupHost(host->CheckString(), - new LookupHostCallback(trigger, frame->GetCall(), false)); + new LookupHostCallback(trigger, frame->GetTriggerAssoc(), false)); return nullptr; %}