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 4cc3f9d572..78e0449780 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -4877,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(); diff --git a/src/ID.cc b/src/ID.cc index 18494c7fd1..03ca67d82e 100644 --- a/src/ID.cc +++ b/src/ID.cc @@ -166,7 +166,7 @@ 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}); diff --git a/src/Options.cc b/src/Options.cc index ea9754a775..016f14995a 100644 --- a/src/Options.cc +++ b/src/Options.cc @@ -107,7 +107,7 @@ 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-usage-warnings | suppress warnings of unused " + 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"); @@ -369,7 +369,7 @@ Options parse_cmdline(int argc, char** argv) } int profile_scripts = 0; - int no_usage_warnings = 0; + int no_unused_warnings = 0; struct option long_opts[] = { {"parse-only", no_argument, nullptr, 'a'}, @@ -413,7 +413,7 @@ Options parse_cmdline(int argc, char** argv) #endif {"profile-scripts", optional_argument, &profile_scripts, 1}, - {"no-usage-warnings", no_argument, &no_usage_warnings, 1}, + {"no-unused-warnings", no_argument, &no_unused_warnings, 1}, {"pseudo-realtime", optional_argument, nullptr, '~'}, {"jobs", optional_argument, nullptr, 'j'}, {"test", no_argument, nullptr, '#'}, @@ -618,8 +618,8 @@ Options parse_cmdline(int argc, char** argv) profile_scripts = 0; } - if ( no_usage_warnings ) - rval.no_usage_warnings = true; + if ( no_unused_warnings ) + rval.no_unused_warnings = true; break; case '?': diff --git a/src/Options.h b/src/Options.h index feea673c47..81cb5c3402 100644 --- a/src/Options.h +++ b/src/Options.h @@ -58,7 +58,7 @@ struct Options bool perftools_profile = false; bool deterministic_mode = false; bool abort_on_scripting_errors = false; - bool no_usage_warnings = false; + bool no_unused_warnings = false; bool run_unit_tests = false; std::vector doctest_args; 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 ef7ae065ba..d020dfa631 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -520,7 +520,7 @@ static void analyze_scripts_for_ZAM(std::unique_ptr& pfs) finalize_functions(funcs); } -void analyze_scripts(bool no_usage_warnings) +void analyze_scripts(bool no_unused_warnings) { static bool did_init = false; @@ -531,7 +531,7 @@ void analyze_scripts(bool no_usage_warnings) } std::unique_ptr ua; - if ( ! no_usage_warnings ) + if ( ! no_unused_warnings ) ua = std::make_unique(funcs); auto& ofuncs = analysis_options.only_funcs; diff --git a/src/script_opt/ScriptOpt.h b/src/script_opt/ScriptOpt.h index 423dfd7218..68986cb7a2 100644 --- a/src/script_opt/ScriptOpt.h +++ b/src/script_opt/ScriptOpt.h @@ -184,7 +184,7 @@ extern bool should_analyze(const ScriptFuncPtr& f, const StmtPtr& body); // Analyze all of the parsed scripts collectively for usage issues (unless // suppressed by the flag) and optimization. -extern void analyze_scripts(bool no_usage_warnings); +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 index 102d021c5f..d91df3448b 100644 --- a/src/script_opt/UsageAnalyzer.cc +++ b/src/script_opt/UsageAnalyzer.cc @@ -2,6 +2,7 @@ #include "zeek/script_opt/UsageAnalyzer.h" +#include "zeek/EventRegistry.h" #include "zeek/module_util.h" #include "zeek/script_opt/IDOptInfo.h" @@ -19,6 +20,15 @@ void register_new_event(const IDPtr& id) 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(); @@ -52,8 +62,7 @@ UsageAnalyzer::UsageAnalyzer(std::vector& funcs) auto flavor = t->AsFuncType()->FlavorString(); auto loc = id->GetLocationInfo(); - reporter->Warning("%s %s (%s:%d): cannot be invoked", flavor.c_str(), id->Name(), - loc->filename, loc->first_line); + 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 @@ -77,8 +86,7 @@ UsageAnalyzer::UsageAnalyzer(std::vector& funcs) auto loc = id->GetLocationInfo(); - reporter->Warning("function %s (%s:%d): cannot be called", id->Name(), loc->filename, - loc->first_line); + id->Warn("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 diff --git a/src/zeek-setup.cc b/src/zeek-setup.cc index 6a3ba211f0..babf952ad3 100644 --- a/src/zeek-setup.cc +++ b/src/zeek-setup.cc @@ -589,8 +589,8 @@ SetupResult setup(int argc, char** argv, Options* zopts) plugin_mgr = new plugin::Manager(); fragment_mgr = new detail::FragmentManager(); - if ( options.no_usage_warnings && options.analysis_options.usage_issues > 0 ) - reporter->FatalError("-u incompatible with --no-usage-warnings"); + 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 ) @@ -929,7 +929,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) if ( options.parse_only ) { if ( analysis_options.usage_issues > 0 ) - analyze_scripts(options.no_usage_warnings); + analyze_scripts(options.no_unused_warnings); early_shutdown(); exit(reporter->Errors() != 0); @@ -937,7 +937,7 @@ SetupResult setup(int argc, char** argv, Options* zopts) auto init_stmts = stmts ? analyze_global_stmts(stmts) : nullptr; - analyze_scripts(options.no_usage_warnings); + 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 0bf43736d5..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 @@ -9,4 +10,3 @@ warning in , line 1: event handler never invoked: SupervisorControl::sta warning in , line 1: event handler never invoked: SupervisorControl::stop_request warning in , line 1: event handler never invoked: spicy_analyzer_for_mime_type warning in , line 1: event handler never invoked: this_is_never_used -warning in , line 1: event this_is_never_used (<...>/check-unused-event-handlers.test:6): cannot be invoked diff --git a/testing/btest/Baseline/core.option-runtime-errors-8/.stderr b/testing/btest/Baseline/core.option-runtime-errors-8/.stderr index 8db3e3a4ca..47e834683f 100644 --- a/testing/btest/Baseline/core.option-runtime-errors-8/.stderr +++ b/testing/btest/Baseline/core.option-runtime-errors-8/.stderr @@ -1,3 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -warning: event option_changed (<...>/option-runtime-errors.zeek:4): cannot be invoked +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/spicy.spicyz-aot/output b/testing/btest/Baseline/spicy.spicyz-aot/output index 0d214344cc..e7ad0949ae 100644 --- a/testing/btest/Baseline/spicy.spicyz-aot/output +++ b/testing/btest/Baseline/spicy.spicyz-aot/output @@ -1,5 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -warning in , line 1: event test::dummy (./test.zeek:3): cannot be invoked 8, [$data=[b"POST /post HTTP/1.1", b"User-Agent: curl/7.29.0", b"Host: httpbin.org", b"Accept: */*", b"Content-Length: 11", b"Content-Type: application/x-www-form-urlencoded", b"", b"hello world"]] Event:, [POST /post HTTP/1.1, User-Agent: curl/7.29.0, Host: httpbin.org, Accept: */*, Content-Length: 11, Content-Type: application/x-www-form-urlencoded, , hello world] 8, [$data=[b"HTTP/1.1 200 OK", b"Server: gunicorn/0.16.1", b"Date: Tue, 19 Mar 2013 16:05:11 GMT", b"Content-Type: application/json", b"Content-Length: 366", b"Connection: close", b"", b"{\x0a \"origin\": \"10.142.133.148\",\x0a \"files\": {},\x0a \"form\": null,\x0a \"url\": \"http://httpbin.org/post\",\x0a \"args\": {},\x0a \"headers\": {\x0a \"Content-Length\": \"11\",\x0a \"Connection\": \"close\",\x0a \"Accept\": \"*/*\",\x0a \"User-Agent\": \"curl/7.29.0\",\x0a \"Host\": \"httpbin.org\",\x0a \"Content-Type\": \"application/x-www-form-urlencoded\"\x0a },\x0a \"json\": null,\x0a \"data\": \"hello world\"\x0a}"]] diff --git a/testing/btest/Baseline/spicy.spicyz-jit/output b/testing/btest/Baseline/spicy.spicyz-jit/output index 0d214344cc..e7ad0949ae 100644 --- a/testing/btest/Baseline/spicy.spicyz-jit/output +++ b/testing/btest/Baseline/spicy.spicyz-jit/output @@ -1,5 +1,4 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -warning in , line 1: event test::dummy (./test.zeek:3): cannot be invoked 8, [$data=[b"POST /post HTTP/1.1", b"User-Agent: curl/7.29.0", b"Host: httpbin.org", b"Accept: */*", b"Content-Length: 11", b"Content-Type: application/x-www-form-urlencoded", b"", b"hello world"]] Event:, [POST /post HTTP/1.1, User-Agent: curl/7.29.0, Host: httpbin.org, Accept: */*, Content-Length: 11, Content-Type: application/x-www-form-urlencoded, , hello world] 8, [$data=[b"HTTP/1.1 200 OK", b"Server: gunicorn/0.16.1", b"Date: Tue, 19 Mar 2013 16:05:11 GMT", b"Content-Type: application/json", b"Content-Length: 366", b"Connection: close", b"", b"{\x0a \"origin\": \"10.142.133.148\",\x0a \"files\": {},\x0a \"form\": null,\x0a \"url\": \"http://httpbin.org/post\",\x0a \"args\": {},\x0a \"headers\": {\x0a \"Content-Length\": \"11\",\x0a \"Connection\": \"close\",\x0a \"Accept\": \"*/*\",\x0a \"User-Agent\": \"curl/7.29.0\",\x0a \"Host\": \"httpbin.org\",\x0a \"Content-Type\": \"application/x-www-form-urlencoded\"\x0a },\x0a \"json\": null,\x0a \"data\": \"hello world\"\x0a}"]]