diff --git a/CHANGES b/CHANGES index 91e06a5eec..eba14d08fb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,25 @@ +5.0.0-dev.523 | 2022-05-31 11:13:47 -0700 + + * Update spicy-plugin with change that checks for zeek version (Tim Wojtulewicz, Corelight) + + * deprecation messages for unused base script functions (Vern Paxson, Corelight) + + * clearer messages for warning about unused functions (Vern Paxson, Corelight) + + * annotate orphan base script components with &deprecated (Vern Paxson, Corelight) + + * annotate base scripts with &is_used as needed (Vern Paxson, Corelight) + + * --no-usage-warnings flag to suppress analysis (Vern Paxson, Corelight) + + * support for associating &is_used attributes with functions (Vern Paxson, Corelight) + + * classes for evaluating function/hook/event usage (Vern Paxson, Corelight) + + * broader support for AST traversal, including Attr and Attributes objects (Vern Paxson, Corelight) + + * include attributes in descriptions of sets and tables (Vern Paxson, Corelight) + 5.0.0-dev.508 | 2022-05-27 14:33:47 -0700 * Update zeek-aux submodule to fix a compiler warning (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index 5ec569752e..3742370fc2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.0.0-dev.508 +5.0.0-dev.523 diff --git a/auxil/spicy-plugin b/auxil/spicy-plugin index 945b5994de..454abb71af 160000 --- a/auxil/spicy-plugin +++ b/auxil/spicy-plugin @@ -1 +1 @@ -Subproject commit 945b5994de4fb85bc015ac9da0921f162ea0b8e9 +Subproject commit 454abb71afb3ea23111d3fafabaa9a1ae37afcaf diff --git a/scripts/base/frameworks/config/main.zeek b/scripts/base/frameworks/config/main.zeek index 47525ee7b7..76744a9c4f 100644 --- a/scripts/base/frameworks/config/main.zeek +++ b/scripts/base/frameworks/config/main.zeek @@ -57,7 +57,7 @@ global option_cache: table[string] of OptionCacheValue; global Config::cluster_set_option: event(ID: string, val: any, location: string); -function broadcast_option(ID: string, val: any, location: string) +function broadcast_option(ID: string, val: any, location: string) &is_used { # There's not currently a common topic to broadcast to as then enabling # implicit Broker forwarding would cause a routing loop. @@ -145,7 +145,7 @@ function format_value(value: any) : string return cat(value); } -function config_option_changed(ID: string, new_value: any, location: string): any +function config_option_changed(ID: string, new_value: any, location: string): any &is_used { local log = Info($ts=network_time(), $id=ID, $old_value=format_value(lookup_ID(ID)), $new_value=format_value(new_value)); if ( location != "" ) diff --git a/scripts/base/frameworks/control/main.zeek b/scripts/base/frameworks/control/main.zeek index 20cb93c234..61bee75f37 100644 --- a/scripts/base/frameworks/control/main.zeek +++ b/scripts/base/frameworks/control/main.zeek @@ -74,7 +74,7 @@ export { global shutdown_response: event(); } -event terminate_event() +event terminate_event() &is_used { terminate(); } diff --git a/scripts/base/frameworks/intel/cluster.zeek b/scripts/base/frameworks/intel/cluster.zeek index 2d51ffb200..53ac323fa5 100644 --- a/scripts/base/frameworks/intel/cluster.zeek +++ b/scripts/base/frameworks/intel/cluster.zeek @@ -6,9 +6,10 @@ module Intel; -# Internal events for cluster data distribution. -global insert_item: event(item: Item); -global insert_indicator: event(item: Item); +# Internal events for cluster data distribution. Marked as &is_used since +# they're communicated via Broker. +global insert_item: event(item: Item) &is_used; +global insert_indicator: event(item: Item) &is_used; # If this process is not a manager process, we don't want the full metadata. @if ( Cluster::local_node_type() != Cluster::MANAGER ) diff --git a/scripts/base/frameworks/logging/main.zeek b/scripts/base/frameworks/logging/main.zeek index 6c57173a47..1e611b5d75 100644 --- a/scripts/base/frameworks/logging/main.zeek +++ b/scripts/base/frameworks/logging/main.zeek @@ -587,7 +587,7 @@ global filters: table[ID, string] of Filter; module Log; # Used internally by the log manager. -function __default_rotation_postprocessor(info: RotationInfo) : bool +function __default_rotation_postprocessor(info: RotationInfo) : bool &is_used { if ( info$writer in default_rotation_postprocessors ) return default_rotation_postprocessors[info$writer](info); diff --git a/scripts/base/frameworks/notice/main.zeek b/scripts/base/frameworks/notice/main.zeek index 5908746b7b..b5d3ec4a32 100644 --- a/scripts/base/frameworks/notice/main.zeek +++ b/scripts/base/frameworks/notice/main.zeek @@ -585,7 +585,7 @@ function is_being_suppressed(n: Notice::Info): bool # Executes a script with all of the notice fields put into the # new process' environment as "ZEEK_ARG_" variables. -function execute_with_notice(cmd: string, n: Notice::Info) +function execute_with_notice(cmd: string, n: Notice::Info) &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { # TODO: fix system calls #local tgs = tags(n); diff --git a/scripts/base/frameworks/signatures/main.zeek b/scripts/base/frameworks/signatures/main.zeek index a610afccf5..51760b5d1f 100644 --- a/scripts/base/frameworks/signatures/main.zeek +++ b/scripts/base/frameworks/signatures/main.zeek @@ -150,7 +150,7 @@ event zeek_init() &priority=5 # Returns true if the given signature has already been triggered for the given # [orig, resp] pair. -function has_signature_matched(id: string, orig: addr, resp: addr): bool +function has_signature_matched(id: string, orig: addr, resp: addr): bool &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { return [orig, resp] in vert_table ? id in vert_table[orig, resp] : F; } diff --git a/scripts/base/frameworks/software/main.zeek b/scripts/base/frameworks/software/main.zeek index b6a86ba644..7e7631a4f3 100644 --- a/scripts/base/frameworks/software/main.zeek +++ b/scripts/base/frameworks/software/main.zeek @@ -454,13 +454,14 @@ function cmp_versions(v1: Version, v2: Version): int return 0; } -function software_endpoint_name(id: conn_id, host: addr): string +function software_endpoint_name(id: conn_id, host: addr): string &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { return fmt("%s %s", host, (host == id$orig_h ? "client" : "server")); } -# Convert a version into a string "a.b.c-x". -function software_fmt_version(v: Version): string +# Convert a version into a string "a.b.c-x". Marked "&is_used" because +# while the base scripts don't call it, the optional policy/ scripts do. +function software_fmt_version(v: Version): string &is_used { return fmt("%s%s%s%s%s", v?$major ? fmt("%d", v$major) : "0", @@ -470,8 +471,8 @@ function software_fmt_version(v: Version): string v?$addl ? fmt("-%s", v$addl) : ""); } -# Convert a software into a string "name a.b.cx". -function software_fmt(i: Info): string +# Convert a software into a string "name a.b.cx". Same as above re "&is_used". +function software_fmt(i: Info): string &is_used { return fmt("%s %s", i$name, software_fmt_version(i$version)); } diff --git a/scripts/base/frameworks/sumstats/main.zeek b/scripts/base/frameworks/sumstats/main.zeek index 8f8638b57b..feb8154ff3 100644 --- a/scripts/base/frameworks/sumstats/main.zeek +++ b/scripts/base/frameworks/sumstats/main.zeek @@ -330,7 +330,7 @@ function compose_resultvals(rv1: ResultVal, rv2: ResultVal): ResultVal return result; } -function compose_results(r1: Result, r2: Result): Result +function compose_results(r1: Result, r2: Result): Result &is_used { local result: Result = table(); diff --git a/scripts/base/frameworks/sumstats/non-cluster.zeek b/scripts/base/frameworks/sumstats/non-cluster.zeek index 3059d78f26..5bf615fcaf 100644 --- a/scripts/base/frameworks/sumstats/non-cluster.zeek +++ b/scripts/base/frameworks/sumstats/non-cluster.zeek @@ -71,7 +71,7 @@ function data_added(ss: SumStat, key: Key, result: Result) threshold_crossed(ss, key, result); } -function request(ss_name: string): ResultTable +function request(ss_name: string): ResultTable &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { # This only needs to be implemented this way for cluster compatibility. return when [ss_name] ( T ) diff --git a/scripts/base/frameworks/sumstats/plugins/std-dev.zeek b/scripts/base/frameworks/sumstats/plugins/std-dev.zeek index bfb02c82cc..a181923b10 100644 --- a/scripts/base/frameworks/sumstats/plugins/std-dev.zeek +++ b/scripts/base/frameworks/sumstats/plugins/std-dev.zeek @@ -23,7 +23,7 @@ function calc_std_dev(rv: ResultVal) rv$std_dev = sqrt(rv$variance); } -hook std_dev_hook(r: Reducer, val: double, obs: Observation, rv: ResultVal) +hook std_dev_hook(r: Reducer, val: double, obs: Observation, rv: ResultVal) &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { calc_std_dev(rv); } diff --git a/scripts/base/misc/find-filtered-trace.zeek b/scripts/base/misc/find-filtered-trace.zeek index 7d25e70a6f..511db74b10 100644 --- a/scripts/base/misc/find-filtered-trace.zeek +++ b/scripts/base/misc/find-filtered-trace.zeek @@ -13,7 +13,7 @@ export { global enable: bool = T &redef; } -function should_detect(): bool +function should_detect(): bool &is_used { local args = zeek_args(); diff --git a/scripts/base/protocols/ftp/main.zeek b/scripts/base/protocols/ftp/main.zeek index 9c00dbb475..f9da769fea 100644 --- a/scripts/base/protocols/ftp/main.zeek +++ b/scripts/base/protocols/ftp/main.zeek @@ -71,7 +71,7 @@ event zeek_init() &priority=5 # Establish the variable for tracking expected connections. global ftp_data_expected: table[addr, port] of Info &read_expire=5mins; -function minimize_info(info: Info): Info +function minimize_info(info: Info): Info &is_used { # Just minimal data for sending to other remote Zeek processes. # Generally, only data that's consistent across an entire FTP session or @@ -103,7 +103,7 @@ const directory_cmds = { ["XPWD", 257], }; -function ftp_relay_topic(): string +function ftp_relay_topic(): string &is_used { local rval = Cluster::rr_topic(Cluster::proxy_pool, "ftp_transfer_rr_key"); @@ -176,7 +176,7 @@ function ftp_message(s: Info) delete s$data_channel; } -event sync_add_expected_data(s: Info, chan: ExpectedDataChannel) +event sync_add_expected_data(s: Info, chan: ExpectedDataChannel) &is_used { @if ( Cluster::local_node_type() == Cluster::PROXY || Cluster::local_node_type() == Cluster::MANAGER ) @@ -189,7 +189,7 @@ event sync_add_expected_data(s: Info, chan: ExpectedDataChannel) @endif } -event sync_remove_expected_data(resp_h: addr, resp_p: port) +event sync_remove_expected_data(resp_h: addr, resp_p: port) &is_used { @if ( Cluster::local_node_type() == Cluster::PROXY || Cluster::local_node_type() == Cluster::MANAGER ) diff --git a/scripts/base/protocols/ftp/utils-commands.zeek b/scripts/base/protocols/ftp/utils-commands.zeek index 5b75bd5032..5cbc4515b1 100644 --- a/scripts/base/protocols/ftp/utils-commands.zeek +++ b/scripts/base/protocols/ftp/utils-commands.zeek @@ -133,7 +133,7 @@ function remove_pending_cmd(pc: PendingCmds, ca: CmdArg): bool return F; } -function pop_pending_cmd(pc: PendingCmds, reply_code: count, reply_msg: string): CmdArg +function pop_pending_cmd(pc: PendingCmds, reply_code: count, reply_msg: string): CmdArg &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { local ca = get_pending_cmd(pc, reply_code, reply_msg); remove_pending_cmd(pc, ca); diff --git a/scripts/base/protocols/irc/dcc-send.zeek b/scripts/base/protocols/irc/dcc-send.zeek index 965f76b7fd..9687a4698e 100644 --- a/scripts/base/protocols/irc/dcc-send.zeek +++ b/scripts/base/protocols/irc/dcc-send.zeek @@ -33,7 +33,7 @@ export { global dcc_expected_transfers: table[addr, port] of Info &read_expire=5mins; -function dcc_relay_topic(): string +function dcc_relay_topic(): string &is_used { local rval = Cluster::rr_topic(Cluster::proxy_pool, "dcc_transfer_rr_key"); @@ -44,7 +44,7 @@ function dcc_relay_topic(): string return rval; } -event dcc_transfer_add(host: addr, p: port, info: Info) +event dcc_transfer_add(host: addr, p: port, info: Info) &is_used { @if ( Cluster::local_node_type() == Cluster::PROXY || Cluster::local_node_type() == Cluster::MANAGER ) @@ -56,7 +56,7 @@ event dcc_transfer_add(host: addr, p: port, info: Info) @endif } -event dcc_transfer_remove(host: addr, p: port) +event dcc_transfer_remove(host: addr, p: port) &is_used { @if ( Cluster::local_node_type() == Cluster::PROXY || Cluster::local_node_type() == Cluster::MANAGER ) diff --git a/scripts/base/protocols/radius/consts.zeek b/scripts/base/protocols/radius/consts.zeek index 2401998c87..06cb014e31 100644 --- a/scripts/base/protocols/radius/consts.zeek +++ b/scripts/base/protocols/radius/consts.zeek @@ -182,7 +182,7 @@ const attr_types: table[count] of string = { [171] = "Delegated-IPv6-Prefix-Pool", [172] = "Stateful-IPv6-Address-Pool", [173] = "IPv6-6rd-Configuration" -} &default=function(i: count): string { return fmt("unknown-%d", i); }; +} &default=function(i: count): string { return fmt("unknown-%d", i); } &deprecated="Remove in v6.1. Usage testing indicates this function is unused."; const nas_port_types: table[count] of string = { [0] = "Async", @@ -205,7 +205,7 @@ const nas_port_types: table[count] of string = { [17] = "Cable", [18] = "Wireless - Other", [19] = "Wireless - IEEE 802.11" -} &default=function(i: count): string { return fmt("unknown-%d", i); }; +} &default=function(i: count): string { return fmt("unknown-%d", i); } &deprecated="Remove in v6.1. Usage testing indicates this function is unused."; const service_types: table[count] of string = { [1] = "Login", @@ -219,7 +219,7 @@ const service_types: table[count] of string = { [9] = "Callback NAS Prompt", [10] = "Call Check", [11] = "Callback Administrative", -} &default=function(i: count): string { return fmt("unknown-%d", i); }; +} &default=function(i: count): string { return fmt("unknown-%d", i); } &deprecated="Remove in v6.1. Usage testing indicates this function is unused."; const framed_protocol_types: table[count] of string = { [1] = "PPP", @@ -228,4 +228,4 @@ const framed_protocol_types: table[count] of string = { [4] = "Gandalf proprietary SingleLink/MultiLink protocol", [5] = "Xylogics proprietary IPX/SLIP", [6] = "X.75 Synchronous" -} &default=function(i: count): string { return fmt("unknown-%d", i); }; +} &default=function(i: count): string { return fmt("unknown-%d", i); } &deprecated="Remove in v6.1. Usage testing indicates this function is unused."; diff --git a/scripts/base/protocols/smb/smb1-main.zeek b/scripts/base/protocols/smb/smb1-main.zeek index a6bfcc8035..9a19f27b95 100644 --- a/scripts/base/protocols/smb/smb1-main.zeek +++ b/scripts/base/protocols/smb/smb1-main.zeek @@ -271,7 +271,7 @@ event smb1_write_andx_request(c: connection, hdr: SMB1::Header, file_id: count, c$smb_state$pipe_map[file_id] = c$smb_state$current_file$uuid; } -event smb_pipe_bind_ack_response(c: connection, hdr: SMB1::Header) +event smb_pipe_bind_ack_response(c: connection, hdr: SMB1::Header) &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { if ( ! c$smb_state?$current_file || ! c$smb_state$current_file?$uuid ) { @@ -283,7 +283,7 @@ event smb_pipe_bind_ack_response(c: connection, hdr: SMB1::Header) c$smb_state$current_cmd$argument = SMB::rpc_uuids[c$smb_state$current_file$uuid]; } -event smb_pipe_bind_request(c: connection, hdr: SMB1::Header, uuid: string, version: string) +event smb_pipe_bind_request(c: connection, hdr: SMB1::Header, uuid: string, version: string) &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { if ( ! c$smb_state?$current_file || ! c$smb_state$current_file?$uuid ) { @@ -296,7 +296,7 @@ event smb_pipe_bind_request(c: connection, hdr: SMB1::Header, uuid: string, vers c$smb_state$current_cmd$argument = fmt("%s v%s", SMB::rpc_uuids[uuid], version); } -event smb_pipe_request(c: connection, hdr: SMB1::Header, op_num: count) +event smb_pipe_request(c: connection, hdr: SMB1::Header, op_num: count) &deprecated="Remove in v6.1. Usage testing indicates this function is unused." { if ( ! c$smb_state?$current_file ) { diff --git a/src/Attr.cc b/src/Attr.cc index da28845fe9..7e80736a31 100644 --- a/src/Attr.cc +++ b/src/Attr.cc @@ -166,6 +166,21 @@ void Attr::AddTag(ODesc* d) const d->Add(attr_name(Tag())); } +detail::TraversalCode Attr::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreAttr(this); + HANDLE_TC_ATTR_PRE(tc); + + if ( expr ) + { + auto tc = expr->Traverse(cb); + HANDLE_TC_ATTR_PRE(tc); + } + + tc = cb->PostAttr(this); + HANDLE_TC_ATTR_POST(tc); + } + Attributes::Attributes(TypePtr t, bool arg_in_record, bool is_global) : Attributes(std::vector{}, std::move(t), arg_in_record, is_global) { @@ -756,4 +771,19 @@ bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_r return false; } +detail::TraversalCode Attributes::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreAttrs(this); + HANDLE_TC_ATTRS_PRE(tc); + + for ( const auto& a : attrs ) + { + tc = a->Traverse(cb); + HANDLE_TC_ATTRS_PRE(tc); + } + + tc = cb->PostAttrs(this); + HANDLE_TC_ATTRS_POST(tc); + } + } diff --git a/src/Attr.h b/src/Attr.h index 6bc37ea726..780c39ac47 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -7,6 +7,7 @@ #include "zeek/IntrusivePtr.h" #include "zeek/Obj.h" +#include "zeek/Traverse.h" #include "zeek/ZeekList.h" // Note that there are two kinds of attributes: the kind (here) which @@ -98,6 +99,8 @@ public: return true; } + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const; + protected: void AddTag(ODesc* d) const; @@ -129,6 +132,8 @@ public: bool operator==(const Attributes& other) const; + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const; + protected: void CheckAttr(Attr* attr); diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index cc3442e850..31ec8e869d 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -398,6 +398,7 @@ set(MAIN_SRCS script_opt/ScriptOpt.cc script_opt/Stmt.cc script_opt/TempVar.cc + script_opt/UsageAnalyzer.cc script_opt/UseDefs.cc script_opt/ZAM/AM-Opt.cc diff --git a/src/Event.cc b/src/Event.cc index 0ae98f8fab..8759eeed74 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -15,7 +15,6 @@ #include "zeek/plugin/Manager.h" zeek::EventMgr zeek::event_mgr; -zeek::EventMgr& mgr = zeek::event_mgr; namespace zeek { diff --git a/src/EventHandler.h b/src/EventHandler.h index f47ff173d8..64f134786e 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -22,7 +22,7 @@ public: const char* Name() const { return name.data(); } - const FuncPtr& GetFunc() { return local; } + const FuncPtr& GetFunc() const { return local; } const FuncTypePtr& GetType(bool check_export = true); diff --git a/src/EventRegistry.cc b/src/EventRegistry.cc index ce3bae3307..1775fc57ab 100644 --- a/src/EventRegistry.cc +++ b/src/EventRegistry.cc @@ -11,7 +11,7 @@ namespace zeek EventRegistry::EventRegistry() = default; EventRegistry::~EventRegistry() noexcept = default; -EventHandlerPtr EventRegistry::Register(std::string_view name) +EventHandlerPtr EventRegistry::Register(std::string_view name, bool is_from_script) { // If there already is an entry in the registry, we have a // local handler on the script layer. @@ -19,21 +19,29 @@ EventHandlerPtr EventRegistry::Register(std::string_view name) if ( h ) { + if ( ! is_from_script ) + not_only_from_script.insert(std::string(name)); + h->SetUsed(); return h; } h = new EventHandler(std::string(name)); - event_registry->Register(h); + event_registry->Register(h, is_from_script); h->SetUsed(); return h; } -void EventRegistry::Register(EventHandlerPtr handler) +void EventRegistry::Register(EventHandlerPtr handler, bool is_from_script) { - handlers[std::string(handler->Name())] = std::unique_ptr(handler.Ptr()); + std::string name = handler->Name(); + + handlers[name] = std::unique_ptr(handler.Ptr()); + + if ( ! is_from_script ) + not_only_from_script.insert(name); } EventHandler* EventRegistry::Lookup(std::string_view name) @@ -45,6 +53,11 @@ EventHandler* EventRegistry::Lookup(std::string_view name) return nullptr; } +bool EventRegistry::NotOnlyRegisteredFromScript(std::string_view name) + { + return not_only_from_script.count(std::string(name)) > 0; + } + EventRegistry::string_list EventRegistry::Match(RE_Matcher* pattern) { string_list names; diff --git a/src/EventRegistry.h b/src/EventRegistry.h index a0dd10d69e..70f5e1cf7d 100644 --- a/src/EventRegistry.h +++ b/src/EventRegistry.h @@ -8,6 +8,7 @@ #include #include #include +#include #include namespace zeek @@ -28,15 +29,21 @@ public: * Performs a lookup for an existing event handler and returns it * if one exists, or else creates one, registers it, and returns it. * @param name The name of the event handler to lookup/register. + * @param name Whether the registration is coming from a script element. * @return The event handler. */ - EventHandlerPtr Register(std::string_view name); + EventHandlerPtr Register(std::string_view name, bool is_from_script = false); - void Register(EventHandlerPtr handler); + void Register(EventHandlerPtr handler, bool is_from_script = false); // Return nil if unknown. EventHandler* Lookup(std::string_view name); + // True if the given event handler (1) exists, and (2) was registered + // in a non-script context (even if perhaps also registered in a script + // context). + bool NotOnlyRegisteredFromScript(std::string_view name); + // Returns a list of all local handlers that match the given pattern. // Passes ownership of list. using string_list = std::vector; @@ -55,6 +62,9 @@ public: private: std::map, std::less<>> handlers; + // Tracks whether a given event handler was registered in a + // non-script context. + std::unordered_set not_only_from_script; }; extern EventRegistry* event_registry; diff --git a/src/Expr.cc b/src/Expr.cc index 98fcb637ec..78e0449780 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -3716,6 +3716,24 @@ TableConstructorExpr::TableConstructorExpr(ListExprPtr constructor_list, } } +TraversalCode TableConstructorExpr::Traverse(TraversalCallback* cb) const + { + TraversalCode tc = cb->PreExpr(this); + HANDLE_TC_EXPR_PRE(tc); + + tc = op->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + + if ( attrs ) + { + tc = attrs->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + } + + tc = cb->PostExpr(this); + HANDLE_TC_EXPR_POST(tc); + } + ValPtr TableConstructorExpr::Eval(Frame* f) const { if ( IsError() ) @@ -3752,6 +3770,9 @@ void TableConstructorExpr::ExprDescribe(ODesc* d) const d->Add("table("); op->Describe(d); d->Add(")"); + + if ( attrs ) + attrs->Describe(d); } SetConstructorExpr::SetConstructorExpr(ListExprPtr constructor_list, @@ -3831,6 +3852,24 @@ SetConstructorExpr::SetConstructorExpr(ListExprPtr constructor_list, } } +TraversalCode SetConstructorExpr::Traverse(TraversalCallback* cb) const + { + TraversalCode tc = cb->PreExpr(this); + HANDLE_TC_EXPR_PRE(tc); + + tc = op->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + + if ( attrs ) + { + tc = attrs->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + } + + tc = cb->PostExpr(this); + HANDLE_TC_EXPR_POST(tc); + } + ValPtr SetConstructorExpr::Eval(Frame* f) const { if ( IsError() ) @@ -3853,6 +3892,9 @@ void SetConstructorExpr::ExprDescribe(ODesc* d) const d->Add("set("); op->Describe(d); d->Add(")"); + + if ( attrs ) + attrs->Describe(d); } VectorConstructorExpr::VectorConstructorExpr(ListExprPtr constructor_list, TypePtr arg_type) @@ -4694,15 +4736,15 @@ LambdaExpr::LambdaExpr(std::unique_ptr arg_ing, IDPList ar } // Install that in the global_scope - auto id = install_ID(my_name.c_str(), current_module.c_str(), true, false); + lambda_id = install_ID(my_name.c_str(), current_module.c_str(), true, false); // Update lamb's name dummy_func->SetName(my_name.c_str()); auto v = make_intrusive(std::move(dummy_func)); - id->SetVal(std::move(v)); - id->SetType(ingredients->id->GetType()); - id->SetConst(); + lambda_id->SetVal(std::move(v)); + lambda_id->SetType(ingredients->id->GetType()); + lambda_id->SetConst(); } void LambdaExpr::CheckCaptures(StmtPtr when_parent) @@ -4817,8 +4859,11 @@ TraversalCode LambdaExpr::Traverse(TraversalCallback* cb) const TraversalCode tc = cb->PreExpr(this); HANDLE_TC_EXPR_PRE(tc); + tc = lambda_id->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + tc = ingredients->body->Traverse(cb); - HANDLE_TC_STMT_PRE(tc); + HANDLE_TC_EXPR_PRE(tc); tc = cb->PostExpr(this); HANDLE_TC_EXPR_POST(tc); @@ -4832,7 +4877,7 @@ EventExpr::EventExpr(const char* arg_name, ListExprPtr arg_args) if ( ! h ) { h = new EventHandler(name.c_str()); - event_registry->Register(h); + event_registry->Register(h, true); } h->SetUsed(); @@ -4889,6 +4934,22 @@ TraversalCode EventExpr::Traverse(TraversalCallback* cb) const TraversalCode tc = cb->PreExpr(this); HANDLE_TC_EXPR_PRE(tc); + auto& f = handler->GetFunc(); + if ( f ) + { + // We don't traverse the function, because that can lead + // to infinite traversals. We do, however, see if we can + // locate the corresponding identifier, and traverse that. + + auto& id = lookup_ID(f->Name(), GLOBAL_MODULE_NAME, false, false, false); + + if ( id ) + { + tc = id->Traverse(cb); + HANDLE_TC_EXPR_PRE(tc); + } + } + tc = args->Traverse(cb); HANDLE_TC_EXPR_PRE(tc); diff --git a/src/Expr.h b/src/Expr.h index d000d88538..6341a659ad 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -1208,6 +1208,8 @@ public: ValPtr Eval(Frame* f) const override; + TraversalCode Traverse(TraversalCallback* cb) const override; + // Optimization-related: ExprPtr Duplicate() override; @@ -1232,6 +1234,8 @@ public: ValPtr Eval(Frame* f) const override; + TraversalCode Traverse(TraversalCallback* cb) const override; + // Optimization-related: ExprPtr Duplicate() override; @@ -1479,6 +1483,7 @@ private: void CheckCaptures(StmtPtr when_parent); std::unique_ptr ingredients; + IDPtr lambda_id; IDPList outer_ids; bool capture_by_ref; // if true, use deprecated reference semantics diff --git a/src/Func.cc b/src/Func.cc index 432c1b5b70..5b39e453aa 100644 --- a/src/Func.cc +++ b/src/Func.cc @@ -864,7 +864,7 @@ static int get_func_priority(const std::vector& attrs) for ( const auto& a : attrs ) { - if ( a->Tag() == ATTR_DEPRECATED ) + if ( a->Tag() == ATTR_DEPRECATED || a->Tag() == ATTR_IS_USED ) continue; if ( a->Tag() != ATTR_PRIORITY ) @@ -903,7 +903,23 @@ function_ingredients::function_ingredients(ScopePtr scope, StmtPtr body) const auto& attrs = this->scope->Attrs(); - priority = (attrs ? get_func_priority(*attrs) : 0); + if ( attrs ) + { + priority = get_func_priority(*attrs); + + for ( const auto& a : *attrs ) + if ( a->Tag() == ATTR_IS_USED ) + { + // Associate this with the identifier, too. + std::vector used_attr; + used_attr.emplace_back(make_intrusive(ATTR_IS_USED)); + id->AddAttrs(make_intrusive(used_attr, nullptr, false, true)); + break; + } + } + else + priority = 0; + this->body = std::move(body); } diff --git a/src/ID.cc b/src/ID.cc index dd9b39a73d..03ca67d82e 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -1,4 +1,3 @@ - // See the file "COPYING" in the main distribution directory for copyright. #include "zeek/ID.h" @@ -18,6 +17,7 @@ #include "zeek/Val.h" #include "zeek/module_util.h" #include "zeek/script_opt/IDOptInfo.h" +#include "zeek/script_opt/UsageAnalyzer.h" #include "zeek/zeekygen/IdentifierInfo.h" #include "zeek/zeekygen/Manager.h" #include "zeek/zeekygen/ScriptInfo.h" @@ -166,7 +166,10 @@ void ID::SetVal(ValPtr v) { handler = new EventHandler(name); handler->SetFunc(func); - event_registry->Register(handler); + event_registry->Register(handler, true); + + if ( ! IsExport() ) + register_new_event({NewRef{}, this}); } else { diff --git a/src/Options.cc b/src/Options.cc index b9938bbb12..016f14995a 100644 --- a/src/Options.cc +++ b/src/Options.cc @@ -107,6 +107,8 @@ void usage(const char* prog, int code) fprintf(stderr, " -s|--rulefile | read rules from given file\n"); fprintf(stderr, " -t|--tracefile | activate execution tracing\n"); fprintf(stderr, " -u|--usage-issues | find variable usage issues and exit\n"); + fprintf(stderr, " --no-unused-warnings | suppress warnings of unused " + "functions/hooks/events\n"); fprintf(stderr, " -v|--version | print version and exit\n"); fprintf(stderr, " -w|--writefile | write to given tcpdump file\n"); #ifdef DEBUG @@ -367,6 +369,7 @@ Options parse_cmdline(int argc, char** argv) } int profile_scripts = 0; + int no_unused_warnings = 0; struct option long_opts[] = { {"parse-only", no_argument, nullptr, 'a'}, @@ -410,6 +413,7 @@ Options parse_cmdline(int argc, char** argv) #endif {"profile-scripts", optional_argument, &profile_scripts, 1}, + {"no-unused-warnings", no_argument, &no_unused_warnings, 1}, {"pseudo-realtime", optional_argument, nullptr, '~'}, {"jobs", optional_argument, nullptr, 'j'}, {"test", no_argument, nullptr, '#'}, @@ -613,6 +617,9 @@ Options parse_cmdline(int argc, char** argv) activate_script_profiling(optarg); profile_scripts = 0; } + + if ( no_unused_warnings ) + rval.no_unused_warnings = true; break; case '?': diff --git a/src/Options.h b/src/Options.h index 0065b527b9..81cb5c3402 100644 --- a/src/Options.h +++ b/src/Options.h @@ -58,6 +58,7 @@ struct Options bool perftools_profile = false; bool deterministic_mode = false; bool abort_on_scripting_errors = false; + bool no_unused_warnings = false; bool run_unit_tests = false; std::vector doctest_args; diff --git a/src/Stmt.cc b/src/Stmt.cc index 0c1265fbfb..a17d59b2de 100644 --- a/src/Stmt.cc +++ b/src/Stmt.cc @@ -2162,22 +2162,27 @@ TraversalCode WhenStmt::Traverse(TraversalCallback* cb) const TraversalCode tc = cb->PreStmt(this); HANDLE_TC_STMT_PRE(tc); - tc = wi->Cond()->Traverse(cb); - HANDLE_TC_STMT_PRE(tc); + auto wl = wi->Lambda(); - tc = wi->WhenBody()->Traverse(cb); - HANDLE_TC_STMT_PRE(tc); - - if ( wi->TimeoutExpr() ) + if ( wl ) { - tc = wi->TimeoutExpr()->Traverse(cb); + tc = wl->Traverse(cb); HANDLE_TC_STMT_PRE(tc); } - if ( wi->TimeoutStmt() ) + else { - tc = wi->TimeoutStmt()->Traverse(cb); + tc = wi->Cond()->Traverse(cb); HANDLE_TC_STMT_PRE(tc); + + tc = wi->WhenBody()->Traverse(cb); + HANDLE_TC_STMT_PRE(tc); + + if ( wi->TimeoutStmt() ) + { + tc = wi->TimeoutStmt()->Traverse(cb); + HANDLE_TC_STMT_PRE(tc); + } } tc = cb->PostStmt(this); diff --git a/src/Traverse.h b/src/Traverse.h index 6d359c177d..0e49d8b000 100644 --- a/src/Traverse.h +++ b/src/Traverse.h @@ -9,6 +9,7 @@ namespace zeek { class Func; +class Type; namespace detail { @@ -16,6 +17,8 @@ namespace detail class Stmt; class Expr; class ID; +class Attributes; +class Attr; class TraversalCallback { @@ -41,6 +44,20 @@ public: virtual TraversalCode PreDecl(const ID*) { return TC_CONTINUE; } virtual TraversalCode PostDecl(const ID*) { return TC_CONTINUE; } + // A caution regarding using the next two: when traversing types, + // there's a possibility of encountering a (directly or indirectly) + // recursive record. So you'll need some way of avoiding that, + // such as remembering which types have already been traversed + // and skipping via TC_ABORTSTMT when seen again. + virtual TraversalCode PreType(const Type*) { return TC_CONTINUE; } + virtual TraversalCode PostType(const Type*) { return TC_CONTINUE; } + + virtual TraversalCode PreAttrs(const Attributes*) { return TC_CONTINUE; } + virtual TraversalCode PostAttrs(const Attributes*) { return TC_CONTINUE; } + + virtual TraversalCode PreAttr(const Attr*) { return TC_CONTINUE; } + virtual TraversalCode PostAttr(const Attr*) { return TC_CONTINUE; } + ScopePtr current_scope; }; diff --git a/src/TraverseTypes.h b/src/TraverseTypes.h index 98d1e1b66f..f1b0ed69be 100644 --- a/src/TraverseTypes.h +++ b/src/TraverseTypes.h @@ -34,14 +34,16 @@ enum TraversalCode return (code); \ } -#define HANDLE_TC_EXPR_PRE(code) \ - { \ - if ( (code) == zeek::detail::TC_ABORTALL ) \ - return (code); \ - else if ( (code) == zeek::detail::TC_ABORTSTMT ) \ - return zeek::detail::TC_CONTINUE; \ - } - +#define HANDLE_TC_EXPR_PRE(code) HANDLE_TC_STMT_PRE(code) #define HANDLE_TC_EXPR_POST(code) return (code); +#define HANDLE_TC_TYPE_PRE(code) HANDLE_TC_STMT_PRE(code) +#define HANDLE_TC_TYPE_POST(code) return (code); + +#define HANDLE_TC_ATTRS_PRE(code) HANDLE_TC_STMT_PRE(code) +#define HANDLE_TC_ATTRS_POST(code) return (code); + +#define HANDLE_TC_ATTR_PRE(code) HANDLE_TC_STMT_PRE(code) +#define HANDLE_TC_ATTR_POST(code) return (code); + } // namespace zeek::detail diff --git a/src/Type.cc b/src/Type.cc index 70128b94f9..0196644288 100644 --- a/src/Type.cc +++ b/src/Type.cc @@ -274,6 +274,15 @@ unsigned int Type::MemoryAllocation() const return padded_sizeof(*this); } +detail::TraversalCode Type::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + bool TypeList::AllMatch(const Type* t, bool is_init) const { for ( const auto& type : types ) @@ -340,6 +349,21 @@ unsigned int TypeList::MemoryAllocation() const #pragma GCC diagnostic pop } +detail::TraversalCode TypeList::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + for ( const auto& type : types ) + { + tc = type->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + } + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + int IndexType::MatchesIndex(detail::ListExpr* const index) const { // If we have a type indexed by subnets, addresses are ok. @@ -435,6 +459,27 @@ bool IndexType::IsSubNetIndex() const return false; } +detail::TraversalCode IndexType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + for ( const auto& ind : GetIndexTypes() ) + { + tc = ind->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + } + + if ( yield_type ) + { + tc = yield_type->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + } + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + static bool is_supported_index_type(const TypePtr& t, const char** tname) { if ( t->InternalType() != TYPE_INTERNAL_OTHER ) @@ -865,6 +910,36 @@ std::optional FuncType::FindPrototype(const RecordType& arg return {}; } +detail::TraversalCode FuncType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + tc = args->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + + if ( yield ) + { + tc = yield->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + } + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + +detail::TraversalCode TypeType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + tc = type->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + TypeDecl::TypeDecl(const char* i, TypePtr t, detail::AttributesPtr arg_attrs) : type(std::move(t)), attrs(std::move(arg_attrs)), id(i) { @@ -1458,6 +1533,28 @@ string RecordType::GetFieldDeprecationWarning(int field, bool has_check) const return ""; } +detail::TraversalCode RecordType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + if ( types ) + for ( const auto& td : *types ) + { + tc = td->type->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + + if ( td->attrs ) + { + tc = td->attrs->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + } + } + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + FileType::FileType(TypePtr yield_type) : Type(TYPE_FILE), yield(std::move(yield_type)) { } FileType::~FileType() = default; @@ -1476,6 +1573,18 @@ void FileType::DoDescribe(ODesc* d) const } } +detail::TraversalCode FileType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + tc = yield->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + OpaqueType::OpaqueType(const string& arg_name) : Type(TYPE_OPAQUE) { name = arg_name; @@ -1832,6 +1941,18 @@ void VectorType::DescribeReST(ODesc* d, bool roles_only) const d->Add(util::fmt(":zeek:type:`%s`", yield_type->GetName().c_str())); } +detail::TraversalCode VectorType::Traverse(detail::TraversalCallback* cb) const + { + auto tc = cb->PreType(this); + HANDLE_TC_TYPE_PRE(tc); + + tc = yield_type->Traverse(cb); + HANDLE_TC_TYPE_PRE(tc); + + tc = cb->PostType(this); + HANDLE_TC_TYPE_POST(tc); + } + // Returns true if t1 is initialization-compatible with t2 (i.e., if an // initializer with type t1 can be used to initialize a value with type t2), // false otherwise. Assumes that t1's tag is different from t2's. Note diff --git a/src/Type.h b/src/Type.h index e184a66c8a..7e9b3a1d29 100644 --- a/src/Type.h +++ b/src/Type.h @@ -13,6 +13,7 @@ #include "zeek/ID.h" #include "zeek/IntrusivePtr.h" #include "zeek/Obj.h" +#include "zeek/Traverse.h" #include "zeek/ZeekList.h" namespace zeek @@ -258,6 +259,8 @@ public: void SetName(const std::string& arg_name) { name = arg_name; } const std::string& GetName() const { return name; } + virtual detail::TraversalCode Traverse(detail::TraversalCallback* cb) const; + struct TypePtrComparer { bool operator()(const TypePtr& a, const TypePtr& b) const { return a.get() < b.get(); } @@ -353,6 +356,8 @@ public: "GHI-572.")]] unsigned int MemoryAllocation() const override; + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: void DoDescribe(ODesc* d) const override; @@ -376,6 +381,8 @@ public: // Returns true if this table is solely indexed by subnet. bool IsSubNetIndex() const; + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: IndexType(TypeTag t, TypeListPtr arg_indices, TypePtr arg_yield_type) : Type(t), indices(std::move(arg_indices)), yield_type(std::move(arg_yield_type)) @@ -533,6 +540,8 @@ public: */ void SetExpressionlessReturnOkay(bool is_ok) { expressionless_return_okay = is_ok; } + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: friend FuncTypePtr make_intrusive(); @@ -564,6 +573,8 @@ public: template IntrusivePtr GetType() const { return cast_intrusive(type); } + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: TypePtr type; }; @@ -698,6 +709,8 @@ public: std::string GetFieldDeprecationWarning(int field, bool has_check) const; + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: RecordType() { types = nullptr; } @@ -731,6 +744,8 @@ public: const TypePtr& Yield() const override { return yield; } + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: void DoDescribe(ODesc* d) const override; @@ -844,6 +859,8 @@ public: void DescribeReST(ODesc* d, bool roles_only = false) const override; + detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override; + protected: void DoDescribe(ODesc* d) const override; diff --git a/src/Var.cc b/src/Var.cc index 35b37ac56b..3527054fd1 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -17,8 +17,8 @@ #include "zeek/Traverse.h" #include "zeek/Val.h" #include "zeek/module_util.h" -#include "zeek/script_opt/ScriptOpt.h" #include "zeek/script_opt/StmtOptInfo.h" +#include "zeek/script_opt/UsageAnalyzer.h" namespace zeek::detail { @@ -658,6 +658,9 @@ void begin_func(IDPtr id, const char* module_name, FunctionFlavor flavor, bool i id->Error("event cannot yield a value", t.get()); t->ClearYieldType(flavor); + + if ( ! event_registry->Lookup(id->Name()) ) + register_new_event(id); } std::optional prototype; diff --git a/src/plugin/Manager.h b/src/plugin/Manager.h index 61a103dd98..b2a0158d4c 100644 --- a/src/plugin/Manager.h +++ b/src/plugin/Manager.h @@ -107,7 +107,7 @@ public: void ActivateDynamicPlugins(bool all); /** - * First-stage initializion of the manager. This is called early on + * First-stage initialization of the manager. This is called early on * during Zeek's initialization, before any scripts are processed, and * forwards to the corresponding Plugin methods. */ diff --git a/src/script_opt/ScriptOpt.cc b/src/script_opt/ScriptOpt.cc index 18a5b9d33a..d020dfa631 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -14,6 +14,7 @@ #include "zeek/script_opt/Inline.h" #include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/Reduce.h" +#include "zeek/script_opt/UsageAnalyzer.h" #include "zeek/script_opt/UseDefs.h" #include "zeek/script_opt/ZAM/Compile.h" @@ -519,7 +520,7 @@ static void analyze_scripts_for_ZAM(std::unique_ptr& pfs) finalize_functions(funcs); } -void analyze_scripts() +void analyze_scripts(bool no_unused_warnings) { static bool did_init = false; @@ -529,6 +530,10 @@ void analyze_scripts() did_init = true; } + std::unique_ptr ua; + if ( ! no_unused_warnings ) + ua = std::make_unique(funcs); + auto& ofuncs = analysis_options.only_funcs; auto& ofiles = analysis_options.only_files; diff --git a/src/script_opt/ScriptOpt.h b/src/script_opt/ScriptOpt.h index 95d7889d7e..68986cb7a2 100644 --- a/src/script_opt/ScriptOpt.h +++ b/src/script_opt/ScriptOpt.h @@ -182,8 +182,9 @@ extern void add_file_analysis_pattern(AnalyOpt& opts, const char* pat); // it should be skipped. extern bool should_analyze(const ScriptFuncPtr& f, const StmtPtr& body); -// Analyze all of the parsed scripts collectively for optimization. -extern void analyze_scripts(); +// Analyze all of the parsed scripts collectively for usage issues (unless +// suppressed by the flag) and optimization. +extern void analyze_scripts(bool no_unused_warnings); // Called when Zeek is terminating. extern void finish_script_execution(); diff --git a/src/script_opt/UsageAnalyzer.cc b/src/script_opt/UsageAnalyzer.cc new file mode 100644 index 0000000000..9085c2e5c9 --- /dev/null +++ b/src/script_opt/UsageAnalyzer.cc @@ -0,0 +1,232 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#include "zeek/script_opt/UsageAnalyzer.h" + +#include "zeek/EventRegistry.h" +#include "zeek/module_util.h" +#include "zeek/script_opt/IDOptInfo.h" + +namespace zeek::detail + { + +// The names of identifiers that correspond to events not-previously-known +// before their declaration in the scripts. +std::unordered_set script_events; + +void register_new_event(const IDPtr& id) + { + script_events.insert(id->Name()); + } + +UsageAnalyzer::UsageAnalyzer(std::vector& funcs) + { + // First, prune the script events to only those that were never + // registered in a non-script context. + auto script_events_orig = script_events; + script_events.clear(); + + for ( auto& ev : script_events_orig ) + if ( ! event_registry->NotOnlyRegisteredFromScript(ev) ) + script_events.insert(ev); + + // Setting a scope cues ID::Traverse to delve into function values. + current_scope = global_scope(); + + FindSeeds(reachables); + FullyExpandReachables(); + + // At this point, we've done the complete reachability analysis. + // Report out on unreachables. We do this in two steps: first, + // unreachable events/hooks, and then unreachable functions. We + // split the two because we don't want to ding a function as being + // unreachable if there's an (unreachable) event-or-hook that calls + // it, since presumably the real problem is the latter being an + // orphan, rather than the function. + + auto& globals = global_scope()->Vars(); + + for ( auto& gpair : globals ) + { + auto id = gpair.second.get(); + auto& t = id->GetType(); + + if ( t->Tag() != TYPE_FUNC ) + continue; + + if ( t->AsFuncType()->Flavor() == FUNC_FLAVOR_FUNCTION ) + continue; + + if ( reachables.count(id) > 0 ) + continue; + + auto flavor = t->AsFuncType()->FlavorString(); + auto loc = id->GetLocationInfo(); + + id->Warn(util::fmt("handler for non-existing %s cannot be invoked", flavor.c_str())); + + // Don't ding any functions that are reachable via this + // identifier. This will also suppress flagging other events + // and hooks, depending on order-of-traversal. That seems + // fine, as the key is to find the root of such issues. + reachables.insert(id); + Expand(id); + } + + // Now make a second pass, focusing solely on functions. + for ( auto& gpair : globals ) + { + auto& id = gpair.second; + + if ( reachables.count(id.get()) > 0 ) + continue; + + auto f = GetFuncIfAny(id); + if ( ! f ) + continue; + + auto loc = id->GetLocationInfo(); + + id->Warn("non-exported function does not have any callers"); + + // Unlike for events/hooks above, we don't add the function to + // the reachables. This is because an orphan function is a + // somewhat more significant potential error than an orphan + // event handler or hook, as the latter can arise from simple + // typos (because there will be a declaration elsewhere that + // they're supposed to match), whereas orphan functions in + // general will not. + } + } + +void UsageAnalyzer::FindSeeds(IDSet& seeds) const + { + for ( auto& gpair : global_scope()->Vars() ) + { + auto& id = gpair.second; + + if ( id->GetAttr(ATTR_IS_USED) || id->GetAttr(ATTR_DEPRECATED) ) + { + seeds.insert(id.get()); + continue; + } + + auto f = GetFuncIfAny(id); + + if ( f && id->GetType()->Flavor() == FUNC_FLAVOR_EVENT ) + { + if ( script_events.count(f->Name()) == 0 ) + seeds.insert(id.get()); + + continue; + } + + // If the global is exported, or has global scope, we assume + // it's meant to be used, even if the current scripts don't + // use it. + if ( id->IsExport() || id->ModuleName() == "GLOBAL" ) + seeds.insert(id.get()); + } + } + +const Func* UsageAnalyzer::GetFuncIfAny(const ID* id) const + { + auto& t = id->GetType(); + if ( t->Tag() != TYPE_FUNC ) + return nullptr; + + auto fv = cast_intrusive(id->GetVal()); + if ( ! fv ) + return nullptr; + + auto func = fv->Get(); + return func->GetKind() == Func::SCRIPT_FUNC ? func : nullptr; + } + +void UsageAnalyzer::FullyExpandReachables() + { + // We use the following structure to avoid having to copy + // the initial set of reachables, which can be quite large. + if ( ExpandReachables(reachables) ) + { + auto r = new_reachables; + reachables.insert(r.begin(), r.end()); + + while ( ExpandReachables(r) ) + { + r = new_reachables; + reachables.insert(r.begin(), r.end()); + } + } + } + +bool UsageAnalyzer::ExpandReachables(const IDSet& curr_r) + { + new_reachables.clear(); + + for ( auto r : curr_r ) + Expand(r); + + return ! new_reachables.empty(); + } + +void UsageAnalyzer::Expand(const ID* id) + { + // A subtle problem arises for exported globals that refer to functions + // that themselves generate events. Because for identifiers we don't + // traverse their values (since there's no Traverse infrastructure for + // Val classes), we can see those identifiers initially in a seeding + // context, where we can't associate them with their functions; and + // then again when actually analyzing that function. + // + // It might be tempting to special-case the seeding phase, but that + // gets hard if the global doesn't direclty refer to the function, + // but instead ultimately incorporates a type with an attribute that + // uses the function. So instead we allow re-visiting of identifiers + // and just suppress them once-per-analysis traversal (to save a bunch + // of computation). + analyzed_IDs.clear(); + + id->Traverse(this); + } + +TraversalCode UsageAnalyzer::PreID(const ID* id) + { + if ( analyzed_IDs.count(id) > 0 ) + // No need to repeat the analysis. + return TC_ABORTSTMT; + + // Mark so that we avoid redundant re-traversal. + analyzed_IDs.insert(id); + + auto f = GetFuncIfAny(id); + + if ( f && reachables.count(id) == 0 ) + // Haven't seen this function before. + new_reachables.insert(id); + + id->GetType()->Traverse(this); + + auto& attrs = id->GetAttrs(); + if ( attrs ) + attrs->Traverse(this); + + // Initialization expressions can have function calls or lambdas that + // themselves link to other identifiers. + for ( auto& ie : id->GetOptInfo()->GetInitExprs() ) + if ( ie ) + ie->Traverse(this); + + return TC_CONTINUE; + } + +TraversalCode UsageAnalyzer::PreType(const Type* t) + { + if ( analyzed_types.count(t) > 0 ) + return TC_ABORTSTMT; + + // Save processing by avoiding a re-traversal of this type. + analyzed_types.insert(t); + return TC_CONTINUE; + } + + } // namespace zeek::detail diff --git a/src/script_opt/UsageAnalyzer.h b/src/script_opt/UsageAnalyzer.h new file mode 100644 index 0000000000..9725fe8dc6 --- /dev/null +++ b/src/script_opt/UsageAnalyzer.h @@ -0,0 +1,82 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +// Classes for analyzing the usage of functions, hooks & events in order +// to locate any that cannot actually be invoked. + +#pragma once + +#include "zeek/Traverse.h" +#include "zeek/script_opt/ScriptOpt.h" + +namespace zeek::detail + { + +class UsageAnalyzer : public TraversalCallback + { +public: + // "funcs" contains the entire set of ASTs. + UsageAnalyzer(std::vector& funcs); + +private: + using IDSet = std::unordered_set; + + // Finds the set of identifiers that serve as a starting point of + // what's-known-to-be-used. An identifier qualifies as such if it is + // (1) an event that was newly introduced by scripting (so, known to + // the event engine), or (2) a function or hook that's either global + // in scope, or exported from its module (so clearly meant for use + // by other scripts), or (3) marked as either &is_used or &deprecated + // (the latter as a way to flag identifiers that in fact are not used + // and will be removed in the future). + void FindSeeds(IDSet& seeds) const; + + // Given an identifier, return its corresponding script function, + // or nil if that's not applicable. + const Func* GetFuncIfAny(const ID* id) const; + const Func* GetFuncIfAny(const IDPtr& id) const { return GetFuncIfAny(id.get()); } + + // Iteratively follows reachability across the set of reachable + // identifiers (starting with the seeds) until there's no more to reap. + void FullyExpandReachables(); + + // Populates new_reachables with identifiers newly reachable (directly) + // from curr_r. + bool ExpandReachables(const IDSet& curr_r); + + // For a given identifier, populates new_reachables with new + // identifiers directly reachable from it. + void Expand(const ID* f); + + // Hooks into AST traversal to find reachable functions/hooks/events. + TraversalCode PreID(const ID* id) override; + + // We traverse types, too, as their attributes can include lambdas + // that we need to incorporate. + TraversalCode PreType(const Type* t) override; + + // The identifiers we've currently determined are (ultimately) + // reachable from the seeds. + IDSet reachables; + + // Newly-reachable identifiers-of-interest. This is a member variable + // rather than a parameter to ExpandReachables() because the coupling + // to populating it is indirect, via AST traversal. + IDSet new_reachables; + + // The following are used to avoid redundant computation. Note that + // they differ in that the first is per-traversal, while the second + // is global across all our analyses. See Expand() for a discussion + // of why the first needs to be per-traversal. + + // All of the identifiers we've analyzed during the current traversal. + std::unordered_set analyzed_IDs; + + // All of the types we've analyzed to date. + std::unordered_set analyzed_types; + }; + +// Marks a given identifier as referring to a script-level event (one +// not previously known before its declaration in a script). +extern void register_new_event(const IDPtr& id); + + } // namespace zeek::detail diff --git a/src/script_opt/UseDefs.cc b/src/script_opt/UseDefs.cc index 21ebb59548..e80f38f6a3 100644 --- a/src/script_opt/UseDefs.cc +++ b/src/script_opt/UseDefs.cc @@ -124,11 +124,11 @@ bool UseDefs::RemoveUnused(int iter) // Because we're dealing with reduced statements, the // assignment expression should be to a simple variable. if ( r->Tag() != EXPR_REF ) - reporter->InternalError("lhs ref inconsistency in UseDefs::FindUnused"); + reporter->InternalError("lhs ref inconsistency in UseDefs::RemoveUnused"); auto n = r->AsRefExprPtr()->GetOp1(); if ( n->Tag() != EXPR_NAME ) - reporter->InternalError("lhs name inconsistency in UseDefs::FindUnused"); + reporter->InternalError("lhs name inconsistency in UseDefs::RemoveUnused"); auto id = n->AsNameExpr()->Id(); diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index df8294816d..babf952ad3 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -589,6 +589,9 @@ SetupResult setup(int argc, char** argv, Options* zopts) plugin_mgr = new plugin::Manager(); fragment_mgr = new detail::FragmentManager(); + if ( options.no_unused_warnings && options.analysis_options.usage_issues > 0 ) + reporter->FatalError("-u incompatible with --no-unused-warnings"); + #ifdef DEBUG if ( options.debug_log_streams ) { @@ -926,7 +929,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( options.parse_only ) { if ( analysis_options.usage_issues > 0 ) - analyze_scripts(); + analyze_scripts(options.no_unused_warnings); early_shutdown(); exit(reporter->Errors() != 0); @@ -934,7 +937,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) auto init_stmts = stmts ? analyze_global_stmts(stmts) : nullptr; - analyze_scripts(); + analyze_scripts(options.no_unused_warnings); if ( analysis_options.report_recursive ) { diff --git a/testing/btest/Baseline/core.check-unused-event-handlers/.stderr b/testing/btest/Baseline/core.check-unused-event-handlers/.stderr index 808ae449a1..3f15b4d3ad 100644 --- a/testing/btest/Baseline/core.check-unused-event-handlers/.stderr +++ b/testing/btest/Baseline/core.check-unused-event-handlers/.stderr @@ -1,5 +1,6 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. ### NOTE: This file has been sorted with diff-sort. +warning in <...>/check-unused-event-handlers.test, line 6: handler for non-existing event cannot be invoked (this_is_never_used) warning in , line 1: event handler never invoked: InputConfig::new_value warning in , line 1: event handler never invoked: InputRaw::process_finished warning in , line 1: event handler never invoked: SupervisorControl::create_request diff --git a/testing/btest/Baseline/core.option-runtime-errors-8/.stderr b/testing/btest/Baseline/core.option-runtime-errors-8/.stderr index 66435b589a..47e834683f 100644 --- a/testing/btest/Baseline/core.option-runtime-errors-8/.stderr +++ b/testing/btest/Baseline/core.option-runtime-errors-8/.stderr @@ -1,2 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +warning in <...>/option-runtime-errors.zeek, line 4: handler for non-existing event cannot be invoked (option_changed) error in <...>/option-runtime-errors.zeek, line 7: Option::on_change needs function argument; not hook or event (Option::set_change_handler(A, to_any_coerceoption_changed, (coerce 0 to int))) diff --git a/testing/btest/Baseline/language.table-type-checking/out b/testing/btest/Baseline/language.table-type-checking/out index 0421c6d9f1..54a8b0534d 100644 --- a/testing/btest/Baseline/language.table-type-checking/out +++ b/testing/btest/Baseline/language.table-type-checking/out @@ -14,4 +14,4 @@ error in port and <...>/table-type-checking.zeek, line 42: type clash (port and error in <...>/table-type-checking.zeek, line 42: inconsistent types in table constructor (table(thousand-two = 1002)) error in <...>/table-type-checking.zeek, line 48: type clash in assignment (lea = table(thousand-three = 1003)) error in count and <...>/table-type-checking.zeek, line 54: arithmetic mixed with non-arithmetic (count and foo) -error in <...>/table-type-checking.zeek, line 4 and <...>/table-type-checking.zeek, line 54: &default value has inconsistent type (MyTable and table()) +error in <...>/table-type-checking.zeek, line 4 and <...>/table-type-checking.zeek, line 54: &default value has inconsistent type (MyTable and table()&default=foo, &optional) diff --git a/testing/btest/core/check-unused-event-handlers.test b/testing/btest/core/check-unused-event-handlers.test index 742a07554c..2d2fe68e14 100644 --- a/testing/btest/core/check-unused-event-handlers.test +++ b/testing/btest/core/check-unused-event-handlers.test @@ -1,7 +1,7 @@ # This test should print a warning that the event handler is never invoked. # @TEST-REQUIRES: $SCRIPTS/have-spicy # This test logs uninvoked event handlers, so disable it if Spicy and its plugin is unavailable. # @TEST-EXEC: zeek -b %INPUT check_for_unused_event_handlers=T -# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-sort btest-diff .stderr +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-sort-and-remove-abspath btest-diff .stderr event this_is_never_used() { diff --git a/testing/btest/language/closure-sending-naming.zeek b/testing/btest/language/closure-sending-naming.zeek index f77c8e50b5..c58d651c49 100644 --- a/testing/btest/language/closure-sending-naming.zeek +++ b/testing/btest/language/closure-sending-naming.zeek @@ -76,7 +76,7 @@ type myfunctype: function(c: count) : function(d: count) : count; global global_with_same_name = 100; -global pong: event(msg: string, f: myfunctype); +global pong: event(msg: string, f: myfunctype) &is_used; # This is one, of many, ways to declare your functions that you plan to receive. # All you are doing is giving the parser a version of their body, so they can be @@ -116,7 +116,7 @@ event Broker::peer_lost(endpoint: Broker::EndpointInfo, msg: string) global n = 0; -event ping(msg: string, f: myfunctype) +event ping(msg: string, f: myfunctype) &is_used { print fmt("receiver got ping: %s", msg); ++n; diff --git a/testing/scripts/diff-sort-and-remove-abspath b/testing/scripts/diff-sort-and-remove-abspath new file mode 100755 index 0000000000..f071ce1c70 --- /dev/null +++ b/testing/scripts/diff-sort-and-remove-abspath @@ -0,0 +1,3 @@ +#! /usr/bin/env bash +# +diff-sort | diff-remove-abspath