diff --git a/CHANGES b/CHANGES index a9ba1e0288..9865fc8c06 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,25 @@ +6.2.0-dev.256 | 2023-12-08 10:01:10 -0700 + + * incorporate latest version of gen-zam to correctly generate indirect calls (Vern Paxson, Corelight) + + * added sub-directory for tracking ZAM maintenance issues (Vern Paxson, Corelight) + + * BTest to stress-test AST optimizer's assessment of side effects (Vern Paxson, Corelight) + + * reworked AST optimizers analysis of side effects during aggregate operations & calls (Vern Paxson, Corelight) + + * script optimization support for tracking information associated with BiFs/functions (Vern Paxson, Corelight) + + * fix for AST analysis of inlined functions (Vern Paxson, Corelight) + + * improved AST optimizer's analysis of variable usage in inlined functions (Vern Paxson, Corelight) + + * new method for Stmt nodes to report whether they could execute a "return" (Vern Paxson, Corelight) + + * bug fixes for indirect function calls when using ZAM (Vern Paxson, Corelight) + + * minor fixes for script optimization, exporting of attr_name, script layout tweak (Vern Paxson, Corelight) + 6.2.0-dev.245 | 2023-12-06 18:42:25 +0100 * Bump cmake submodule (Arne Welzel, Corelight) diff --git a/VERSION b/VERSION index 1acd1fad8e..1be7c1d0b7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.2.0-dev.245 +6.2.0-dev.256 diff --git a/auxil/gen-zam b/auxil/gen-zam index cbba05dbaa..bbb5b3645a 160000 --- a/auxil/gen-zam +++ b/auxil/gen-zam @@ -1 +1 @@ -Subproject commit cbba05dbaa58fdabe863f4e8a122ca92809b52d6 +Subproject commit bbb5b3645aa7b65db9183b5fc0b3be8a952ea308 diff --git a/scripts/policy/protocols/ssl/ssl-log-ext.zeek b/scripts/policy/protocols/ssl/ssl-log-ext.zeek index 844111e7ca..6fed64169a 100644 --- a/scripts/policy/protocols/ssl/ssl-log-ext.zeek +++ b/scripts/policy/protocols/ssl/ssl-log-ext.zeek @@ -137,7 +137,7 @@ event ssl_extension_supported_versions(c: connection, is_client: bool, versions: c$ssl$server_supported_version = versions[0]; } - event ssl_extension_psk_key_exchange_modes(c: connection, is_client: bool, modes: index_vec) +event ssl_extension_psk_key_exchange_modes(c: connection, is_client: bool, modes: index_vec) { if ( ! c?$ssl || ! is_client ) return; diff --git a/src/Attr.h b/src/Attr.h index 546afb7d88..94c04a380c 100644 --- a/src/Attr.h +++ b/src/Attr.h @@ -150,5 +150,8 @@ protected: // it will be returned in err_msg. extern bool check_default_attr(Attr* a, const TypePtr& type, bool global_var, bool in_record, std::string& err_msg); +// Returns the script-level name associated with an attribute, e.g. "&default". +extern const char* attr_name(AttrTag t); + } // namespace detail } // namespace zeek diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 8552ad3105..37f6109cd2 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -395,6 +395,7 @@ set(MAIN_SRCS script_opt/CPP/Vars.cc ${_gen_zeek_script_cpp} script_opt/Expr.cc + script_opt/FuncInfo.cc script_opt/GenIDDefs.cc script_opt/IDOptInfo.cc script_opt/Inline.cc diff --git a/src/Stmt.h b/src/Stmt.h index d79a8631fb..2a439ccaa8 100644 --- a/src/Stmt.h +++ b/src/Stmt.h @@ -122,6 +122,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; protected: ValPtr DoExec(Frame* f, Val* v, StmtFlowType& flow) override; @@ -182,6 +183,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; protected: friend class ZAMCompiler; @@ -301,6 +303,8 @@ public: // Note, no need for a NoFlowAfter method because the loop might // execute zero times, so it's always the default of "false". + // However, we do need to check for potential returns. + bool CouldReturn(bool ignore_break) const override; protected: ValPtr Exec(Frame* f, StmtFlowType& flow) override; @@ -350,6 +354,8 @@ public: // Note, no need for a NoFlowAfter method because the loop might // execute zero times, so it's always the default of "false". + // However, we do need to check for potential returns. + bool CouldReturn(bool ignore_break) const override; protected: ValPtr DoExec(Frame* f, Val* v, StmtFlowType& flow) override; @@ -395,6 +401,7 @@ public: StmtPtr Duplicate() override { return SetSucc(new BreakStmt()); } bool NoFlowAfter(bool ignore_break) const override { return ! ignore_break; } + bool CouldReturn(bool ignore_break) const override { return ! ignore_break; } protected: }; @@ -436,6 +443,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override { return true; } + bool CouldReturn(bool ignore_break) const override { return true; } }; class StmtList : public Stmt { @@ -460,6 +468,7 @@ public: StmtPtr DoReduce(Reducer* c) override; bool NoFlowAfter(bool ignore_break) const override; + bool CouldReturn(bool ignore_break) const override; // Idioms commonly used in reduction. StmtList(StmtPtr s1, StmtPtr s2); @@ -725,7 +734,7 @@ public: // Note, no need for a NoFlowAfter() method because anything that // has "NoFlowAfter" inside the body still gets caught and we - // continue afterwards. + // continue afterwards. Same goes for CouldReturn(). StmtPtr Duplicate() override; diff --git a/src/StmtBase.h b/src/StmtBase.h index 46ff718d07..db47926c6f 100644 --- a/src/StmtBase.h +++ b/src/StmtBase.h @@ -138,6 +138,12 @@ public: // in that case, they do lead to flow reaching the end. virtual bool NoFlowAfter(bool ignore_break) const { return false; } + // True if the statement could potentially execute a return. This is + // used by script optimization to track confluence inside catch-return + // blocks. Here, ignore_break is false if we're analyzing a coalesced + // hook body. + virtual bool CouldReturn(bool ignore_break) const { return false; } + // Access to the original statement from which this one is derived, // or this one if we don't have an original. Returns a bare pointer // rather than a StmtPtr to emphasize that the access is read-only. diff --git a/src/parse.y b/src/parse.y index c469ec93e4..69fe94eda8 100644 --- a/src/parse.y +++ b/src/parse.y @@ -132,6 +132,10 @@ std::string zeek::detail::current_module = GLOBAL_MODULE_NAME; bool is_export = false; // true if in an export {} block +// Used to temporarily turn off "is_export". A stack because the need +// to do so can nest. +std::vector hold_is_export; + // When parsing an expression for the debugger, where to put the result // (obviously not reentrant). extern Expr* g_curr_debug_expr; @@ -586,7 +590,10 @@ expr: if ( IsArithmetic($1->GetType()->Tag()) ) { - ExprPtr sum = make_intrusive(lhs, rhs); + // Script optimization assumes that each AST + // node is distinct, hence the call to + // Duplicate() here. + ExprPtr sum = make_intrusive(lhs->Duplicate(), rhs); if ( sum->GetType()->Tag() != tag1 ) sum = make_intrusive(sum, tag1); @@ -1584,8 +1591,24 @@ lambda_body: ; anonymous_function: - TOK_FUNCTION begin_lambda conditional_list lambda_body - { $$ = $4; } + TOK_FUNCTION + { + // "is_export" is used in some contexts to determine + // whether a given newly seen identifier is a global. + // We're about parse a lambda body, for which all of + // the new identifiers should be locals, not globals, + // so we need to turn off "is_export" here. We use + // a stack because lambdas can have additional lambdas + // inside their bodies. + hold_is_export.push_back(is_export); + is_export = false; + } + begin_lambda conditional_list lambda_body + { + is_export = hold_is_export.back(); + hold_is_export.pop_back(); + $$ = $5; + } ; begin_lambda: diff --git a/src/script_opt/ExprOptInfo.h b/src/script_opt/ExprOptInfo.h index 30452aa344..be304f0bb5 100644 --- a/src/script_opt/ExprOptInfo.h +++ b/src/script_opt/ExprOptInfo.h @@ -7,11 +7,32 @@ namespace zeek::detail { +// Class for tracking whether a given expression has side effects. Currently, +// we just need to know whether Yes-it-does or No-it-doesn't, so the structure +// is very simple. + +class ExprSideEffects { +public: + ExprSideEffects(bool _has_side_effects) : has_side_effects(_has_side_effects) {} + + bool HasSideEffects() const { return has_side_effects; } + +protected: + bool has_side_effects; +}; + class ExprOptInfo { public: // The AST number of the statement in which this expression // appears. int stmt_num = -1; // -1 = not assigned yet + + auto& SideEffects() { return side_effects; } + +protected: + // This optional value missing means "we haven't yet determined the + // side effects". + std::optional side_effects; }; } // namespace zeek::detail diff --git a/src/script_opt/FuncInfo.cc b/src/script_opt/FuncInfo.cc new file mode 100644 index 0000000000..57730670e0 --- /dev/null +++ b/src/script_opt/FuncInfo.cc @@ -0,0 +1,506 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +#include "zeek/script_opt/FuncInfo.h" + +#include + +namespace zeek::detail { + +// The following BiFs do not have any script-level side effects. It's +// followed by comments listing the BiFs that have been omitted, and why. +// +// See script_opt/ZAM/maint/README for maintenance of these lists. + +static std::unordered_set side_effects_free_BiFs = { + "Analyzer::__disable_all_analyzers", + "Analyzer::__disable_analyzer", + "Analyzer::__enable_analyzer", + "Analyzer::__has_tag", + "Analyzer::__name", + "Analyzer::__register_for_port", + "Analyzer::__schedule_analyzer", + "Analyzer::__tag", + "FileExtract::__set_limit", + "Files::__add_analyzer", + "Files::__analyzer_enabled", + "Files::__analyzer_name", + "Files::__disable_analyzer", + "Files::__disable_reassembly", + "Files::__enable_analyzer", + "Files::__enable_reassembly", + "Files::__file_exists", + "Files::__lookup_file", + "Files::__remove_analyzer", + "Files::__set_reassembly_buffer", + "Files::__set_timeout_interval", + "Files::__stop", + "Input::__create_analysis_stream", + "Input::__create_event_stream", + "Input::__create_table_stream", + "Input::__force_update", + "Input::__remove_stream", + "Log::__add_filter", + "Log::__create_stream", + "Log::__disable_stream", + "Log::__enable_stream", + "Log::__flush", + "Log::__remove_filter", + "Log::__remove_stream", + "Log::__set_buf", + "Option::any_set_to_any_vec", + "Option::set_change_handler", + "PacketAnalyzer::GTPV1::remove_gtpv1_connection", + "PacketAnalyzer::TEREDO::remove_teredo_connection", + "PacketAnalyzer::__disable_analyzer", + "PacketAnalyzer::__enable_analyzer", + "PacketAnalyzer::__set_ignore_checksums_nets", + "PacketAnalyzer::register_packet_analyzer", + "PacketAnalyzer::register_protocol_detection", + "PacketAnalyzer::try_register_packet_analyzer_by_name", + "Pcap::error", + "Pcap::findalldevs", + "Pcap::get_filter_state", + "Pcap::get_filter_state_string", + "Pcap::install_pcap_filter", + "Pcap::precompile_pcap_filter", + "Reporter::conn_weird", + "Reporter::error", + "Reporter::fatal", + "Reporter::fatal_error_with_core", + "Reporter::file_weird", + "Reporter::flow_weird", + "Reporter::get_weird_sampling_duration", + "Reporter::get_weird_sampling_global_list", + "Reporter::get_weird_sampling_rate", + "Reporter::get_weird_sampling_threshold", + "Reporter::get_weird_sampling_whitelist", + "Reporter::info", + "Reporter::net_weird", + "Reporter::set_weird_sampling_duration", + "Reporter::set_weird_sampling_global_list", + "Reporter::set_weird_sampling_rate", + "Reporter::set_weird_sampling_threshold", + "Reporter::set_weird_sampling_whitelist", + "Reporter::warning", + "Spicy::__resource_usage", + "Spicy::__toggle_analyzer", + "Supervisor::__create", + "Supervisor::__destroy", + "Supervisor::__init_cluster", + "Supervisor::__is_supervised", + "Supervisor::__is_supervisor", + "Supervisor::__node", + "Supervisor::__restart", + "Supervisor::__status", + "Supervisor::__stem_pid", + "Telemetry::__collect_histogram_metrics", + "Telemetry::__collect_metrics", + "Telemetry::__dbl_counter_family", + "Telemetry::__dbl_counter_inc", + "Telemetry::__dbl_counter_metric_get_or_add", + "Telemetry::__dbl_counter_value", + "Telemetry::__dbl_gauge_dec", + "Telemetry::__dbl_gauge_family", + "Telemetry::__dbl_gauge_inc", + "Telemetry::__dbl_gauge_metric_get_or_add", + "Telemetry::__dbl_gauge_value", + "Telemetry::__dbl_histogram_family", + "Telemetry::__dbl_histogram_metric_get_or_add", + "Telemetry::__dbl_histogram_observe", + "Telemetry::__dbl_histogram_sum", + "Telemetry::__int_counter_family", + "Telemetry::__int_counter_inc", + "Telemetry::__int_counter_metric_get_or_add", + "Telemetry::__int_counter_value", + "Telemetry::__int_gauge_dec", + "Telemetry::__int_gauge_family", + "Telemetry::__int_gauge_inc", + "Telemetry::__int_gauge_metric_get_or_add", + "Telemetry::__int_gauge_value", + "Telemetry::__int_histogram_family", + "Telemetry::__int_histogram_metric_get_or_add", + "Telemetry::__int_histogram_observe", + "Telemetry::__int_histogram_sum", + "__init_primary_bifs", + "__init_secondary_bifs", + "active_file", + "addr_to_counts", + "addr_to_ptr_name", + "addr_to_subnet", + "all_set", + "anonymize_addr", + "any_set", + "backtrace", + "bare_mode", + "bloomfilter_add", + "bloomfilter_basic_init", + "bloomfilter_basic_init2", + "bloomfilter_clear", + "bloomfilter_counting_init", + "bloomfilter_decrement", + "bloomfilter_internal_state", + "bloomfilter_intersect", + "bloomfilter_lookup", + "bloomfilter_merge", + "bytestring_to_count", + "bytestring_to_double", + "bytestring_to_float", + "bytestring_to_hexstr", + "calc_next_rotate", + "cat", + "cat_sep", + "ceil", + "check_subnet", + "clean", + "close", + "community_id_v1", + "compress_path", + "connection_exists", + "continue_processing", + "convert_for_pattern", + "count_substr", + "count_to_double", + "count_to_port", + "count_to_v4_addr", + "counts_to_addr", + "current_analyzer", + "current_event_time", + "current_time", + "decode_base64", + "decode_base64_conn", + "decode_netbios_name", + "decode_netbios_name_type", + "disable_event_group", + "disable_module_events", + "do_profiling", + "double_to_count", + "double_to_int", + "double_to_interval", + "double_to_time", + "dump_current_packet", + "dump_packet", + "dump_rule_stats", + "edit", + "enable_event_group", + "enable_module_events", + "enable_raw_output", + "encode_base64", + "ends_with", + "entropy_test_add", + "entropy_test_finish", + "entropy_test_init", + "enum_names", + "enum_to_int", + "escape_string", + "exit", + "exp", + "file_magic", + "file_mode", + "file_size", + "filter_subnet_table", + "find_all", + "find_all_ordered", + "find_entropy", + "find_last", + "find_str", + "floor", + "flush_all", + "fmt", + "fmt_ftp_port", + "fnv1a32", + "generate_all_events", + "get_broker_stats", + "get_conn_stats", + "get_conn_transport_proto", + "get_contents_file", + "get_current_conn_bytes_threshold", + "get_current_conn_duration_threshold", + "get_current_conn_packets_threshold", + "get_current_packet", + "get_current_packet_header", + "get_dns_stats", + "get_event_handler_stats", + "get_event_stats", + "get_file_analysis_stats", + "get_file_name", + "get_gap_stats", + "get_identifier_comments", + "get_identifier_declaring_script", + "get_login_state", + "get_matcher_stats", + "get_net_stats", + "get_orig_seq", + "get_package_readme", + "get_port_transport_proto", + "get_proc_stats", + "get_reassembler_stats", + "get_record_field_comments", + "get_record_field_declaring_script", + "get_reporter_stats", + "get_resp_seq", + "get_script_comments", + "get_thread_stats", + "get_timer_stats", + "getenv", + "gethostname", + "getpid", + "global_container_footprints", + "global_ids", + "global_options", + "gsub", + "has_event_group", + "has_module_events", + "have_spicy", + "have_spicy_analyzers", + "haversine_distance", + "hexdump", + "hexstr_to_bytestring", + "hll_cardinality_add", + "hll_cardinality_copy", + "hll_cardinality_estimate", + "hll_cardinality_init", + "hll_cardinality_merge_into", + "hrw_weight", + "identify_data", + "install_dst_addr_filter", + "install_dst_net_filter", + "install_src_addr_filter", + "install_src_net_filter", + "int_to_count", + "int_to_double", + "interval_to_double", + "is_alnum", + "is_alpha", + "is_ascii", + "is_file_analyzer", + "is_icmp_port", + "is_local_interface", + "is_num", + "is_packet_analyzer", + "is_processing_suspended", + "is_protocol_analyzer", + "is_remote_event", + "is_tcp_port", + "is_udp_port", + "is_v4_addr", + "is_v4_subnet", + "is_v6_addr", + "is_v6_subnet", + "is_valid_ip", + "join_string_set", + "join_string_vec", + "levenshtein_distance", + "ljust", + "ln", + "load_CPP", + "log10", + "log2", + "lookup_ID", + "lookup_addr", + "lookup_autonomous_system", + "lookup_connection", + "lookup_hostname", + "lookup_hostname_txt", + "lookup_location", + "lstrip", + "mask_addr", + "match_signatures", + "matching_subnets", + "md5_hash", + "md5_hash_finish", + "md5_hash_init", + "md5_hash_update", + "md5_hmac", + "mkdir", + "mmdb_open_asn_db", + "mmdb_open_location_db", + "network_time", + "open", + "open_for_append", + "packet_source", + "paraglob_equals", + "paraglob_init", + "paraglob_match", + "parse_distinguished_name", + "parse_eftp_port", + "parse_ftp_epsv", + "parse_ftp_pasv", + "parse_ftp_port", + "piped_exec", + "port_to_count", + "pow", + "preserve_prefix", + "preserve_subnet", + "print_raw", + "ptr_name_to_addr", + "rand", + "raw_bytes_to_v4_addr", + "raw_bytes_to_v6_addr", + "reading_live_traffic", + "reading_traces", + "record_fields", + "record_type_to_vector", + "remask_addr", + "remove_prefix", + "remove_suffix", + "rename", + "reverse", + "rfind_str", + "rjust", + "rmdir", + "rotate_file", + "rotate_file_by_name", + "routing0_data_to_addrs", + "rstrip", + "safe_shell_quote", + "same_object", + "sct_verify", + "set_buf", + "set_contents_file", + "set_current_conn_bytes_threshold", + "set_current_conn_duration_threshold", + "set_current_conn_packets_threshold", + "set_file_handle", + "set_inactivity_timeout", + "set_keys", + "set_login_state", + "set_network_time", + "set_record_packets", + "set_secret", + "set_ssl_established", + "setenv", + "sha1_hash", + "sha1_hash_finish", + "sha1_hash_init", + "sha1_hash_update", + "sha256_hash", + "sha256_hash_finish", + "sha256_hash_init", + "sha256_hash_update", + "skip_further_processing", + "skip_http_entity_data", + "skip_smtp_data", + "split_string", + "split_string1", + "split_string_all", + "split_string_n", + "sqrt", + "srand", + "starts_with", + "str_smith_waterman", + "str_split_indices", + "strcmp", + "strftime", + "string_cat", + "string_fill", + "string_to_ascii_hex", + "string_to_pattern", + "strip", + "strptime", + "strstr", + "sub", + "sub_bytes", + "subnet_to_addr", + "subnet_width", + "subst_string", + "suspend_processing", + "swap_case", + "syslog", + "system", + "system_env", + "table_keys", + "table_values", + "terminate", + "time_to_double", + "to_addr", + "to_count", + "to_double", + "to_int", + "to_json", + "to_lower", + "to_port", + "to_string_literal", + "to_subnet", + "to_title", + "to_upper", + "topk_add", + "topk_count", + "topk_epsilon", + "topk_get_top", + "topk_init", + "topk_merge", + "topk_merge_prune", + "topk_size", + "topk_sum", + "type_aliases", + "type_name", + "unescape_URI", + "uninstall_dst_addr_filter", + "uninstall_dst_net_filter", + "uninstall_src_addr_filter", + "uninstall_src_net_filter", + "unique_id", + "unique_id_from", + "unlink", + "uuid_to_string", + "val_footprint", + "write_file", + "x509_check_cert_hostname", + "x509_check_hostname", + "x509_from_der", + "x509_get_certificate_string", + "x509_issuer_name_hash", + "x509_ocsp_verify", + "x509_parse", + "x509_set_certificate_cache", + "x509_set_certificate_cache_hit_callback", + "x509_spki_hash", + "x509_subject_name_hash", + "x509_verify", + "zeek_args", + "zeek_is_terminating", + "zeek_version", + "zfill", +}; + +// Ones not listed: +// +// Broker::* +// These can manipulate unspecified (at script level) records. +// +// Cluster::publish_hrw +// Cluster::publish_rr +// These call script functions to get topic names. +// +// Log::__write +// Calls log policy functions. +// +// Option::set +// Both explicitly changes a global and potentially calls a +// function specified at run-time. +// +// clear_table +// Both clears a set/table and potentially calls an &on_change handler. +// +// disable_analyzer +// Can call Analyzer::disabling_analyzer hook. +// +// from_json +// Can call a normalization function. +// +// order +// Can call a comparison function. +// +// resize +// Changes a vector in place. +// +// sort +// Both changes a vector in place and can call an arbitrary comparison +// function. +// +// Some of these have side effects that could be checked for in a specific +// context, but the gains from doing so likely aren't worth the complexity. + +bool is_side_effect_free(std::string func_name) { return side_effects_free_BiFs.count(func_name) > 0; } + +} // namespace zeek::detail diff --git a/src/script_opt/FuncInfo.h b/src/script_opt/FuncInfo.h new file mode 100644 index 0000000000..a7925619ae --- /dev/null +++ b/src/script_opt/FuncInfo.h @@ -0,0 +1,17 @@ +// See the file "COPYING" in the main distribution directory for copyright. + +// Utility functions that return information about Zeek functions. Currently +// this is limited to information about whether BiFs are side-effect-free +// (from a Zeek scripting perspective), but could be expanded in the future +// to include information about Zeek script functions, idempotency, and the +// like. + +#pragma once + +#include "zeek/Func.h" + +namespace zeek::detail { + +extern bool is_side_effect_free(std::string func_name); + +} // namespace zeek::detail diff --git a/src/script_opt/GenIDDefs.cc b/src/script_opt/GenIDDefs.cc index 209176b3d7..836e99586b 100644 --- a/src/script_opt/GenIDDefs.cc +++ b/src/script_opt/GenIDDefs.cc @@ -62,7 +62,45 @@ TraversalCode GenIDDefs::PreStmt(const Stmt* s) { auto block = cr->Block(); cr_active.push_back(confluence_blocks.size()); - block->Traverse(this); + + // Confluence for the bodies of catch-return's is a bit complex. + // We would like any expressions computed at the outermost level + // of the body to be available for script optimization *outside* + // the catch-return; this in particular is helpful in optimizing + // coalesced event handlers, but has other benefits as well. + // + // However, if one of the outermost statements executes a "return", + // then any outermost expressions computed after it might not + // be available. Put another way, the potentially-returning + // statement starts a confluence region that runs through the end + // of the body. + // + // To deal with this, we start off without a new confluence block, + // but create one upon encountering a statement that could return. + + bool did_confluence = false; + + if ( block->Tag() == STMT_LIST ) { + auto prev_stmt = s; + auto& stmts = block->AsStmtList()->Stmts(); + for ( auto& st : stmts ) { + if ( ! did_confluence && st->CouldReturn(false) ) { + StartConfluenceBlock(prev_stmt); + did_confluence = true; + } + + st->Traverse(this); + } + } + else + // If there's just a single statement then there are no + // expressions computed subsequent to it that we need to + // worry about, so just do ordinary traversal. + block->Traverse(this); + + if ( did_confluence ) + EndConfluenceBlock(); + cr_active.pop_back(); auto retvar = cr->RetVar(); diff --git a/src/script_opt/GenIDDefs.h b/src/script_opt/GenIDDefs.h index bc1c2785bd..a8b05864cb 100644 --- a/src/script_opt/GenIDDefs.h +++ b/src/script_opt/GenIDDefs.h @@ -93,9 +93,10 @@ private: // Stack of confluence blocks corresponding to activate catch-return // statements. We used to stop identifier definitions at these - // boundaries, but given there's no confluence (i.e., the body of the - // catch-return *will* execute), we can do broader optimization if we - // don't treat them as their own (new) confluence blocks. + // boundaries, but given there's limited confluence (i.e., the body of + // the catch-return *will* execute, up through its first return), we + // can do broader optimization if we don't treat them as their own + // (new) confluence blocks. std::vector cr_active; // The following is parallel to confluence_blocks except diff --git a/src/script_opt/ProfileFunc.cc b/src/script_opt/ProfileFunc.cc index bd4990113d..fc5abc6cbd 100644 --- a/src/script_opt/ProfileFunc.cc +++ b/src/script_opt/ProfileFunc.cc @@ -8,6 +8,7 @@ #include "zeek/Desc.h" #include "zeek/Func.h" #include "zeek/Stmt.h" +#include "zeek/script_opt/FuncInfo.h" #include "zeek/script_opt/IDOptInfo.h" namespace zeek::detail { @@ -85,7 +86,16 @@ TraversalCode ProfileFunc::PreStmt(const Stmt* s) { case STMT_INIT: for ( const auto& id : s->AsInitStmt()->Inits() ) { inits.insert(id.get()); - TrackType(id->GetType()); + + auto& t = id->GetType(); + TrackType(t); + + auto attrs = id->GetAttrs(); + if ( attrs ) + constructor_attrs[attrs.get()] = t; + + if ( t->Tag() == TYPE_RECORD ) + CheckRecordConstructor(t); } // Don't traverse further into the statement, since we @@ -147,6 +157,14 @@ TraversalCode ProfileFunc::PreStmt(const Stmt* s) { expr_switches.insert(sw); } break; + case STMT_ADD: + case STMT_DELETE: { + auto ad_stmt = static_cast(s); + auto ad_e = ad_stmt->StmtExpr(); + auto& lhs_t = ad_e->GetOp1()->GetType(); + aggr_mods.insert(lhs_t.get()); + } break; + default: break; } @@ -221,32 +239,89 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { } break; + case EXPR_INDEX: { + auto lhs_t = e->GetOp1()->GetType(); + if ( lhs_t->Tag() == TYPE_TABLE ) + tbl_refs.insert(lhs_t.get()); + } break; + case EXPR_INCR: case EXPR_DECR: case EXPR_ADD_TO: case EXPR_REMOVE_FROM: case EXPR_ASSIGN: { - if ( e->GetOp1()->Tag() != EXPR_REF ) - // this isn't a direct assignment + auto lhs = e->GetOp1(); + + if ( lhs->Tag() == EXPR_REF ) + lhs = lhs->GetOp1(); + + else if ( e->Tag() == EXPR_ASSIGN ) + // This isn't a direct assignment, but instead an overloaded + // use of "=" such as in a table constructor. break; - auto lhs = e->GetOp1()->GetOp1(); - if ( lhs->Tag() != EXPR_NAME ) - break; + auto lhs_t = lhs->GetType(); - auto id = lhs->AsNameExpr()->Id(); - TrackAssignment(id); + switch ( lhs->Tag() ) { + case EXPR_NAME: { + auto id = lhs->AsNameExpr()->Id(); + TrackAssignment(id); - if ( e->Tag() == EXPR_ASSIGN ) { - auto a_e = static_cast(e); - auto& av = a_e->AssignVal(); - if ( av ) - // This is a funky "local" assignment - // inside a when clause. - when_locals.insert(id); + if ( e->Tag() == EXPR_ASSIGN ) { + auto a_e = static_cast(e); + auto& av = a_e->AssignVal(); + if ( av ) + // This is a funky "local" assignment + // inside a when clause. + when_locals.insert(id); + } + else if ( IsAggr(lhs_t->Tag()) ) + aggr_mods.insert(lhs_t.get()); + } break; + + case EXPR_INDEX: { + auto lhs_aggr = lhs->GetOp1(); + auto lhs_aggr_t = lhs_aggr->GetType(); + + // Determine which aggregate is being modified. For an + // assignment "a[b] = aggr", it's not a[b]'s type but + // rather a's type. However, for any of the others, + // e.g. "a[b] -= aggr" it is a[b]'s type. + if ( e->Tag() == EXPR_ASSIGN ) + aggr_mods.insert(lhs_aggr_t.get()); + else + aggr_mods.insert(lhs_t.get()); + + if ( lhs_aggr_t->Tag() == TYPE_TABLE ) { + // We don't want the default recursion into the + // expression's LHS because that will treat this + // table modification as a reference instead. So + // do it manually. Given that, we need to do the + // expression's RHS manually too. + lhs->GetOp1()->Traverse(this); + lhs->GetOp2()->Traverse(this); + + auto rhs = e->GetOp2(); + if ( rhs ) + rhs->Traverse(this); + + return TC_ABORTSTMT; + } + } break; + + case EXPR_FIELD: aggr_mods.insert(lhs_t.get()); break; + + case EXPR_LIST: { + for ( auto id : lhs->AsListExpr()->Exprs() ) { + auto id_t = id->GetType(); + if ( IsAggr(id_t->Tag()) ) + aggr_mods.insert(id_t.get()); + } + } break; + + default: reporter->InternalError("bad expression in ProfileFunc: %s", obj_desc(e).c_str()); } - break; - } + } break; case EXPR_CALL: { auto c = e->AsCallExpr(); @@ -272,8 +347,8 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { auto func_vf = func_v->AsFunc(); if ( func_vf->GetKind() == Func::SCRIPT_FUNC ) { - auto bf = static_cast(func_vf); - script_calls.insert(bf); + auto sf = static_cast(func_vf); + script_calls.insert(sf); } else BiF_globals.insert(func); @@ -329,18 +404,20 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { // In general, we don't want to recurse into the body. // However, we still want to *profile* it so we can // identify calls within it. - ProfileFunc body_pf(l->Ingredients()->Body().get(), false); - script_calls.insert(body_pf.ScriptCalls().begin(), body_pf.ScriptCalls().end()); + auto pf = std::make_shared(l->Ingredients()->Body().get(), false); + script_calls.insert(pf->ScriptCalls().begin(), pf->ScriptCalls().end()); return TC_ABORTSTMT; } + case EXPR_RECORD_CONSTRUCTOR: CheckRecordConstructor(e->GetType()); break; + case EXPR_SET_CONSTRUCTOR: { auto sc = static_cast(e); const auto& attrs = sc->GetAttrs(); if ( attrs ) - constructor_attrs.insert(attrs.get()); + constructor_attrs[attrs.get()] = sc->GetType(); } break; case EXPR_TABLE_CONSTRUCTOR: { @@ -348,7 +425,24 @@ TraversalCode ProfileFunc::PreExpr(const Expr* e) { const auto& attrs = tc->GetAttrs(); if ( attrs ) - constructor_attrs.insert(attrs.get()); + constructor_attrs[attrs.get()] = tc->GetType(); + } break; + + case EXPR_RECORD_COERCE: + // This effectively does a record construction of the target + // type, so check that. + CheckRecordConstructor(e->GetType()); + break; + + case EXPR_TABLE_COERCE: { + // This is written without casting so it can work with other + // types if needed. + auto res_type = e->GetType().get(); + auto orig_type = e->GetOp1()->GetType().get(); + if ( type_aliases.count(res_type) == 0 ) + type_aliases[orig_type] = {res_type}; + else + type_aliases[orig_type].insert(res_type); } break; default: break; @@ -395,20 +489,42 @@ void ProfileFunc::TrackAssignment(const ID* id) { ++assignees[id]; else assignees[id] = 1; + + if ( id->IsGlobal() || captures.count(id) > 0 ) + non_local_assignees.insert(id); +} + +void ProfileFunc::CheckRecordConstructor(TypePtr t) { + auto rt = cast_intrusive(t); + for ( auto td : *rt->Types() ) + if ( td->attrs ) { + // In principle we could figure out whether this particular + // constructor happens to explicitly specify &default fields, and + // not include those attributes if it does since they won't come + // into play. However that seems like added complexity for almost + // surely no ultimate gain. + auto attrs = td->attrs.get(); + constructor_attrs[attrs] = rt; + + if ( rec_constructor_attrs.count(rt.get()) == 0 ) + rec_constructor_attrs[rt.get()] = {attrs}; + else + rec_constructor_attrs[rt.get()].insert(attrs); + } } ProfileFuncs::ProfileFuncs(std::vector& funcs, is_compilable_pred pred, bool _full_record_hashes) { full_record_hashes = _full_record_hashes; for ( auto& f : funcs ) { - if ( f.ShouldSkip() ) - continue; - - auto pf = std::make_unique(f.Func(), f.Body(), full_record_hashes); + auto pf = std::make_shared(f.Func(), f.Body(), full_record_hashes); if ( ! pred || (*pred)(pf.get(), nullptr) ) MergeInProfile(pf.get()); + // Track the profile even if we're not compiling the function, since + // the AST optimizer will still need it to reason about function-call + // side effects. f.SetProfile(std::move(pf)); func_profs[f.Func()] = f.ProfilePtr(); } @@ -432,6 +548,81 @@ ProfileFuncs::ProfileFuncs(std::vector& funcs, is_compilable_pred pred // Computing those hashes could have led to traversals that // create more pending expressions to analyze. } while ( ! pending_exprs.empty() ); + + // Now that we have everything profiled, we can proceed to analyses + // that require full global information. + ComputeSideEffects(); +} + +bool ProfileFuncs::IsTableWithDefaultAggr(const Type* t) { + auto analy = tbl_has_aggr_default.find(t); + if ( analy != tbl_has_aggr_default.end() ) + // We already have the answer. + return analy->second; + + // See whether an alias for the type has already been resolved. + if ( t->AsTableType()->Yield() ) { + for ( auto& at : tbl_has_aggr_default ) + if ( same_type(at.first, t) ) { + tbl_has_aggr_default[t] = at.second; + return at.second; + } + } + + tbl_has_aggr_default[t] = false; + return false; +} + +bool ProfileFuncs::HasSideEffects(SideEffectsOp::AccessType access, const TypePtr& t) const { + IDSet nli; + TypeSet aggrs; + + if ( GetSideEffects(access, t.get(), nli, aggrs) ) + return true; + + return ! nli.empty() || ! aggrs.empty(); +} + +bool ProfileFuncs::GetSideEffects(SideEffectsOp::AccessType access, const Type* t, IDSet& non_local_ids, + TypeSet& aggrs) const { + for ( auto se : side_effects_ops ) + if ( AssessSideEffects(se.get(), access, t, non_local_ids, aggrs) ) + return true; + + return false; +} + +bool ProfileFuncs::GetCallSideEffects(const NameExpr* n, IDSet& non_local_ids, TypeSet& aggrs, bool& is_unknown) { + auto fid = n->Id(); + auto fv = fid->GetVal(); + + if ( ! fv || ! fid->IsConst() ) { + // The value is unavailable (likely a bug), or might change at run-time. + is_unknown = true; + return true; + } + + auto func = fv->AsFunc(); + if ( func->GetKind() == Func::BUILTIN_FUNC ) { + if ( ! is_side_effect_free(func->Name()) ) + is_unknown = true; + return true; + } + + auto sf = static_cast(func); + auto seo = GetCallSideEffects(sf); + if ( ! seo ) + return false; + + if ( seo->HasUnknownChanges() ) + is_unknown = true; + + for ( auto a : seo->ModAggrs() ) + aggrs.insert(a); + for ( auto nl : seo->ModNonLocals() ) + non_local_ids.insert(nl); + + return true; } void ProfileFuncs::MergeInProfile(ProfileFunc* pf) { @@ -460,7 +651,7 @@ void ProfileFuncs::MergeInProfile(ProfileFunc* pf) { auto& attrs = g->GetAttrs(); if ( attrs ) - AnalyzeAttrs(attrs.get()); + AnalyzeAttrs(attrs.get(), t.get()); } constants.insert(pf->Constants().begin(), pf->Constants().end()); @@ -475,7 +666,13 @@ void ProfileFuncs::MergeInProfile(ProfileFunc* pf) { } for ( auto& a : pf->ConstructorAttrs() ) - AnalyzeAttrs(a); + AnalyzeAttrs(a.first, a.second.get()); + + for ( auto& ta : pf->TypeAliases() ) { + if ( type_aliases.count(ta.first) == 0 ) + type_aliases[ta.first] = std::set{}; + type_aliases[ta.first].insert(ta.second.begin(), ta.second.end()); + } } void ProfileFuncs::TraverseValue(const ValPtr& v) { @@ -579,8 +776,12 @@ void ProfileFuncs::ComputeBodyHashes(std::vector& funcs) { if ( ! f.ShouldSkip() ) ComputeProfileHash(f.ProfilePtr()); - for ( auto& l : lambdas ) - ComputeProfileHash(ExprProf(l)); + for ( auto& l : lambdas ) { + auto pf = ExprProf(l); + func_profs[l->PrimaryFunc().get()] = pf; + lambda_primaries[l->Name()] = l->PrimaryFunc().get(); + ComputeProfileHash(pf); + } } void ProfileFuncs::ComputeProfileHash(std::shared_ptr pf) { @@ -710,11 +911,8 @@ p_hash_type ProfileFuncs::HashType(const Type* t) { // We don't hash the field name, as in some contexts // those are ignored. - if ( f->attrs ) { - if ( do_hash ) - h = merge_p_hashes(h, HashAttrs(f->attrs)); - AnalyzeAttrs(f->attrs.get()); - } + if ( f->attrs && do_hash ) + h = merge_p_hashes(h, HashAttrs(f->attrs)); } } break; @@ -731,8 +929,24 @@ p_hash_type ProfileFuncs::HashType(const Type* t) { auto ft = t->AsFuncType(); auto flv = ft->FlavorString(); h = merge_p_hashes(h, p_hash(flv)); + + // We deal with the parameters individually, rather than just + // recursing into the RecordType that's used (for convenience) + // to represent them. We do so because their properties are + // somewhat different - in particular, an &default on a parameter + // field is resolved in the context of the caller, not the + // function itself, and so we don't want to track those as + // attributes associated with the function body's execution. h = merge_p_hashes(h, p_hash("params")); - h = merge_p_hashes(h, HashType(ft->Params())); + auto params = ft->Params()->Types(); + + if ( params ) { + h = merge_p_hashes(h, p_hash(params->length())); + + for ( auto p : *params ) + h = merge_p_hashes(h, HashType(p->type)); + } + h = merge_p_hashes(h, p_hash("func-yield")); h = merge_p_hashes(h, HashType(ft->Yield())); } break; @@ -803,18 +1017,367 @@ p_hash_type ProfileFuncs::HashAttrs(const AttributesPtr& Attrs) { return h; } -void ProfileFuncs::AnalyzeAttrs(const Attributes* Attrs) { - auto attrs = Attrs->GetAttrs(); +void ProfileFuncs::AnalyzeAttrs(const Attributes* attrs, const Type* t) { + for ( const auto& a : attrs->GetAttrs() ) { + auto& e = a->GetExpr(); - for ( const auto& a : attrs ) { - const Expr* e = a->GetExpr().get(); + if ( ! e ) + continue; - if ( e ) { - pending_exprs.push_back(e); - if ( e->Tag() == EXPR_LAMBDA ) - lambdas.insert(e->AsLambdaExpr()); + pending_exprs.push_back(e.get()); + + auto prev_ea = expr_attrs.find(a.get()); + if ( prev_ea == expr_attrs.end() ) + expr_attrs[a.get()] = {t}; + else { + // Add it if new. This is rare, but can arise due to attributes + // being shared for example from initializers with a variable + // itself. + bool found = false; + for ( auto ea : prev_ea->second ) + if ( ea == t ) { + found = true; + break; + } + + if ( ! found ) + prev_ea->second.push_back(t); + } + + if ( e->Tag() == EXPR_LAMBDA ) + lambdas.insert(e->AsLambdaExpr()); + } +} + +void ProfileFuncs::ComputeSideEffects() { + // Computing side effects is an iterative process, because whether + // a given expression has a side effect can depend on whether it + // includes accesses to types that themselves have side effects. + + // Step one: assemble the candidate pool of attributes to assess. + for ( auto& ea : expr_attrs ) { + // Is this an attribute that can be triggered by + // statement/expression execution? + auto a = ea.first; + auto at = a->Tag(); + if ( at == ATTR_DEFAULT || at == ATTR_DEFAULT_INSERT || at == ATTR_ON_CHANGE ) { + if ( at == ATTR_DEFAULT ) { + // Look for tables with &default's returning aggregate values. + for ( auto t : ea.second ) { + if ( t->Tag() != TYPE_TABLE ) + continue; + + auto y = t->AsTableType()->Yield(); + + if ( y && IsAggr(y->Tag()) ) { + tbl_has_aggr_default[t] = true; + for ( auto ta : type_aliases[t] ) + tbl_has_aggr_default[ta] = true; + } + } + } + + // Weed out very-common-and-completely-safe expressions. + if ( ! DefinitelyHasNoSideEffects(a->GetExpr()) ) + candidates.insert(a); + } + } + + // At this point, very often there are no candidates and we're done. + // However, if we have candidates then we need to process them in an + // iterative fashion because it's possible that the side effects of + // some of them depend on the side effects of other candidates. + + while ( ! candidates.empty() ) { + // For which attributes have we resolved their status. + AttrSet made_decision; + + for ( auto c : candidates ) { + IDSet non_local_ids; + TypeSet aggrs; + bool is_unknown = false; + + // Track the candidate we're currently analyzing, since sometimes + // it's self-referential and we need to identify that fact. + curr_candidate = c; + + if ( ! AssessSideEffects(c->GetExpr(), non_local_ids, aggrs, is_unknown) ) + // Can't make a decision yet. + continue; + + // We've resolved this candidate. + made_decision.insert(c); + SetSideEffects(c, non_local_ids, aggrs, is_unknown); + } + + if ( made_decision.empty() ) { + // We weren't able to make forward progress. This happens when + // the pending candidates are mutually dependent. While in + // principle we could scope the worst-case resolution of their + // side effects, this is such an unlikely situation that we just + // mark them all as unknown. + + // We keep these empty. + IDSet non_local_ids; + TypeSet aggrs; + + for ( auto c : candidates ) + SetSideEffects(c, non_local_ids, aggrs, true); + + // We're now all done. + break; + } + + for ( auto md : made_decision ) + candidates.erase(md); + } +} + +bool ProfileFuncs::DefinitelyHasNoSideEffects(const ExprPtr& e) const { + if ( e->Tag() == EXPR_CONST || e->Tag() == EXPR_VECTOR_CONSTRUCTOR ) + return true; + + if ( e->Tag() == EXPR_NAME ) + return e->GetType()->Tag() != TYPE_FUNC; + + auto ep = expr_profs.find(e.get()); + ASSERT(ep != expr_profs.end()); + + const auto& pf = ep->second; + + if ( ! pf->NonLocalAssignees().empty() || ! pf->TableRefs().empty() || ! pf->AggrMods().empty() || + ! pf->ScriptCalls().empty() ) + return false; + + for ( auto& b : pf->BiFGlobals() ) + if ( ! is_side_effect_free(b->Name()) ) + return false; + + return true; +} + +void ProfileFuncs::SetSideEffects(const Attr* a, IDSet& non_local_ids, TypeSet& aggrs, bool is_unknown) { + auto seo_vec = std::vector>{}; + bool is_rec = expr_attrs[a][0]->Tag() == TYPE_RECORD; + + SideEffectsOp::AccessType at; + if ( is_rec ) + at = SideEffectsOp::CONSTRUCTION; + else if ( a->Tag() == ATTR_ON_CHANGE ) + at = SideEffectsOp::WRITE; + else + at = SideEffectsOp::READ; + + if ( non_local_ids.empty() && aggrs.empty() && ! is_unknown ) + // Definitely no side effects. + seo_vec.push_back(std::make_shared()); + else { + attrs_with_side_effects.insert(a); + + // Set side effects for all of the types associated with this attribute. + for ( auto ea_t : expr_attrs[a] ) { + auto seo = std::make_shared(at, ea_t); + seo->AddModNonGlobal(non_local_ids); + seo->AddModAggrs(aggrs); + + if ( is_unknown ) + seo->SetUnknownChanges(); + + side_effects_ops.push_back(seo); + seo_vec.push_back(std::move(seo)); + } + } + + if ( is_rec ) + record_constr_with_side_effects[a] = std::move(seo_vec); + else + aggr_side_effects[a] = std::move(seo_vec); +} + +AttrVec ProfileFuncs::AssociatedAttrs(const Type* t) { + AttrVec assoc_attrs; + + // Search both the pending candidates and the ones already identified. + // You might think we'd just do the latter, but we want to include the + // pending ones, too, so we can identify not-yet-resolved dependencies. + FindAssociatedAttrs(candidates, t, assoc_attrs); + FindAssociatedAttrs(attrs_with_side_effects, t, assoc_attrs); + + return assoc_attrs; +} + +void ProfileFuncs::FindAssociatedAttrs(const AttrSet& attrs, const Type* t, AttrVec& assoc_attrs) { + for ( auto a : attrs ) { + for ( auto ea_t : expr_attrs[a] ) { + if ( same_type(t, ea_t) ) { + assoc_attrs.push_back(a); + break; + } + + for ( auto ta : type_aliases[ea_t] ) + if ( same_type(t, ta) ) { + assoc_attrs.push_back(a); + break; + } } } } +bool ProfileFuncs::AssessSideEffects(const ExprPtr& e, IDSet& non_local_ids, TypeSet& aggrs, bool& is_unknown) { + if ( e->Tag() == EXPR_NAME && e->GetType()->Tag() == TYPE_FUNC ) + // This occurs when the expression is itself a function name, and + // in an attribute context indicates an implicit call. + return GetCallSideEffects(e->AsNameExpr(), non_local_ids, aggrs, is_unknown); + + ASSERT(expr_profs.count(e.get()) != 0); + auto pf = expr_profs[e.get()]; + return AssessSideEffects(pf.get(), non_local_ids, aggrs, is_unknown); +} + +bool ProfileFuncs::AssessSideEffects(const ProfileFunc* pf, IDSet& non_local_ids, TypeSet& aggrs, bool& is_unknown) { + if ( pf->DoesIndirectCalls() ) { + is_unknown = true; + return true; + } + + for ( auto& b : pf->BiFGlobals() ) + if ( ! is_side_effect_free(b->Name()) ) { + is_unknown = true; + return true; + } + + IDSet nla; + TypeSet mod_aggrs; + + for ( auto& a : pf->NonLocalAssignees() ) + nla.insert(a); + + for ( auto& r : pf->RecordConstructorAttrs() ) + if ( ! AssessAggrEffects(SideEffectsOp::CONSTRUCTION, r.first, nla, mod_aggrs, is_unknown) ) + // Not enough information yet to know all of the side effects. + return false; + + for ( auto& tr : pf->TableRefs() ) + if ( ! AssessAggrEffects(SideEffectsOp::READ, tr, nla, mod_aggrs, is_unknown) ) + return false; + + for ( auto& tm : pf->AggrMods() ) { + if ( tm->Tag() == TYPE_TABLE && ! AssessAggrEffects(SideEffectsOp::WRITE, tm, nla, mod_aggrs, is_unknown) ) + return false; + + mod_aggrs.insert(tm); + } + + for ( auto& f : pf->ScriptCalls() ) { + if ( f->Flavor() != FUNC_FLAVOR_FUNCTION ) { + // A hook (since events can't be called) - not something + // to analyze further. + is_unknown = true; + return true; + } + + auto pff = func_profs[f]; + if ( active_func_profiles.count(pff) > 0 ) + // We're already processing this function and arrived here via + // recursion. Skip further analysis here, we'll do it instead + // for the original instance. + continue; + + // Track this analysis so we can detect recursion. + active_func_profiles.insert(pff); + auto a = AssessSideEffects(pff.get(), nla, mod_aggrs, is_unknown); + active_func_profiles.erase(pff); + + if ( ! a ) + return false; + } + + non_local_ids.insert(nla.begin(), nla.end()); + aggrs.insert(mod_aggrs.begin(), mod_aggrs.end()); + + return true; +} + +bool ProfileFuncs::AssessAggrEffects(SideEffectsOp::AccessType access, const Type* t, IDSet& non_local_ids, + TypeSet& aggrs, bool& is_unknown) { + auto assoc_attrs = AssociatedAttrs(t); + + for ( auto a : assoc_attrs ) { + if ( a == curr_candidate ) + // Self-reference - don't treat the absence of any determination + // for it as meaning we can't resolve the candidate. + continue; + + // See whether we've already determined the side affects associated + // with this attribute. + auto ase = aggr_side_effects.find(a); + if ( ase == aggr_side_effects.end() ) { + ase = record_constr_with_side_effects.find(a); + if ( ase == record_constr_with_side_effects.end() ) + // Haven't resolved it yet, so can't resolve current candidate. + return false; + } + + for ( auto& se : ase->second ) + if ( AssessSideEffects(se.get(), access, t, non_local_ids, aggrs) ) { + is_unknown = true; + return true; + } + } + + return true; +} + +bool ProfileFuncs::AssessSideEffects(const SideEffectsOp* se, SideEffectsOp::AccessType access, const Type* t, + IDSet& non_local_ids, TypeSet& aggrs) const { + // First determine whether the SideEffectsOp applies. + if ( se->GetAccessType() != access ) + return false; + + if ( ! same_type(se->GetType(), t) ) + return false; + + // It applies, return its effects. + if ( se->HasUnknownChanges() ) + return true; + + for ( auto a : se->ModAggrs() ) + aggrs.insert(a); + for ( auto nl : se->ModNonLocals() ) + non_local_ids.insert(nl); + + return false; +} + +std::shared_ptr ProfileFuncs::GetCallSideEffects(const ScriptFunc* sf) { + if ( lambda_primaries.count(sf->Name()) > 0 ) + sf = lambda_primaries[sf->Name()]; + + auto sf_se = func_side_effects.find(sf); + if ( sf_se != func_side_effects.end() ) + // Return cached result. + return sf_se->second; + + bool is_unknown = false; + IDSet nla; + TypeSet mod_aggrs; + + ASSERT(func_profs.count(sf) != 0); + auto pf = func_profs[sf]; + if ( ! AssessSideEffects(pf.get(), nla, mod_aggrs, is_unknown) ) + // Can't figure it out yet. + return nullptr; + + auto seo = std::make_shared(SideEffectsOp::CALL); + seo->AddModNonGlobal(nla); + seo->AddModAggrs(mod_aggrs); + + if ( is_unknown ) + seo->SetUnknownChanges(); + + func_side_effects[sf] = seo; + + return seo; +} + } // namespace zeek::detail diff --git a/src/script_opt/ProfileFunc.h b/src/script_opt/ProfileFunc.h index cf34b1b6df..27c68a215c 100644 --- a/src/script_opt/ProfileFunc.h +++ b/src/script_opt/ProfileFunc.h @@ -63,6 +63,9 @@ inline p_hash_type merge_p_hashes(p_hash_type h1, p_hash_type h2) { return h1 ^ (h2 + 0x9e3779b9 + (h1 << 6) + (h1 >> 2)); } +using AttrSet = std::unordered_set; +using AttrVec = std::vector; + // Class for profiling the components of a single function (or expression). class ProfileFunc : public TraversalCallback { public: @@ -93,6 +96,9 @@ public: const IDSet& WhenLocals() const { return when_locals; } const IDSet& Params() const { return params; } const std::unordered_map& Assignees() const { return assignees; } + const IDSet& NonLocalAssignees() const { return non_local_assignees; } + const auto& TableRefs() const { return tbl_refs; } + const auto& AggrMods() const { return aggr_mods; } const IDSet& Inits() const { return inits; } const std::vector& Stmts() const { return stmts; } const std::vector& Exprs() const { return exprs; } @@ -100,16 +106,20 @@ public: const std::vector& Constants() const { return constants; } const IDSet& UnorderedIdentifiers() const { return ids; } const std::vector& OrderedIdentifiers() const { return ordered_ids; } - const std::unordered_set& UnorderedTypes() const { return types; } + const TypeSet& UnorderedTypes() const { return types; } const std::vector& OrderedTypes() const { return ordered_types; } + const auto& TypeAliases() const { return type_aliases; } const std::unordered_set& ScriptCalls() const { return script_calls; } const IDSet& BiFGlobals() const { return BiF_globals; } const std::unordered_set& Events() const { return events; } - const std::unordered_set& ConstructorAttrs() const { return constructor_attrs; } + const std::unordered_map& ConstructorAttrs() const { return constructor_attrs; } + const std::unordered_map>& RecordConstructorAttrs() const { + return rec_constructor_attrs; + } const std::unordered_set& ExprSwitches() const { return expr_switches; } const std::unordered_set& TypeSwitches() const { return type_switches; } - bool DoesIndirectCalls() { return does_indirect_calls; } + bool DoesIndirectCalls() const { return does_indirect_calls; } int NumParams() const { return num_params; } int NumLambdas() const { return lambdas.size(); } @@ -139,6 +149,10 @@ protected: // Take note of an assignment to an identifier. void TrackAssignment(const ID* id); + // Extracts attributes of a record type used in a constructor (or implicit + // initialization, or coercion, which does an implicit construction). + void CheckRecordConstructor(TypePtr t); + // The function, body, or expression profiled. Can be null // depending on which constructor was used. const Func* profiled_func = nullptr; @@ -175,6 +189,15 @@ protected: // captured in "inits". std::unordered_map assignees; + // A subset of assignees reflecting those that are globals or captures. + IDSet non_local_assignees; + + // TableType's that are used in table references (i.e., index operations). + TypeSet tbl_refs; + + // Types corresponding to aggregates that are modified. + TypeSet aggr_mods; + // Same for locals seen in initializations, so we can find, // for example, unused aggregates. IDSet inits; @@ -209,11 +232,15 @@ protected: // Types seen in the function. A set rather than a vector because // the same type can be seen numerous times. - std::unordered_set types; + TypeSet types; // The same, but in a deterministic order, with duplicates removed. std::vector ordered_types; + // For a given type (seen in an attribute), tracks other types that + // are effectively aliased with it via coercions. + std::unordered_map> type_aliases; + // Script functions that this script calls. Includes calls made // by lambdas and when bodies, as the goal is to identify recursion. std::unordered_set script_calls; @@ -228,8 +255,13 @@ protected: // Names of generated events. std::unordered_set events; - // Attributes seen in set or table constructors. - std::unordered_set constructor_attrs; + // Attributes seen in set, table, or record constructors, mapped back + // to the type where they appear. + std::unordered_map constructor_attrs; + + // Attributes associated with record constructors. There can be several, + // so we use a set. + std::unordered_map> rec_constructor_attrs; // Switch statements with either expression cases or type cases. std::unordered_set expr_switches; @@ -256,6 +288,50 @@ protected: bool abs_rec_fields; }; +// Describes an operation for which some forms of access can lead to state +// modifications. +class SideEffectsOp { +public: + // Access types correspond to: + // NONE - there are no side effects + // CALL - relevant for function calls + // CONSTRUCTION - relevant for constructing/coercing a record + // READ - relevant for reading a table element + // WRITE - relevant for modifying a table element + enum AccessType { NONE, CALL, CONSTRUCTION, READ, WRITE }; + + SideEffectsOp(AccessType at = NONE, const Type* t = nullptr) : access(at), type(t) {} + + auto GetAccessType() const { return access; } + const Type* GetType() const { return type; } + + void SetUnknownChanges() { has_unknown_changes = true; } + bool HasUnknownChanges() const { return has_unknown_changes; } + + void AddModNonGlobal(IDSet ids) { mod_non_locals.insert(ids.begin(), ids.end()); } + void AddModAggrs(TypeSet types) { mod_aggrs.insert(types.begin(), types.end()); } + + const auto& ModNonLocals() const { return mod_non_locals; } + const auto& ModAggrs() const { return mod_aggrs; } + +private: + AccessType access; + const Type* type; // type for which some operations alter state + + // Globals and/or captures that the operation potentially modifies. + IDSet mod_non_locals; + + // Aggregates (specified by types) that potentially modified. + TypeSet mod_aggrs; + + // Sometimes the side effects are not known (such as when making + // indirect function calls, so we can't know statically what function + // will be called). We refer to as Unknown, and their implications are + // presumed to be worst-case - any non-local or aggregate is potentially + // affected. + bool has_unknown_changes = false; +}; + // Function pointer for a predicate that determines whether a given // profile is compilable. Alternatively we could derive subclasses // from ProfileFuncs and use a virtual method for this, but that seems @@ -286,11 +362,38 @@ public: const std::unordered_set& Lambdas() const { return lambdas; } const std::unordered_set& Events() const { return events; } - std::shared_ptr FuncProf(const ScriptFunc* f) { return func_profs[f]; } + const auto& FuncProfs() const { return func_profs; } - // This is only externally germane for LambdaExpr's. + // Profiles associated with LambdaExpr's and expressions appearing in + // attributes. std::shared_ptr ExprProf(const Expr* e) { return expr_profs[e]; } + // Returns true if the given type corresponds to a table that has a + // &default attribute that returns an aggregate value. + bool IsTableWithDefaultAggr(const Type* t); + + // Returns true if the given operation has non-zero side effects. + bool HasSideEffects(SideEffectsOp::AccessType access, const TypePtr& t) const; + + // Retrieves the side effects of the given operation, updating non_local_ids + // and aggrs with identifiers and aggregate types that are modified. + // + // A return value of true means the side effects are Unknown. If false, + // then there are side effects iff either (or both) of non_local_ids + // or aggrs are non-empty. + bool GetSideEffects(SideEffectsOp::AccessType access, const Type* t, IDSet& non_local_ids, TypeSet& aggrs) const; + + // Retrieves the side effects of calling the function corresponding to + // the NameExpr, updating non_local_ids and aggrs with identifiers and + // aggregate types that are modified. is_unknown is set to true if the + // call has Unknown side effects (which overrides the relevance of the + // updates to the sets). + // + // A return value of true means that side effects cannot yet be determined, + // due to dependencies on other side effects. This can happen when + // constructing a ProfileFuncs, but should not happen once its constructed. + bool GetCallSideEffects(const NameExpr* n, IDSet& non_local_ids, TypeSet& aggrs, bool& is_unknown); + // Returns the "representative" Type* for the hash associated with // the parameter (which might be the parameter itself). const Type* TypeRep(const Type* orig) { @@ -332,8 +435,56 @@ protected: void ComputeProfileHash(std::shared_ptr pf); // Analyze the expressions and lambdas appearing in a set of - // attributes. - void AnalyzeAttrs(const Attributes* Attrs); + // attributes, in the context of a given type. + void AnalyzeAttrs(const Attributes* attrs, const Type* t); + + // In the abstract, computes side-effects associated with operations other + // than explicit function calls. Currently, this means tables and records + // that can implicitly call functions that have side effects due to + // attributes such as &default. The machinery also applies to assessing + // the side effects of explicit function calls, which is done by + // (the two versions of) GetCallSideEffects(). + void ComputeSideEffects(); + + // True if the given expression for sure has no side effects, which is + // almost always the case. False if the expression *may* have side effects + // and requires further analysis. + bool DefinitelyHasNoSideEffects(const ExprPtr& e) const; + + // Records the side effects associated with the given attribute. + void SetSideEffects(const Attr* a, IDSet& non_local_ids, TypeSet& aggrs, bool is_unknown); + + // Returns the attributes associated with the given type *and its aliases*. + AttrVec AssociatedAttrs(const Type* t); + + // For a given set of attributes, assesses which ones are associated with + // the given type or its aliases and adds them to the given vector. + void FindAssociatedAttrs(const AttrSet& candidate_attrs, const Type* t, AttrVec& assoc_attrs); + + // Assesses the side effects associated with the given expression. Returns + // true if a complete assessment was possible, false if not because the + // results depend on resolving other potential side effects first. + bool AssessSideEffects(const ExprPtr& e, IDSet& non_local_ids, TypeSet& types, bool& is_unknown); + + // Same, but for the given profile. + bool AssessSideEffects(const ProfileFunc* pf, IDSet& non_local_ids, TypeSet& types, bool& is_unknown); + + // Same but for the particular case of a relevant access to an aggregate + // (which can be constructing a record; reading a table element; or + // modifying a table element). + bool AssessAggrEffects(SideEffectsOp::AccessType access, const Type* t, IDSet& non_local_ids, TypeSet& aggrs, + bool& is_unknown); + + // For a given set of side effects, determines whether the given aggregate + // access applies. If so, updates non_local_ids and aggrs and returns true + // if there are Unknown side effects; otherwise returns false. + bool AssessSideEffects(const SideEffectsOp* se, SideEffectsOp::AccessType access, const Type* t, + IDSet& non_local_ids, TypeSet& aggrs) const; + + // Returns nil if side effects are not available. That should never be + // the case after we've done our initial analysis, but is provided + // as a signal so that this method can also be used during that analysis. + std::shared_ptr GetCallSideEffects(const ScriptFunc* f); // Globals seen across the functions, other than those solely seen // as the function being called in a call. @@ -357,6 +508,11 @@ protected: // Maps a type to its representative (which might be itself). std::unordered_map type_to_rep; + // For a given type, tracks which other types are aliased to it. + // Alias occurs via operations that can propagate attributes, which + // are various forms of aggregate coercions. + std::unordered_map> type_aliases; + // Script functions that get called. std::unordered_set script_calls; @@ -369,35 +525,77 @@ protected: // Names of generated events. std::unordered_set events; - // Maps script functions to associated profiles. This isn't - // actually well-defined in the case of event handlers and hooks, - // which can have multiple bodies. However, the need for this - // is temporary (it's for skipping compilation of functions that - // appear in "when" clauses), and in that context it suffices. + // Maps script functions to associated profiles. This isn't actually + // well-defined in the case of event handlers and hooks, which can have + // multiple bodies. However, we only use this in the context of calls + // to regular functions, and for that it suffices. std::unordered_map> func_profs; - // Maps expressions to their profiles. This is only germane - // externally for LambdaExpr's, but internally it abets memory - // management. + // Map lambda names to their primary functions + std::unordered_map lambda_primaries; + + // Tracks side effects associated with script functions. If we decide in + // the future to associate richer side-effect information with BiFs then + // we could expand this to track Func*'s instead. + std::unordered_map> func_side_effects; + + // Maps expressions to their profiles. std::unordered_map> expr_profs; // These remaining member variables are only used internally, // not provided via accessors: + // Maps expression-valued attributes to a collection of types in which + // the attribute appears. Usually there's just one type, but there are + // some scripting constructs that can result in the same attribute being + // shared across multiple distinct (though compatible) types. + std::unordered_map> expr_attrs; + + // Tracks whether a given TableType has a &default that returns an + // aggregate. Expressions involving indexing tables with such types + // cannot be optimized out using CSE because each returned value is + // distinct. + std::unordered_map tbl_has_aggr_default; + + // For a given attribute, maps it to side effects associated with aggregate + // operations (table reads/writes). + std::unordered_map>> aggr_side_effects; + + // The same, but for record constructors. + std::unordered_map>> record_constr_with_side_effects; + + // The set of attributes that may have side effects but we haven't yet + // resolved if that's the case. Empty after we're done analyzing for + // side effects. + AttrSet candidates; + + // The current candidate we're analyzing. We track this to deal with + // the possibility of the candidate's side effects recursively referring + // to the candidate itself. + const Attr* curr_candidate; + + // The set of attributes that definitely have side effects. + AttrSet attrs_with_side_effects; + + // The full collection of operations with side effects. + std::vector> side_effects_ops; + + // Which function profiles we are currently analyzing. Used to detect + // recursion and prevent it from leading to non-termination of the analysis. + std::unordered_set> active_func_profiles; + // Maps types to their hashes. std::unordered_map type_hashes; // An inverse mapping, to a representative for each distinct hash. std::unordered_map type_hash_reps; - // For types with names, tracks the ones we've already hashed, - // so we can avoid work for distinct pointers that refer to the - // same underlying type. + // For types with names, tracks the ones we've already hashed, so we can + // avoid work for distinct pointers that refer to the same underlying type. std::unordered_map seen_type_names; - // Expressions that we've discovered that we need to further - // profile. These can arise for example due to lambdas or - // record attributes. + // Expressions that we've discovered that we need to further profile. + // These can arise for example due to lambdas or record attributes. std::vector pending_exprs; // Whether the hashes for extended records should cover their final, diff --git a/src/script_opt/Reduce.cc b/src/script_opt/Reduce.cc index 580f32806a..3a0fb8ba0d 100644 --- a/src/script_opt/Reduce.cc +++ b/src/script_opt/Reduce.cc @@ -11,12 +11,14 @@ #include "zeek/Stmt.h" #include "zeek/Var.h" #include "zeek/script_opt/ExprOptInfo.h" +#include "zeek/script_opt/FuncInfo.h" #include "zeek/script_opt/StmtOptInfo.h" #include "zeek/script_opt/TempVar.h" namespace zeek::detail { -Reducer::Reducer(const ScriptFunc* func, std::shared_ptr _pf) : pf(std::move(_pf)) { +Reducer::Reducer(const ScriptFunc* func, std::shared_ptr _pf, ProfileFuncs& _pfs) + : pf(std::move(_pf)), pfs(_pfs) { auto& ft = func->GetType(); // Track the parameters so we don't remap them. @@ -424,6 +426,41 @@ IDPtr Reducer::FindExprTmp(const Expr* rhs, const Expr* a, const std::shared_ptr } bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const { + // First check for whether e1 is already known to itself have side effects. + // If so, then it's never safe to reuse its associated identifier in lieu + // of e2. + std::optional& e1_se = e1->GetOptInfo()->SideEffects(); + if ( ! e1_se ) { + bool has_side_effects = false; + auto e1_t = e1->GetType(); + + if ( e1_t->Tag() == TYPE_OPAQUE || e1_t->Tag() == TYPE_ANY ) + // These have difficult-to-analyze semantics. + has_side_effects = true; + + else if ( e1->Tag() == EXPR_INDEX ) { + auto aggr = e1->GetOp1(); + auto aggr_t = aggr->GetType(); + + if ( pfs.HasSideEffects(SideEffectsOp::READ, aggr_t) ) + has_side_effects = true; + + else if ( aggr_t->Tag() == TYPE_TABLE && pfs.IsTableWithDefaultAggr(aggr_t.get()) ) + has_side_effects = true; + } + + else if ( e1->Tag() == EXPR_RECORD_CONSTRUCTOR || e1->Tag() == EXPR_RECORD_COERCE ) + has_side_effects = pfs.HasSideEffects(SideEffectsOp::CONSTRUCTION, e1->GetType()); + + e1_se = ExprSideEffects(has_side_effects); + } + + if ( e1_se->HasSideEffects() ) { + // We already know that e2 is structurally identical to e1. + e2->GetOptInfo()->SideEffects() = ExprSideEffects(true); + return false; + } + // Here are the considerations for expression validity. // // * None of the operands used in the given expression can @@ -437,11 +474,14 @@ bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const { // * Same goes to modifications of aggregates via "add" or "delete" // or "+=" append. // - // * No propagation of expressions that are based on aggregates - // across function calls. + // * Assessment of any record constructors or coercions, or + // table references or modifications, for possible invocation of + // associated handlers that have side effects. // - // * No propagation of expressions that are based on globals - // across calls. + // * Assessment of function calls for potential side effects. + // + // These latter two are guided by the global profile of the full set + // of script functions. // Tracks which ID's are germane for our analysis. std::vector ids; @@ -456,7 +496,7 @@ bool Reducer::ExprValid(const ID* id, const Expr* e1, const Expr* e2) const { if ( e1->Tag() == EXPR_NAME ) ids.push_back(e1->AsNameExpr()->Id()); - CSE_ValidityChecker vc(ids, e1, e2); + CSE_ValidityChecker vc(pfs, ids, e1, e2); reduction_root->Traverse(&vc); return vc.IsValid(); @@ -785,15 +825,12 @@ std::shared_ptr Reducer::FindTemporary(const ID* id) const { return tmp->second; } -CSE_ValidityChecker::CSE_ValidityChecker(const std::vector& _ids, const Expr* _start_e, const Expr* _end_e) - : ids(_ids) { +CSE_ValidityChecker::CSE_ValidityChecker(ProfileFuncs& _pfs, const std::vector& _ids, const Expr* _start_e, + const Expr* _end_e) + : pfs(_pfs), ids(_ids) { start_e = _start_e; end_e = _end_e; - for ( auto i : ids ) - if ( i->IsGlobal() || IsAggr(i->GetType()) ) - sensitive_to_calls = true; - // Track whether this is a record assignment, in which case // we're attuned to assignments to the same field for the // same type of record. @@ -811,7 +848,16 @@ CSE_ValidityChecker::CSE_ValidityChecker(const std::vector& _ids, con } TraversalCode CSE_ValidityChecker::PreStmt(const Stmt* s) { - if ( s->Tag() == STMT_ADD || s->Tag() == STMT_DELETE ) + auto t = s->Tag(); + + if ( t == STMT_WHEN ) { + // These are too hard to analyze - they result in lambda calls + // that can affect aggregates, etc. + is_valid = false; + return TC_ABORTALL; + } + + if ( t == STMT_ADD || t == STMT_DELETE ) in_aggr_mod_stmt = true; return TC_CONTINUE; @@ -831,7 +877,7 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { // Don't analyze the expression, as it's our starting // point and we don't want to conflate its properties - // with those of any intervening expression. + // with those of any intervening expressions. return TC_CONTINUE; } @@ -858,25 +904,26 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { auto lhs_ref = e->GetOp1()->AsRefExprPtr(); auto lhs = lhs_ref->GetOp1()->AsNameExpr(); - if ( CheckID(ids, lhs->Id(), false) ) { - is_valid = false; + if ( CheckID(lhs->Id(), false) ) return TC_ABORTALL; - } - // Note, we don't use CheckAggrMod() because this - // is a plain assignment. It might be changing a variable's - // binding to an aggregate, but it's not changing the - // aggregate itself. + // Note, we don't use CheckAggrMod() because this is a plain + // assignment. It might be changing a variable's binding to + // an aggregate ("aggr_var = new_aggr_val"), but we don't + // introduce temporaries that are simply aliases of existing + // variables (e.g., we don't have "::#8 = aggr_var"), + // and so there's no concern that the temporary could now be + // referring to the wrong aggregate. If instead we have + // "::#8 = aggr_var$foo", then a reassignment here + // to "aggr_var" will already be caught by CheckID(). } break; case EXPR_INDEX_ASSIGN: { auto lhs_aggr = e->GetOp1(); auto lhs_aggr_id = lhs_aggr->AsNameExpr()->Id(); - if ( CheckID(ids, lhs_aggr_id, true) || CheckAggrMod(ids, e) ) { - is_valid = false; + if ( CheckID(lhs_aggr_id, true) || CheckTableMod(lhs_aggr->GetType()) ) return TC_ABORTALL; - } } break; case EXPR_FIELD_LHS_ASSIGN: { @@ -884,17 +931,9 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { auto lhs_aggr_id = lhs->AsNameExpr()->Id(); auto lhs_field = e->AsFieldLHSAssignExpr()->Field(); - if ( lhs_field == field && same_type(lhs_aggr_id->GetType(), field_type) ) { - // Potential assignment to the same field as for - // our expression of interest. Even if the - // identifier involved is not one we have our eye - // on, due to aggregate aliasing this could be - // altering the value of our expression, so bail. - is_valid = false; + if ( CheckID(lhs_aggr_id, true) ) return TC_ABORTALL; - } - - if ( CheckID(ids, lhs_aggr_id, true) || CheckAggrMod(ids, e) ) { + if ( lhs_field == field && same_type(lhs_aggr_id->GetType(), field_type) ) { is_valid = false; return TC_ABORTALL; } @@ -903,17 +942,13 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { case EXPR_APPEND_TO: // This doesn't directly change any identifiers, but does // alter an aggregate. - if ( CheckAggrMod(ids, e) ) { - is_valid = false; + if ( CheckAggrMod(e->GetType()) ) return TC_ABORTALL; - } break; case EXPR_CALL: - if ( sensitive_to_calls ) { - is_valid = false; + if ( CheckCall(e->AsCallExpr()) ) return TC_ABORTALL; - } break; case EXPR_TABLE_CONSTRUCTOR: @@ -922,49 +957,35 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { // so we don't want to traverse them. return TC_ABORTSTMT; + case EXPR_RECORD_COERCE: case EXPR_RECORD_CONSTRUCTOR: - // If these have initializations done at construction - // time, those can include function calls. - if ( sensitive_to_calls ) { - auto& et = e->GetType(); - if ( et->Tag() == TYPE_RECORD && ! et->AsRecordType()->IdempotentCreation() ) { - is_valid = false; - return TC_ABORTALL; - } - } + // Note, record coercion behaves like constructors in terms of + // potentially executing &default functions. In either case, + // the type of the expression reflects the type we want to analyze + // for side effects. + if ( CheckRecordConstructor(e->GetType()) ) + return TC_ABORTALL; break; case EXPR_INDEX: - case EXPR_FIELD: - // We treat these together because they both have - // to be checked when inside an "add" or "delete" - // statement. + case EXPR_FIELD: { + // We treat these together because they both have to be checked + // when inside an "add" or "delete" statement. + auto aggr = e->GetOp1(); + auto aggr_t = aggr->GetType(); + if ( in_aggr_mod_stmt ) { - auto aggr = e->GetOp1(); auto aggr_id = aggr->AsNameExpr()->Id(); - if ( CheckID(ids, aggr_id, true) ) { - is_valid = false; + if ( CheckID(aggr_id, true) || CheckAggrMod(aggr_t) ) return TC_ABORTALL; - } } - if ( t == EXPR_INDEX && sensitive_to_calls ) { - // Unfortunately in isolation we can't - // statically determine whether this table - // has a &default associated with it. In - // principle we could track all instances - // of the table type seen (across the - // entire set of scripts), and note whether - // any of those include an expression, but - // that's a lot of work for what might be - // minimal gain. - - is_valid = false; - return TC_ABORTALL; + else if ( t == EXPR_INDEX && aggr_t->Tag() == TYPE_TABLE ) { + if ( CheckTableRef(aggr_t) ) + return TC_ABORTALL; } - - break; + } break; default: break; } @@ -972,33 +993,92 @@ TraversalCode CSE_ValidityChecker::PreExpr(const Expr* e) { return TC_CONTINUE; } -bool CSE_ValidityChecker::CheckID(const std::vector& ids, const ID* id, bool ignore_orig) const { - // Only check type info for aggregates. - auto id_t = IsAggr(id->GetType()) ? id->GetType() : nullptr; - +bool CSE_ValidityChecker::CheckID(const ID* id, bool ignore_orig) { for ( auto i : ids ) { if ( ignore_orig && i == ids.front() ) continue; if ( id == i ) - return true; // reassignment - - if ( id_t && same_type(id_t, i->GetType()) ) - // Same-type aggregate. - return true; + return Invalid(); // reassignment } return false; } -bool CSE_ValidityChecker::CheckAggrMod(const std::vector& ids, const Expr* e) const { - const auto& e_i_t = e->GetType(); - if ( IsAggr(e_i_t) ) { - // This assignment sets an aggregate value. - // Look for type matches. - for ( auto i : ids ) - if ( same_type(e_i_t, i->GetType()) ) - return true; +bool CSE_ValidityChecker::CheckAggrMod(const TypePtr& t) { + if ( ! IsAggr(t) ) + return false; + + for ( auto i : ids ) + if ( same_type(t, i->GetType()) ) + return Invalid(); + + return false; +} + +bool CSE_ValidityChecker::CheckRecordConstructor(const TypePtr& t) { + if ( t->Tag() != TYPE_RECORD ) + return false; + + return CheckSideEffects(SideEffectsOp::CONSTRUCTION, t); +} + +bool CSE_ValidityChecker::CheckTableMod(const TypePtr& t) { + if ( CheckAggrMod(t) ) + return true; + + if ( t->Tag() != TYPE_TABLE ) + return false; + + return CheckSideEffects(SideEffectsOp::WRITE, t); +} + +bool CSE_ValidityChecker::CheckTableRef(const TypePtr& t) { return CheckSideEffects(SideEffectsOp::READ, t); } + +bool CSE_ValidityChecker::CheckCall(const CallExpr* c) { + auto func = c->Func(); + std::string desc; + if ( func->Tag() != EXPR_NAME ) + // Can't analyze indirect calls. + return Invalid(); + + IDSet non_local_ids; + TypeSet aggrs; + bool is_unknown = false; + + auto resolved = pfs.GetCallSideEffects(func->AsNameExpr(), non_local_ids, aggrs, is_unknown); + ASSERT(resolved); + + if ( is_unknown || CheckSideEffects(non_local_ids, aggrs) ) + return Invalid(); + + return false; +} + +bool CSE_ValidityChecker::CheckSideEffects(SideEffectsOp::AccessType access, const TypePtr& t) { + IDSet non_local_ids; + TypeSet aggrs; + + if ( pfs.GetSideEffects(access, t.get(), non_local_ids, aggrs) ) + return Invalid(); + + return CheckSideEffects(non_local_ids, aggrs); +} + +bool CSE_ValidityChecker::CheckSideEffects(const IDSet& non_local_ids, const TypeSet& aggrs) { + if ( non_local_ids.empty() && aggrs.empty() ) + // This is far and away the most common case. + return false; + + for ( auto i : ids ) { + for ( auto nli : non_local_ids ) + if ( nli == i ) + return Invalid(); + + auto i_t = i->GetType(); + for ( auto a : aggrs ) + if ( same_type(a, i_t.get()) ) + return Invalid(); } return false; diff --git a/src/script_opt/Reduce.h b/src/script_opt/Reduce.h index 68532130e5..b4f4cb4743 100644 --- a/src/script_opt/Reduce.h +++ b/src/script_opt/Reduce.h @@ -16,7 +16,7 @@ class TempVar; class Reducer { public: - Reducer(const ScriptFunc* func, std::shared_ptr pf); + Reducer(const ScriptFunc* func, std::shared_ptr pf, ProfileFuncs& pfs); StmtPtr Reduce(StmtPtr s); @@ -131,24 +131,22 @@ public: replaced_stmts.clear(); } - // Given the LHS and RHS of an assignment, returns true - // if the RHS is a common subexpression (meaning that the - // current assignment statement should be deleted). In - // that case, has the side effect of associating an alias - // for the LHS with the temporary variable that holds the - // equivalent RHS; or if the LHS is a local that has no other - // assignments, and the same for the RHS. + // Given the LHS and RHS of an assignment, returns true if the RHS is + // a common subexpression (meaning that the current assignment statement + // should be deleted). In that case, has the side effect of associating + // an alias for the LHS with the temporary variable that holds the + // equivalent RHS; or if the LHS is a local that has no other assignments, + // and the same for the RHS. // - // Assumes reduction (including alias propagation) has - // already been applied. + // Assumes reduction (including alias propagation) has already been applied. + bool IsCSE(const AssignExpr* a, const NameExpr* lhs, const Expr* rhs); // Returns a constant representing folding of the given expression // (which must have constant operands). ConstExprPtr Fold(ExprPtr e); - // Notes that the given expression has been folded to the - // given constant. + // Notes that the given expression has been folded to the given constant. void FoldedTo(ExprPtr orig, ConstExprPtr c); // Given an lhs=rhs statement followed by succ_stmt, returns @@ -237,6 +235,9 @@ protected: // Profile associated with the function. std::shared_ptr pf; + // Profile across all script functions - used for optimization decisions. + ProfileFuncs& pfs; + // Tracks the temporary variables created during the reduction/ // optimization process. std::vector> temps; @@ -324,7 +325,7 @@ protected: class CSE_ValidityChecker : public TraversalCallback { public: - CSE_ValidityChecker(const std::vector& ids, const Expr* start_e, const Expr* end_e); + CSE_ValidityChecker(ProfileFuncs& pfs, const std::vector& ids, const Expr* start_e, const Expr* end_e); TraversalCode PreStmt(const Stmt*) override; TraversalCode PostStmt(const Stmt*) override; @@ -342,21 +343,47 @@ public: protected: // Returns true if an assignment involving the given identifier on - // the LHS is in conflict with the given list of identifiers. - bool CheckID(const std::vector& ids, const ID* id, bool ignore_orig) const; + // the LHS is in conflict with the identifiers we're tracking. + bool CheckID(const ID* id, bool ignore_orig); - // Returns true if the assignment given by 'e' modifies an aggregate - // with the same type as that of one of the identifiers. - bool CheckAggrMod(const std::vector& ids, const Expr* e) const; + // Returns true if a modification to an aggregate of the given type + // potentially aliases with one of the identifiers we're tracking. + bool CheckAggrMod(const TypePtr& t); + + // Returns true if a record constructor/coercion of the given type has + // side effects and invalides the CSE opportunity. + bool CheckRecordConstructor(const TypePtr& t); + + // The same for modifications to tables. + bool CheckTableMod(const TypePtr& t); + + // The same for accessing (reading) tables. + bool CheckTableRef(const TypePtr& t); + + // The same for the given function call. + bool CheckCall(const CallExpr* c); + + // True if the given form of access to the given type has side effects. + bool CheckSideEffects(SideEffectsOp::AccessType access, const TypePtr& t); + + // True if side effects to the given identifiers and aggregates invalidate + // the CSE opportunity. + bool CheckSideEffects(const IDSet& non_local_ids, const TypeSet& aggrs); + + // Helper function that marks the CSE opportunity as invalid and returns + // "true" (used by various methods to signal invalidation). + bool Invalid() { + is_valid = false; + return true; + } + + // Profile across all script functions. + ProfileFuncs& pfs; // The list of identifiers for which an assignment to one of them // renders the CSE unsafe. const std::vector& ids; - // Whether the list of identifiers includes some that we should - // consider potentially altered by a function call. - bool sensitive_to_calls = false; - // Where in the AST to start our analysis. This is the initial // assignment expression. const Expr* start_e; @@ -379,8 +406,9 @@ protected: bool have_start_e = false; bool have_end_e = false; - // Whether analyzed expressions occur in the context of - // a statement that modifies an aggregate ("add" or "delete"). + // Whether analyzed expressions occur in the context of a statement + // that modifies an aggregate ("add" or "delete"), which changes the + // interpretation of the expressions. bool in_aggr_mod_stmt = false; }; diff --git a/src/script_opt/ScriptOpt.cc b/src/script_opt/ScriptOpt.cc index 35de232ce1..1ca1199528 100644 --- a/src/script_opt/ScriptOpt.cc +++ b/src/script_opt/ScriptOpt.cc @@ -147,7 +147,8 @@ static bool optimize_AST(ScriptFunc* f, std::shared_ptr& pf, std::s return true; } -static void optimize_func(ScriptFunc* f, std::shared_ptr pf, ScopePtr scope, StmtPtr& body) { +static void optimize_func(ScriptFunc* f, std::shared_ptr pf, ProfileFuncs& pfs, ScopePtr scope, + StmtPtr& body) { if ( reporter->Errors() > 0 ) return; @@ -167,7 +168,7 @@ static void optimize_func(ScriptFunc* f, std::shared_ptr pf, ScopeP push_existing_scope(scope); - auto rc = std::make_shared(f, pf); + auto rc = std::make_shared(f, pf, pfs); auto new_body = rc->Reduce(body); if ( reporter->Errors() > 0 ) { @@ -230,7 +231,7 @@ static void optimize_func(ScriptFunc* f, std::shared_ptr pf, ScopeP f->SetFrameSize(new_frame_size); if ( analysis_options.gen_ZAM_code ) { - ZAMCompiler ZAM(f, pf, scope, new_body, ud, rc); + ZAMCompiler ZAM(f, pfs, pf, scope, new_body, ud, rc); new_body = ZAM.CompileBody(); @@ -413,16 +414,18 @@ static void use_CPP() { reporter->FatalError("no C++ functions found to use"); } -static void generate_CPP(std::unique_ptr& pfs) { +static void generate_CPP() { const auto gen_name = CPP_dir + "CPP-gen.cc"; const bool standalone = analysis_options.gen_standalone_CPP; const bool report = analysis_options.report_uncompilable; + auto pfs = std::make_unique(funcs, is_CPP_compilable, false); + CPPCompile cpp(funcs, *pfs, gen_name, standalone, report); } -static void analyze_scripts_for_ZAM(std::unique_ptr& pfs) { +static void analyze_scripts_for_ZAM() { if ( analysis_options.usage_issues > 0 && analysis_options.optimize_AST ) { fprintf(stderr, "warning: \"-O optimize-AST\" option is incompatible with -u option, " @@ -430,15 +433,7 @@ static void analyze_scripts_for_ZAM(std::unique_ptr& pfs) { analysis_options.optimize_AST = false; } - // Re-profile the functions, now without worrying about compatibility - // with compilation to C++. - - // The first profiling pass earlier may have marked some of the - // functions as to-skip, so clear those markings. - for ( auto& f : funcs ) - f.SetSkip(false); - - pfs = std::make_unique(funcs, nullptr, true); + auto pfs = std::make_unique(funcs, nullptr, true); bool report_recursive = analysis_options.report_recursive; std::unique_ptr inl; @@ -492,7 +487,7 @@ static void analyze_scripts_for_ZAM(std::unique_ptr& pfs) { } auto new_body = f.Body(); - optimize_func(func, f.ProfilePtr(), f.Scope(), new_body); + optimize_func(func, f.ProfilePtr(), *pfs, f.Scope(), new_body); f.SetBody(new_body); if ( is_lambda ) @@ -566,10 +561,6 @@ void analyze_scripts(bool no_unused_warnings) { if ( ! have_one_to_do ) reporter->FatalError("no matching functions/files for C++ compilation"); - // Now that everything's parsed and BiF's have been initialized, - // profile the functions. - auto pfs = std::make_unique(funcs, is_CPP_compilable, false); - if ( CPP_init_hook ) { (*CPP_init_hook)(); if ( compiled_scripts.empty() ) @@ -591,13 +582,13 @@ void analyze_scripts(bool no_unused_warnings) { if ( analysis_options.gen_ZAM ) reporter->FatalError("-O ZAM and -O gen-C++ conflict"); - generate_CPP(pfs); + generate_CPP(); exit(0); } // At this point we're done with C++ considerations, so instead // are compiling to ZAM. - analyze_scripts_for_ZAM(pfs); + analyze_scripts_for_ZAM(); if ( reporter->Errors() > 0 ) reporter->FatalError("Optimized script execution aborted due to errors"); diff --git a/src/script_opt/ScriptOpt.h b/src/script_opt/ScriptOpt.h index 3ca5636492..715f7d9dc4 100644 --- a/src/script_opt/ScriptOpt.h +++ b/src/script_opt/ScriptOpt.h @@ -18,6 +18,8 @@ struct Options; namespace zeek::detail { +using TypeSet = std::unordered_set; + // Flags controlling what sorts of analysis to do. struct AnalyOpt { diff --git a/src/script_opt/Stmt.cc b/src/script_opt/Stmt.cc index 73f14649f9..7524eb4549 100644 --- a/src/script_opt/Stmt.cc +++ b/src/script_opt/Stmt.cc @@ -273,6 +273,10 @@ bool IfStmt::NoFlowAfter(bool ignore_break) const { return false; } +bool IfStmt::CouldReturn(bool ignore_break) const { + return (s1 && s1->CouldReturn(ignore_break)) || (s2 && s2->CouldReturn(ignore_break)); +} + IntrusivePtr Case::Duplicate() { if ( expr_cases ) { auto new_exprs = expr_cases->Duplicate()->AsListExprPtr(); @@ -404,6 +408,14 @@ bool SwitchStmt::NoFlowAfter(bool ignore_break) const { return default_seen_with_no_flow_after; } +bool SwitchStmt::CouldReturn(bool ignore_break) const { + for ( const auto& c : *Cases() ) + if ( c->Body()->CouldReturn(true) ) + return true; + + return false; +} + bool AddDelStmt::IsReduced(Reducer* c) const { return e->HasReducedOps(c); } StmtPtr AddDelStmt::DoReduce(Reducer* c) { @@ -499,6 +511,8 @@ StmtPtr WhileStmt::DoReduce(Reducer* c) { return ThisPtr(); } +bool WhileStmt::CouldReturn(bool ignore_break) const { return body->CouldReturn(false); } + StmtPtr ForStmt::Duplicate() { auto expr_copy = e->Duplicate(); @@ -568,6 +582,8 @@ StmtPtr ForStmt::DoReduce(Reducer* c) { return ThisPtr(); } +bool ForStmt::CouldReturn(bool ignore_break) const { return body->CouldReturn(false); } + StmtPtr ReturnStmt::Duplicate() { return SetSucc(new ReturnStmt(e ? e->Duplicate() : nullptr, true)); } ReturnStmt::ReturnStmt(ExprPtr arg_e, bool ignored) : ExprStmt(STMT_RETURN, std::move(arg_e)) {} @@ -784,6 +800,14 @@ bool StmtList::NoFlowAfter(bool ignore_break) const { return false; } +bool StmtList::CouldReturn(bool ignore_break) const { + for ( auto& s : stmts ) + if ( s->CouldReturn(ignore_break) ) + return true; + + return false; +} + StmtPtr InitStmt::Duplicate() { // Need to duplicate the initializer list since later reductions // can modify it in place. @@ -951,10 +975,13 @@ TraversalCode CatchReturnStmt::Traverse(TraversalCallback* cb) const { TraversalCode tc = cb->PreStmt(this); HANDLE_TC_STMT_PRE(tc); - block->Traverse(cb); + tc = block->Traverse(cb); + HANDLE_TC_STMT_PRE(tc); - if ( ret_var ) - ret_var->Traverse(cb); + if ( ret_var ) { + tc = ret_var->Traverse(cb); + HANDLE_TC_STMT_PRE(tc); + } tc = cb->PostStmt(this); HANDLE_TC_STMT_POST(tc); diff --git a/src/script_opt/UsageAnalyzer.h b/src/script_opt/UsageAnalyzer.h index 2c74595a47..d02e112c52 100644 --- a/src/script_opt/UsageAnalyzer.h +++ b/src/script_opt/UsageAnalyzer.h @@ -16,8 +16,6 @@ public: 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 @@ -67,10 +65,10 @@ private: // 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; + IDSet analyzed_IDs; // All of the types we've analyzed to date. - std::unordered_set analyzed_types; + TypeSet analyzed_types; }; // Marks a given identifier as referring to a script-level event (one diff --git a/src/script_opt/ZAM/Compile.h b/src/script_opt/ZAM/Compile.h index 7d3c29bfbd..e25ac57fcc 100644 --- a/src/script_opt/ZAM/Compile.h +++ b/src/script_opt/ZAM/Compile.h @@ -5,6 +5,7 @@ #pragma once #include "zeek/Event.h" +#include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/UseDefs.h" #include "zeek/script_opt/ZAM/ZBody.h" @@ -23,8 +24,6 @@ class Stmt; class SwitchStmt; class CatchReturnStmt; -class ProfileFunc; - using InstLabel = ZInstI*; // Class representing a single compiled statement. (This is different from, @@ -53,7 +52,7 @@ public: class ZAMCompiler { public: - ZAMCompiler(ScriptFunc* f, std::shared_ptr pf, ScopePtr scope, StmtPtr body, + ZAMCompiler(ScriptFunc* f, ProfileFuncs& pfs, std::shared_ptr pf, ScopePtr scope, StmtPtr body, std::shared_ptr ud, std::shared_ptr rd); ~ZAMCompiler(); @@ -503,6 +502,7 @@ private: std::vector retvars; ScriptFunc* func; + ProfileFuncs& pfs; std::shared_ptr pf; ScopePtr scope; StmtPtr body; diff --git a/src/script_opt/ZAM/Driver.cc b/src/script_opt/ZAM/Driver.cc index 390ad15788..be6e165907 100644 --- a/src/script_opt/ZAM/Driver.cc +++ b/src/script_opt/ZAM/Driver.cc @@ -8,14 +8,14 @@ #include "zeek/Reporter.h" #include "zeek/Scope.h" #include "zeek/module_util.h" -#include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/ScriptOpt.h" #include "zeek/script_opt/ZAM/Compile.h" namespace zeek::detail { -ZAMCompiler::ZAMCompiler(ScriptFunc* f, std::shared_ptr _pf, ScopePtr _scope, StmtPtr _body, - std::shared_ptr _ud, std::shared_ptr _rd) { +ZAMCompiler::ZAMCompiler(ScriptFunc* f, ProfileFuncs& _pfs, std::shared_ptr _pf, ScopePtr _scope, + StmtPtr _body, std::shared_ptr _ud, std::shared_ptr _rd) + : pfs(_pfs) { func = f; pf = std::move(_pf); scope = std::move(_scope); diff --git a/src/script_opt/ZAM/Expr.cc b/src/script_opt/ZAM/Expr.cc index ea5af97939..4bcde31aea 100644 --- a/src/script_opt/ZAM/Expr.cc +++ b/src/script_opt/ZAM/Expr.cc @@ -4,7 +4,6 @@ #include "zeek/Desc.h" #include "zeek/Reporter.h" -#include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/ZAM/Compile.h" namespace zeek::detail { @@ -667,11 +666,10 @@ const ZAMStmt ZAMCompiler::CompileIndex(const NameExpr* n1, int n2_slot, const T z = ZInstI(zop, Frame1Slot(n1, zop), n2_slot, c3); } - // See the discussion in CSE_ValidityChecker::PreExpr - // regarding always needing to treat this as potentially - // modifying globals. - z.aux = new ZInstAux(0); - z.aux->can_change_non_locals = true; + if ( pfs.HasSideEffects(SideEffectsOp::READ, n2t) ) { + z.aux = new ZInstAux(0); + z.aux->can_change_non_locals = true; + } return AddInst(z); } @@ -853,6 +851,9 @@ const ZAMStmt ZAMCompiler::AssignTableElem(const Expr* e) { z.aux = InternalBuildVals(op2); z.t = op3->GetType(); + if ( pfs.HasSideEffects(SideEffectsOp::WRITE, op1->GetType()) ) + z.aux->can_change_non_locals = true; + return AddInst(z); } @@ -953,8 +954,12 @@ const ZAMStmt ZAMCompiler::DoCall(const CallExpr* c, const NameExpr* n) { op = OP_WHENCALLN_V; } - else if ( indirect ) - op = n ? OP_INDCALLN_VV : OP_INDCALLN_V; + else if ( indirect ) { + if ( func_id->IsGlobal() ) + op = n ? OP_INDCALLN_V : OP_INDCALLN_X; + else + op = n ? OP_LOCAL_INDCALLN_VV : OP_LOCAL_INDCALLN_V; + } else op = n ? OP_CALLN_V : OP_CALLN_X; @@ -968,11 +973,14 @@ const ZAMStmt ZAMCompiler::DoCall(const CallExpr* c, const NameExpr* n) { auto n_slot = Frame1Slot(n, OP1_WRITE); if ( indirect ) { - if ( func_id->IsGlobal() ) - z = ZInstI(op, n_slot, -1); - else + if ( func_id->IsGlobal() ) { + z = ZInstI(op, n_slot); + z.op_type = OP_V; + } + else { z = ZInstI(op, n_slot, FrameSlot(func)); - z.op_type = OP_VV; + z.op_type = OP_VV; + } } else { @@ -981,11 +989,8 @@ const ZAMStmt ZAMCompiler::DoCall(const CallExpr* c, const NameExpr* n) { } } else { - if ( indirect ) { - if ( func_id->IsGlobal() ) - z = ZInstI(op, -1); - else - z = ZInstI(op, FrameSlot(func)); + if ( indirect && ! func_id->IsGlobal() ) { + z = ZInstI(op, FrameSlot(func)); z.op_type = OP_V; } else { @@ -1000,7 +1005,20 @@ const ZAMStmt ZAMCompiler::DoCall(const CallExpr* c, const NameExpr* n) { if ( ! z.aux ) z.aux = new ZInstAux(0); - z.aux->can_change_non_locals = true; + if ( indirect ) + z.aux->can_change_non_locals = true; + + else { + IDSet non_local_ids; + TypeSet aggrs; + bool is_unknown = false; + + auto resolved = pfs.GetCallSideEffects(func, non_local_ids, aggrs, is_unknown); + ASSERT(resolved); + + if ( is_unknown || ! non_local_ids.empty() || ! aggrs.empty() ) + z.aux->can_change_non_locals = true; + } z.call_expr = {NewRef{}, const_cast(c)}; @@ -1085,7 +1103,7 @@ const ZAMStmt ZAMCompiler::ConstructRecord(const NameExpr* n, const Expr* e) { z.t = e->GetType(); - if ( ! rc->GetType()->IdempotentCreation() ) + if ( pfs.HasSideEffects(SideEffectsOp::CONSTRUCTION, z.t) ) z.aux->can_change_non_locals = true; return AddInst(z); @@ -1184,6 +1202,9 @@ const ZAMStmt ZAMCompiler::RecordCoerce(const NameExpr* n, const Expr* e) { // Mark the integer entries in z.aux as not being frame slots as usual. z.aux->slots = nullptr; + if ( pfs.HasSideEffects(SideEffectsOp::CONSTRUCTION, e->GetType()) ) + z.aux->can_change_non_locals = true; + return AddInst(z); } diff --git a/src/script_opt/ZAM/Ops.in b/src/script_opt/ZAM/Ops.in index 3ffbeb6462..dd092689bf 100644 --- a/src/script_opt/ZAM/Ops.in +++ b/src/script_opt/ZAM/Ops.in @@ -183,8 +183,11 @@ # version for assigning to a frame variable # # indirect-call the operation represents an indirect call (through -# a variable, rather than directly). Only meaningful -# if num-call-args is also specified. +# a global variable, rather than directly). Only +# meaningful if num-call-args is also specified. +# +# indirect-local-call same, but via a local variable rather than +# global # # method-post C++ code to add to the end of the method that # dynamically generates ZAM code @@ -1587,22 +1590,36 @@ side-effects OP_CALLN_X OP_X assign-val v num-call-args n -# Same, but for indirect calls via a local variable. +# Same, but for indirect calls via a global variable. internal-op IndCallN -op1-read -type V +type X side-effects indirect-call num-call-args n # Same with a return value. internal-assignment-op IndCallN -type VV -side-effects OP_INDCALLN_V OP_V +type V +side-effects OP_INDCALLN_X OP_X assign-val v indirect-call num-call-args n +# And versions with a local variable rather than a global. +internal-op Local-IndCallN +op1-read +type V +side-effects +indirect-local-call +num-call-args n + +internal-assignment-op Local-IndCallN +type VV +side-effects OP_LOCAL_INDCALLN_V OP_V +assign-val v +indirect-local-call +num-call-args n + # A call made in a "when" context. These always have assignment targets. # To keep things simple, we just use one generic flavor (for N arguments, # doing a less-streamlined-but-simpler Val-based assignment). diff --git a/src/script_opt/ZAM/Stmt.cc b/src/script_opt/ZAM/Stmt.cc index c5540bdc6b..d38b99f95f 100644 --- a/src/script_opt/ZAM/Stmt.cc +++ b/src/script_opt/ZAM/Stmt.cc @@ -5,7 +5,6 @@ #include "zeek/IPAddr.h" #include "zeek/Reporter.h" #include "zeek/ZeekString.h" -#include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/ZAM/Compile.h" namespace zeek::detail { diff --git a/src/script_opt/ZAM/Vars.cc b/src/script_opt/ZAM/Vars.cc index bab4ee0761..3994c12f2d 100644 --- a/src/script_opt/ZAM/Vars.cc +++ b/src/script_opt/ZAM/Vars.cc @@ -4,7 +4,6 @@ #include "zeek/Desc.h" #include "zeek/Reporter.h" -#include "zeek/script_opt/ProfileFunc.h" #include "zeek/script_opt/Reduce.h" #include "zeek/script_opt/ZAM/Compile.h" diff --git a/src/script_opt/ZAM/maint/BiFs.list b/src/script_opt/ZAM/maint/BiFs.list new file mode 100644 index 0000000000..d07fcb33b5 --- /dev/null +++ b/src/script_opt/ZAM/maint/BiFs.list @@ -0,0 +1,543 @@ +Analyzer::__disable_all_analyzers +Analyzer::__disable_analyzer +Analyzer::__enable_analyzer +Analyzer::__has_tag +Analyzer::__name +Analyzer::__register_for_port +Analyzer::__schedule_analyzer +Analyzer::__tag +Broker::__append +Broker::__auto_publish +Broker::__auto_unpublish +Broker::__clear +Broker::__close +Broker::__create_clone +Broker::__create_master +Broker::__data +Broker::__data_type +Broker::__decrement +Broker::__erase +Broker::__exists +Broker::__flush_logs +Broker::__forward +Broker::__get +Broker::__get_index_from_value +Broker::__increment +Broker::__insert_into_set +Broker::__insert_into_table +Broker::__is_closed +Broker::__keys +Broker::__listen +Broker::__node_id +Broker::__opaque_clone_through_serialization +Broker::__peer +Broker::__peer_no_retry +Broker::__peers +Broker::__pop +Broker::__publish_id +Broker::__push +Broker::__put +Broker::__put_unique +Broker::__record_assign +Broker::__record_create +Broker::__record_iterator +Broker::__record_iterator_last +Broker::__record_iterator_next +Broker::__record_iterator_value +Broker::__record_lookup +Broker::__record_size +Broker::__remove_from +Broker::__set_clear +Broker::__set_contains +Broker::__set_create +Broker::__set_insert +Broker::__set_iterator +Broker::__set_iterator_last +Broker::__set_iterator_next +Broker::__set_iterator_value +Broker::__set_metrics_export_endpoint_name +Broker::__set_metrics_export_interval +Broker::__set_metrics_export_prefixes +Broker::__set_metrics_export_topic +Broker::__set_metrics_import_topics +Broker::__set_remove +Broker::__set_size +Broker::__store_name +Broker::__subscribe +Broker::__table_clear +Broker::__table_contains +Broker::__table_create +Broker::__table_insert +Broker::__table_iterator +Broker::__table_iterator_last +Broker::__table_iterator_next +Broker::__table_iterator_value +Broker::__table_lookup +Broker::__table_remove +Broker::__table_size +Broker::__unpeer +Broker::__unsubscribe +Broker::__vector_clear +Broker::__vector_create +Broker::__vector_insert +Broker::__vector_iterator +Broker::__vector_iterator_last +Broker::__vector_iterator_next +Broker::__vector_iterator_value +Broker::__vector_lookup +Broker::__vector_remove +Broker::__vector_replace +Broker::__vector_size +Broker::make_event +Broker::publish +Cluster::publish_hrw +Cluster::publish_rr +FileExtract::__set_limit +Files::__add_analyzer +Files::__analyzer_enabled +Files::__analyzer_name +Files::__disable_analyzer +Files::__disable_reassembly +Files::__enable_analyzer +Files::__enable_reassembly +Files::__file_exists +Files::__lookup_file +Files::__remove_analyzer +Files::__set_reassembly_buffer +Files::__set_timeout_interval +Files::__stop +Input::__create_analysis_stream +Input::__create_event_stream +Input::__create_table_stream +Input::__force_update +Input::__remove_stream +Log::__add_filter +Log::__create_stream +Log::__disable_stream +Log::__enable_stream +Log::__flush +Log::__remove_filter +Log::__remove_stream +Log::__set_buf +Log::__write +Option::any_set_to_any_vec +Option::set +Option::set_change_handler +PacketAnalyzer::GTPV1::remove_gtpv1_connection +PacketAnalyzer::TEREDO::remove_teredo_connection +PacketAnalyzer::__disable_analyzer +PacketAnalyzer::__enable_analyzer +PacketAnalyzer::__set_ignore_checksums_nets +PacketAnalyzer::register_packet_analyzer +PacketAnalyzer::register_protocol_detection +PacketAnalyzer::try_register_packet_analyzer_by_name +Pcap::error +Pcap::findalldevs +Pcap::get_filter_state +Pcap::get_filter_state_string +Pcap::install_pcap_filter +Pcap::precompile_pcap_filter +Reporter::conn_weird +Reporter::error +Reporter::fatal +Reporter::fatal_error_with_core +Reporter::file_weird +Reporter::flow_weird +Reporter::get_weird_sampling_duration +Reporter::get_weird_sampling_global_list +Reporter::get_weird_sampling_rate +Reporter::get_weird_sampling_threshold +Reporter::get_weird_sampling_whitelist +Reporter::info +Reporter::net_weird +Reporter::set_weird_sampling_duration +Reporter::set_weird_sampling_global_list +Reporter::set_weird_sampling_rate +Reporter::set_weird_sampling_threshold +Reporter::set_weird_sampling_whitelist +Reporter::warning +Spicy::__resource_usage +Spicy::__toggle_analyzer +Supervisor::__create +Supervisor::__destroy +Supervisor::__init_cluster +Supervisor::__is_supervised +Supervisor::__is_supervisor +Supervisor::__node +Supervisor::__restart +Supervisor::__status +Supervisor::__stem_pid +Telemetry::__collect_histogram_metrics +Telemetry::__collect_metrics +Telemetry::__dbl_counter_family +Telemetry::__dbl_counter_inc +Telemetry::__dbl_counter_metric_get_or_add +Telemetry::__dbl_counter_value +Telemetry::__dbl_gauge_dec +Telemetry::__dbl_gauge_family +Telemetry::__dbl_gauge_inc +Telemetry::__dbl_gauge_metric_get_or_add +Telemetry::__dbl_gauge_value +Telemetry::__dbl_histogram_family +Telemetry::__dbl_histogram_metric_get_or_add +Telemetry::__dbl_histogram_observe +Telemetry::__dbl_histogram_sum +Telemetry::__int_counter_family +Telemetry::__int_counter_inc +Telemetry::__int_counter_metric_get_or_add +Telemetry::__int_counter_value +Telemetry::__int_gauge_dec +Telemetry::__int_gauge_family +Telemetry::__int_gauge_inc +Telemetry::__int_gauge_metric_get_or_add +Telemetry::__int_gauge_value +Telemetry::__int_histogram_family +Telemetry::__int_histogram_metric_get_or_add +Telemetry::__int_histogram_observe +Telemetry::__int_histogram_sum +__init_primary_bifs +__init_secondary_bifs +active_file +addr_to_counts +addr_to_ptr_name +addr_to_subnet +all_set +anonymize_addr +any_set +backtrace +bare_mode +bloomfilter_add +bloomfilter_basic_init +bloomfilter_basic_init2 +bloomfilter_clear +bloomfilter_counting_init +bloomfilter_decrement +bloomfilter_internal_state +bloomfilter_intersect +bloomfilter_lookup +bloomfilter_merge +bytestring_to_count +bytestring_to_double +bytestring_to_float +bytestring_to_hexstr +calc_next_rotate +cat +cat_sep +ceil +check_subnet +clean +clear_table +close +community_id_v1 +compress_path +connection_exists +continue_processing +convert_for_pattern +count_substr +count_to_double +count_to_port +count_to_v4_addr +counts_to_addr +current_analyzer +current_event_time +current_time +decode_base64 +decode_base64_conn +decode_netbios_name +decode_netbios_name_type +disable_analyzer +disable_event_group +disable_module_events +do_profiling +double_to_count +double_to_int +double_to_interval +double_to_time +dump_current_packet +dump_packet +dump_rule_stats +edit +enable_event_group +enable_module_events +enable_raw_output +encode_base64 +ends_with +entropy_test_add +entropy_test_finish +entropy_test_init +enum_names +enum_to_int +escape_string +exit +exp +file_magic +file_mode +file_size +filter_subnet_table +find_all +find_all_ordered +find_entropy +find_last +find_str +floor +flush_all +fmt +fmt_ftp_port +fnv1a32 +from_json +generate_all_events +get_broker_stats +get_conn_stats +get_conn_transport_proto +get_contents_file +get_current_conn_bytes_threshold +get_current_conn_duration_threshold +get_current_conn_packets_threshold +get_current_packet +get_current_packet_header +get_dns_stats +get_event_handler_stats +get_event_stats +get_file_analysis_stats +get_file_name +get_gap_stats +get_identifier_comments +get_identifier_declaring_script +get_login_state +get_matcher_stats +get_net_stats +get_orig_seq +get_package_readme +get_port_transport_proto +get_proc_stats +get_reassembler_stats +get_record_field_comments +get_record_field_declaring_script +get_reporter_stats +get_resp_seq +get_script_comments +get_thread_stats +get_timer_stats +getenv +gethostname +getpid +global_container_footprints +global_ids +global_options +gsub +has_event_group +has_module_events +have_spicy +have_spicy_analyzers +haversine_distance +hexdump +hexstr_to_bytestring +hll_cardinality_add +hll_cardinality_copy +hll_cardinality_estimate +hll_cardinality_init +hll_cardinality_merge_into +hrw_weight +identify_data +install_dst_addr_filter +install_dst_net_filter +install_src_addr_filter +install_src_net_filter +int_to_count +int_to_double +interval_to_double +is_alnum +is_alpha +is_ascii +is_file_analyzer +is_icmp_port +is_local_interface +is_num +is_packet_analyzer +is_processing_suspended +is_protocol_analyzer +is_remote_event +is_tcp_port +is_udp_port +is_v4_addr +is_v4_subnet +is_v6_addr +is_v6_subnet +is_valid_ip +join_string_set +join_string_vec +levenshtein_distance +ljust +ln +load_CPP +log10 +log2 +lookup_ID +lookup_addr +lookup_autonomous_system +lookup_connection +lookup_hostname +lookup_hostname_txt +lookup_location +lstrip +mask_addr +match_signatures +matching_subnets +md5_hash +md5_hash_finish +md5_hash_init +md5_hash_update +md5_hmac +mkdir +mmdb_open_asn_db +mmdb_open_location_db +network_time +open +open_for_append +order +packet_source +paraglob_equals +paraglob_init +paraglob_match +parse_distinguished_name +parse_eftp_port +parse_ftp_epsv +parse_ftp_pasv +parse_ftp_port +piped_exec +port_to_count +pow +preserve_prefix +preserve_subnet +print_raw +ptr_name_to_addr +rand +raw_bytes_to_v4_addr +raw_bytes_to_v6_addr +reading_live_traffic +reading_traces +record_fields +record_type_to_vector +remask_addr +remove_prefix +remove_suffix +rename +resize +reverse +rfind_str +rjust +rmdir +rotate_file +rotate_file_by_name +routing0_data_to_addrs +rstrip +safe_shell_quote +same_object +sct_verify +set_buf +set_contents_file +set_current_conn_bytes_threshold +set_current_conn_duration_threshold +set_current_conn_packets_threshold +set_file_handle +set_inactivity_timeout +set_keys +set_login_state +set_network_time +set_record_packets +set_secret +set_ssl_established +setenv +sha1_hash +sha1_hash_finish +sha1_hash_init +sha1_hash_update +sha256_hash +sha256_hash_finish +sha256_hash_init +sha256_hash_update +skip_further_processing +skip_http_entity_data +skip_smtp_data +sort +split_string +split_string1 +split_string_all +split_string_n +sqrt +srand +starts_with +str_smith_waterman +str_split_indices +strcmp +strftime +string_cat +string_fill +string_to_ascii_hex +string_to_pattern +strip +strptime +strstr +sub +sub_bytes +subnet_to_addr +subnet_width +subst_string +suspend_processing +swap_case +syslog +system +system_env +table_keys +table_values +terminate +time_to_double +to_addr +to_count +to_double +to_int +to_json +to_lower +to_port +to_string_literal +to_subnet +to_title +to_upper +topk_add +topk_count +topk_epsilon +topk_get_top +topk_init +topk_merge +topk_merge_prune +topk_size +topk_sum +type_aliases +type_name +unescape_URI +uninstall_dst_addr_filter +uninstall_dst_net_filter +uninstall_src_addr_filter +uninstall_src_net_filter +unique_id +unique_id_from +unlink +uuid_to_string +val_footprint +write_file +x509_check_cert_hostname +x509_check_hostname +x509_from_der +x509_get_certificate_string +x509_issuer_name_hash +x509_ocsp_verify +x509_parse +x509_set_certificate_cache +x509_set_certificate_cache_hit_callback +x509_spki_hash +x509_subject_name_hash +x509_verify +zeek_args +zeek_is_terminating +zeek_version +zfill diff --git a/src/script_opt/ZAM/maint/README b/src/script_opt/ZAM/maint/README new file mode 100644 index 0000000000..ee69f88149 --- /dev/null +++ b/src/script_opt/ZAM/maint/README @@ -0,0 +1,14 @@ +This directory holds scripts and associated data to support maintenance of +ZAM optimization: + +list-bifs.zeek + A Zeek script that prints to stdout a sorted list of the BiFs + available for the Zeek invocation. + + Use this to compare with BiFs.list to see whether there are any + new BiFs (or old ones that have been removed). If so, update + src/script_opt/FuncInfo.cc and then BiFs.list accordingly. + +BiFs.list + The BiFs that were present last time ZAM maintenance included + looking for any updates to available BiFs. diff --git a/src/script_opt/ZAM/maint/list-bifs.zeek b/src/script_opt/ZAM/maint/list-bifs.zeek new file mode 100644 index 0000000000..07215d37b6 --- /dev/null +++ b/src/script_opt/ZAM/maint/list-bifs.zeek @@ -0,0 +1,14 @@ +# Prints to stdout an alphabetized list of all of the BiFs registered with Zeek. +event zeek_init() + { + local bifs: vector of string; + + for ( gn, gi in global_ids() ) + if ( gi$type_name == "func" && gi?$value && fmt("%s", gi$value) == gn ) + bifs += gn; + + bifs = sort(bifs, strcmp); + + for ( _, b in bifs ) + print b; + } diff --git a/testing/btest/Baseline.zam/opt.AST-side-effects/output b/testing/btest/Baseline.zam/opt.AST-side-effects/output new file mode 100644 index 0000000000..d28be37fb4 --- /dev/null +++ b/testing/btest/Baseline.zam/opt.AST-side-effects/output @@ -0,0 +1,635 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Original: { +print 2 * g1, tbl_c_of_c[2]; +print benign(4); +print 2 * g1, tbl_c_of_c[2]; +print safe_bif(); +print 2 * g1, tbl_c_of_c[2]; +print dangerous_bif(); +print 2 * g1, tbl_c_of_c[2]; +print 2 * g1, tbl_c_of_c[2]; +} <...>/AST-side-effects.zeek, lines 90-106 +Transformed: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +print ::#0, ::#1; +::#2 = benign(4); +print ::#2; +::#3 = 2 * g1; +::#4 = tbl_c_of_c[2]; +print ::#3, ::#4; +::#5 = safe_bif(); +print ::#5; +::#6 = 2 * g1; +::#7 = tbl_c_of_c[2]; +print ::#6, ::#7; +::#8 = dangerous_bif(); +print ::#8; +::#9 = 2 * g1; +::#10 = tbl_c_of_c[2]; +print ::#9, ::#10; +::#11 = 2 * g1; +::#12 = tbl_c_of_c[2]; +print ::#11, ::#12; +} <...>/AST-side-effects.zeek, lines 90-106 +Optimized: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +print ::#0, ::#1; +::#2 = benign(4); +print ::#2; +print ::#0, ::#1; +::#5 = safe_bif(); +print ::#5; +print ::#0, ::#1; +::#8 = dangerous_bif(); +print ::#8; +::#9 = 2 * g1; +::#10 = tbl_c_of_c[2]; +print ::#9, ::#10; +print ::#9, ::#10; +} <...>/AST-side-effects.zeek, lines 90-106 +Post removal of unused: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +print ::#0, ::#1; +::#2 = benign(4); +print ::#2; +print ::#0, ::#1; +::#5 = safe_bif(); +print ::#5; +print ::#0, ::#1; +::#8 = dangerous_bif(); +print ::#8; +::#9 = 2 * g1; +::#10 = tbl_c_of_c[2]; +print ::#9, ::#10; +print ::#9, ::#10; +} <...>/AST-side-effects.zeek, lines 90-106 +Original: { +print 2 * g1, 3 * g2, tbl_c_of_c[2]; +print tbl_addr_of_count[1.2.3.4]; +print 2 * g1, 3 * g2, tbl_c_of_c[2]; +print tbl_addr_of_count[127.0.0.1]; +print 2 * g1, 3 * g2, tbl_c_of_c[2]; +} <...>/AST-side-effects.zeek, lines 119-128 +Transformed: { +::#0 = 2 * g1; +::#1 = 3 * g2; +::#2 = tbl_c_of_c[2]; +print ::#0, ::#1, ::#2; +::#3 = tbl_addr_of_count[1.2.3.4]; +print ::#3; +::#4 = 2 * g1; +::#5 = 3 * g2; +::#6 = tbl_c_of_c[2]; +print ::#4, ::#5, ::#6; +::#7 = tbl_addr_of_count[127.0.0.1]; +print ::#7; +::#8 = 2 * g1; +::#9 = 3 * g2; +::#10 = tbl_c_of_c[2]; +print ::#8, ::#9, ::#10; +} <...>/AST-side-effects.zeek, lines 119-128 +Optimized: { +::#0 = 2 * g1; +::#1 = 3 * g2; +::#2 = tbl_c_of_c[2]; +print ::#0, ::#1, ::#2; +::#3 = tbl_addr_of_count[1.2.3.4]; +print ::#3; +::#4 = 2 * g1; +print ::#4, ::#1, ::#2; +::#7 = tbl_addr_of_count[127.0.0.1]; +print ::#7; +::#8 = 2 * g1; +print ::#8, ::#1, ::#2; +} <...>/AST-side-effects.zeek, lines 119-128 +Post removal of unused: { +::#0 = 2 * g1; +::#1 = 3 * g2; +::#2 = tbl_c_of_c[2]; +print ::#0, ::#1, ::#2; +::#3 = tbl_addr_of_count[1.2.3.4]; +print ::#3; +::#4 = 2 * g1; +print ::#4, ::#1, ::#2; +::#7 = tbl_addr_of_count[127.0.0.1]; +print ::#7; +::#8 = 2 * g1; +print ::#8, ::#1, ::#2; +} <...>/AST-side-effects.zeek, lines 119-128 +Original: { +print tbl_c_of_c[2]; +print tbl_c_of_c[2]; +tcc_mod[F] = 33; +print tbl_c_of_c[2]; +print tbl_c_of_c[2]; +mess_is_active = T; +tcc_mod[T] = 66; +print tbl_c_of_c[2]; +print tbl_c_of_c[2]; +} <...>/AST-side-effects.zeek, lines 153-170 +Transformed: { +::#0 = tbl_c_of_c[2]; +print ::#0; +::#1 = tbl_c_of_c[2]; +print ::#1; +tcc_mod[F] []= 33; +::#2 = tbl_c_of_c[2]; +print ::#2; +::#3 = tbl_c_of_c[2]; +print ::#3; +mess_is_active = T; +tcc_mod[T] []= 66; +::#4 = tbl_c_of_c[2]; +print ::#4; +::#5 = tbl_c_of_c[2]; +print ::#5; +} <...>/AST-side-effects.zeek, lines 153-170 +Optimized: { +::#0 = tbl_c_of_c[2]; +print ::#0; +print ::#0; +tcc_mod[F] []= 33; +::#2 = tbl_c_of_c[2]; +print ::#2; +print ::#2; +mess_is_active = T; +tcc_mod[T] []= 66; +::#4 = tbl_c_of_c[2]; +print ::#4; +print ::#4; +} <...>/AST-side-effects.zeek, lines 153-170 +Post removal of unused: { +::#0 = tbl_c_of_c[2]; +print ::#0; +print ::#0; +tcc_mod[F] []= 33; +::#2 = tbl_c_of_c[2]; +print ::#2; +print ::#2; +mess_is_active = T; +tcc_mod[T] []= 66; +::#4 = tbl_c_of_c[2]; +print ::#4; +print ::#4; +} <...>/AST-side-effects.zeek, lines 153-170 +Original: { +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_c2[10]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_c2[10]; +local_tbl_c_of_c3 = table(4 = 1, 12 = 0)&default=function(){ +return (benign(c + 7) - 2); +}, &optional; +print local_tbl_c_of_c3[12]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12]; +print local_tbl_c_of_c3[10]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[10]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[10]; +print dangerous_bif(); +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12], local_tbl_c_of_c3[10]; +} <...>/AST-side-effects.zeek, lines 178-204 +Transformed: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_c2[10]; +print ::#0, ::#1, ::#2, ::#3; +::#4 = 2 * g1; +::#5 = tbl_c_of_c[2]; +::#6 = tbl_c_of_c2[1]; +::#7 = tbl_c_of_c2[10]; +print ::#4, ::#5, ::#6, ::#7; +local_tbl_c_of_c3 = table(4 = 1, 12 = 0)&default=function(){ +return (benign(c + 7) - 2); +}, &optional; +::#8 = local_tbl_c_of_c3[12]; +print ::#8; +::#9 = 2 * g1; +::#10 = tbl_c_of_c[2]; +::#11 = tbl_c_of_c2[1]; +::#12 = local_tbl_c_of_c3[12]; +print ::#9, ::#10, ::#11, ::#12; +::#13 = local_tbl_c_of_c3[10]; +print ::#13; +::#14 = 2 * g1; +::#15 = tbl_c_of_c[2]; +::#16 = tbl_c_of_c2[1]; +::#17 = local_tbl_c_of_c3[10]; +print ::#14, ::#15, ::#16, ::#17; +::#18 = 2 * g1; +::#19 = tbl_c_of_c[2]; +::#20 = tbl_c_of_c2[1]; +::#21 = local_tbl_c_of_c3[10]; +print ::#18, ::#19, ::#20, ::#21; +::#22 = dangerous_bif(); +print ::#22; +::#23 = 2 * g1; +::#24 = tbl_c_of_c[2]; +::#25 = tbl_c_of_c2[1]; +::#26 = local_tbl_c_of_c3[12]; +print ::#23, ::#24, ::#25, ::#26; +::#27 = 2 * g1; +::#28 = tbl_c_of_c[2]; +::#29 = tbl_c_of_c2[1]; +::#30 = local_tbl_c_of_c3[12]; +::#31 = local_tbl_c_of_c3[10]; +print ::#27, ::#28, ::#29, ::#30, ::#31; +} <...>/AST-side-effects.zeek, lines 178-204 +Optimized: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_c2[10]; +print ::#0, ::#1, ::#2, ::#3; +print ::#0, ::#1, ::#2, ::#3; +local_tbl_c_of_c3 = table(4 = 1, 12 = 0)&default=function(){ +return (benign(c + 7) - 2); +}, &optional; +::#8 = local_tbl_c_of_c3[12]; +print ::#8; +print ::#0, ::#1, ::#2, ::#8; +::#13 = local_tbl_c_of_c3[10]; +print ::#13; +print ::#0, ::#1, ::#2, ::#13; +print ::#0, ::#1, ::#2, ::#13; +::#22 = dangerous_bif(); +print ::#22; +::#23 = 2 * g1; +::#24 = tbl_c_of_c[2]; +::#25 = tbl_c_of_c2[1]; +::#26 = local_tbl_c_of_c3[12]; +print ::#23, ::#24, ::#25, ::#26; +::#31 = local_tbl_c_of_c3[10]; +print ::#23, ::#24, ::#25, ::#26, ::#31; +} <...>/AST-side-effects.zeek, lines 178-204 +Post removal of unused: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_c2[10]; +print ::#0, ::#1, ::#2, ::#3; +print ::#0, ::#1, ::#2, ::#3; +local_tbl_c_of_c3 = table(4 = 1, 12 = 0)&default=function(){ +return (benign(c + 7) - 2); +}, &optional; +::#8 = local_tbl_c_of_c3[12]; +print ::#8; +print ::#0, ::#1, ::#2, ::#8; +::#13 = local_tbl_c_of_c3[10]; +print ::#13; +print ::#0, ::#1, ::#2, ::#13; +print ::#0, ::#1, ::#2, ::#13; +::#22 = dangerous_bif(); +print ::#22; +::#23 = 2 * g1; +::#24 = tbl_c_of_c[2]; +::#25 = tbl_c_of_c2[1]; +::#26 = local_tbl_c_of_c3[12]; +print ::#23, ::#24, ::#25, ::#26; +::#31 = local_tbl_c_of_c3[10]; +print ::#23, ::#24, ::#25, ::#26, ::#31; +} <...>/AST-side-effects.zeek, lines 178-204 +Original: { +side_effect_free = table(0 = 0.5, 1 = 1.5); +print side_effect_free[1]; +print side_effect_free[1]; +has_side_effects = table(0 = -0.5, 1 = -1.5)&default_insert=function(){ +my_exponential = my_exponential * my_exponential; +return (my_exponential); +}; +print side_effect_free[1], has_side_effects[2]; +print side_effect_free[1], has_side_effects[2]; +} <...>/AST-side-effects.zeek, lines 213-238 +Transformed: { +side_effect_free = table(0 = 0.5, 1 = 1.5); +::#0 = side_effect_free[1]; +print ::#0; +::#1 = side_effect_free[1]; +print ::#1; +has_side_effects = table(0 = -0.5, 1 = -1.5)&default_insert=function(){ +my_exponential = my_exponential * my_exponential; +return (my_exponential); +}; +::#2 = side_effect_free[1]; +::#3 = has_side_effects[2]; +print ::#2, ::#3; +::#4 = side_effect_free[1]; +::#5 = has_side_effects[2]; +print ::#4, ::#5; +} <...>/AST-side-effects.zeek, lines 213-238 +Optimized: { +side_effect_free = table(0 = 0.5, 1 = 1.5); +::#0 = side_effect_free[1]; +print ::#0; +::#1 = side_effect_free[1]; +print ::#1; +has_side_effects = table(0 = -0.5, 1 = -1.5)&default_insert=function(){ +my_exponential = my_exponential * my_exponential; +return (my_exponential); +}; +::#2 = side_effect_free[1]; +::#3 = has_side_effects[2]; +print ::#2, ::#3; +::#4 = side_effect_free[1]; +::#5 = has_side_effects[2]; +print ::#4, ::#5; +} <...>/AST-side-effects.zeek, lines 213-238 +Post removal of unused: { +side_effect_free = table(0 = 0.5, 1 = 1.5); +::#0 = side_effect_free[1]; +print ::#0; +::#1 = side_effect_free[1]; +print ::#1; +has_side_effects = table(0 = -0.5, 1 = -1.5)&default_insert=function(){ +my_exponential = my_exponential * my_exponential; +return (my_exponential); +}; +::#2 = side_effect_free[1]; +::#3 = has_side_effects[2]; +print ::#2, ::#3; +::#4 = side_effect_free[1]; +::#5 = has_side_effects[2]; +print ::#4, ::#5; +} <...>/AST-side-effects.zeek, lines 213-238 +Original: { +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_b[100], tbl_s_of_s[4]; +print 2 * g1, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_b[100], tbl_s_of_s[4]; +print table(11 = F)&default=function(){ +tbl_s_of_s2[2] = two; +return (sizeoftbl_s_of_s2 < 9); +}, &optional; +} <...>/AST-side-effects.zeek, lines 247-261 +Transformed: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_b[100]; +::#4 = tbl_s_of_s[4]; +print ::#0, ::#1, ::#2, ::#3, ::#4; +::#5 = 2 * g1; +::#6 = tbl_c_of_c[2]; +::#7 = tbl_c_of_c2[1]; +::#8 = tbl_c_of_b[100]; +::#9 = tbl_s_of_s[4]; +print ::#5, ::#6, ::#7, ::#8, ::#9; +::#10 = table(11 = F)&default=function(){ +tbl_s_of_s2[2] = two; +return (sizeoftbl_s_of_s2 < 9); +}, &optional; +print ::#10; +} <...>/AST-side-effects.zeek, lines 247-261 +Optimized: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_b[100]; +::#4 = tbl_s_of_s[4]; +print ::#0, ::#1, ::#2, ::#3, ::#4; +::#8 = tbl_c_of_b[100]; +::#9 = tbl_s_of_s[4]; +print ::#0, ::#1, ::#2, ::#8, ::#9; +::#10 = table(11 = F)&default=function(){ +tbl_s_of_s2[2] = two; +return (sizeoftbl_s_of_s2 < 9); +}, &optional; +print ::#10; +} <...>/AST-side-effects.zeek, lines 247-261 +Post removal of unused: { +::#0 = 2 * g1; +::#1 = tbl_c_of_c[2]; +::#2 = tbl_c_of_c2[1]; +::#3 = tbl_c_of_b[100]; +::#4 = tbl_s_of_s[4]; +print ::#0, ::#1, ::#2, ::#3, ::#4; +::#8 = tbl_c_of_b[100]; +::#9 = tbl_s_of_s[4]; +print ::#0, ::#1, ::#2, ::#8, ::#9; +::#10 = table(11 = F)&default=function(){ +tbl_s_of_s2[2] = two; +return (sizeoftbl_s_of_s2 < 9); +}, &optional; +print ::#10; +} <...>/AST-side-effects.zeek, lines 247-261 +Original: { +print tbl_s_of_s[4]; +print tbl_s_of_s[4]; +force_tbl_c_of_c_str_reload = R1($b=match-on-type-of-tbl_c_of_c); +print force_tbl_c_of_c_str_reload; +print tbl_s_of_s[4]; +print tbl_s_of_s[4]; +problem_r2 = R2($b=hello); +print tbl_s_of_s[4], problem_r2$b; +problem_r3 = R3($b=hello again); +print tbl_s_of_s[4], problem_r3$b; +} <...>/AST-side-effects.zeek, lines 306-336 +Transformed: { +::#0 = tbl_s_of_s[4]; +print ::#0; +::#1 = tbl_s_of_s[4]; +print ::#1; +force_tbl_c_of_c_str_reload = R1($b=match-on-type-of-tbl_c_of_c); +print force_tbl_c_of_c_str_reload; +::#2 = tbl_s_of_s[4]; +print ::#2; +::#3 = tbl_s_of_s[4]; +print ::#3; +problem_r2 = R2($b=hello); +::#4 = tbl_s_of_s[4]; +::#5 = problem_r2$b; +print ::#4, ::#5; +problem_r3 = R3($b=hello again); +::#6 = tbl_s_of_s[4]; +::#7 = problem_r3$b; +print ::#6, ::#7; +} <...>/AST-side-effects.zeek, lines 306-336 +Optimized: { +::#0 = tbl_s_of_s[4]; +print ::#0; +print ::#0; +force_tbl_c_of_c_str_reload = R1($b=match-on-type-of-tbl_c_of_c); +print force_tbl_c_of_c_str_reload; +::#2 = tbl_s_of_s[4]; +print ::#2; +print ::#2; +problem_r2 = R2($b=hello); +::#4 = tbl_s_of_s[4]; +::#5 = problem_r2$b; +print ::#4, ::#5; +problem_r3 = R3($b=hello again); +::#7 = problem_r3$b; +print ::#4, ::#7; +} <...>/AST-side-effects.zeek, lines 306-336 +Post removal of unused: { +::#0 = tbl_s_of_s[4]; +print ::#0; +print ::#0; +force_tbl_c_of_c_str_reload = R1($b=match-on-type-of-tbl_c_of_c); +print force_tbl_c_of_c_str_reload; +::#2 = tbl_s_of_s[4]; +print ::#2; +print ::#2; +problem_r2 = R2($b=hello); +::#4 = tbl_s_of_s[4]; +::#5 = problem_r2$b; +print ::#4, ::#5; +problem_r3 = R3($b=hello again); +::#7 = problem_r3$b; +print ::#4, ::#7; +} <...>/AST-side-effects.zeek, lines 306-336 +Original: { +print 5 * g2; +print 5 * g2; +l1 = table(1, 3 = 4, 2, 5 = 6)&default=function(){ +my_tbl = table(1.0, 3.0 = 10000.0); +return (double_to_count(my_tbl[1.0, 3.0])); +}, &optional; +l2 = table(1.0, 3.0 = 4.0, 2.0, 5.0 = 6.0)&default=function(){ +my_tbl = table(1, 3 = 1000); +return ((coerce my_tbl[1, 3] to double)); +}, &optional; +print 5 * g2; +print 5 * g2; +print l1[3, 8]; +print 5 * g2; +print 5 * g2; +print l2[2.0, 5.0]; +print 5 * g2; +print 5 * g2; +} <...>/AST-side-effects.zeek, lines 341-390 +Transformed: { +::#0 = 5 * g2; +print ::#0; +::#1 = 5 * g2; +print ::#1; +l1 = table(1, 3 = 4, 2, 5 = 6)&default=function(){ +my_tbl = table(1.0, 3.0 = 10000.0); +return (double_to_count(my_tbl[1.0, 3.0])); +}, &optional; +l2 = table(1.0, 3.0 = 4.0, 2.0, 5.0 = 6.0)&default=function(){ +my_tbl = table(1, 3 = 1000); +return ((coerce my_tbl[1, 3] to double)); +}, &optional; +::#2 = 5 * g2; +print ::#2; +::#3 = 5 * g2; +print ::#3; +::#4 = l1[3, 8]; +print ::#4; +::#5 = 5 * g2; +print ::#5; +::#6 = 5 * g2; +print ::#6; +::#7 = l2[2.0, 5.0]; +print ::#7; +::#8 = 5 * g2; +print ::#8; +::#9 = 5 * g2; +print ::#9; +} <...>/AST-side-effects.zeek, lines 341-390 +Optimized: { +::#0 = 5 * g2; +print ::#0; +print ::#0; +l1 = table(1, 3 = 4, 2, 5 = 6)&default=function(){ +my_tbl = table(1.0, 3.0 = 10000.0); +return (double_to_count(my_tbl[1.0, 3.0])); +}, &optional; +l2 = table(1.0, 3.0 = 4.0, 2.0, 5.0 = 6.0)&default=function(){ +my_tbl = table(1, 3 = 1000); +return ((coerce my_tbl[1, 3] to double)); +}, &optional; +print ::#0; +print ::#0; +::#4 = l1[3, 8]; +print ::#4; +::#5 = 5 * g2; +print ::#5; +print ::#5; +::#7 = l2[2.0, 5.0]; +print ::#7; +::#8 = 5 * g2; +print ::#8; +print ::#8; +} <...>/AST-side-effects.zeek, lines 341-390 +Post removal of unused: { +::#0 = 5 * g2; +print ::#0; +print ::#0; +l1 = table(1, 3 = 4, 2, 5 = 6)&default=function(){ +my_tbl = table(1.0, 3.0 = 10000.0); +return (double_to_count(my_tbl[1.0, 3.0])); +}, &optional; +l2 = table(1.0, 3.0 = 4.0, 2.0, 5.0 = 6.0)&default=function(){ +my_tbl = table(1, 3 = 1000); +return ((coerce my_tbl[1, 3] to double)); +}, &optional; +print ::#0; +print ::#0; +::#4 = l1[3, 8]; +print ::#4; +::#5 = 5 * g2; +print ::#5; +print ::#5; +::#7 = l2[2.0, 5.0]; +print ::#7; +::#8 = 5 * g2; +print ::#8; +print ::#8; +} <...>/AST-side-effects.zeek, lines 341-390 +8, 3 +7 +8, 3 +28 +8, 3 +3 +8, 456 +8, 456 +8, 132, 456 +1001 +8, 132, 456 +5 +10, 132, 456 +456 +456 +456 +456 +999 +999 +10, 999, 4, 13 +10, 999, 4, 13 +0 +10, 999, 4, 0 +18 +10, 999, 4, 18 +10, 999, 4, 18 +999 +10, 456, 4, 0 +10, 456, 4, 0, 18 +1.5 +1.5 +1.5, 4.0 +1.5, 4.0 +10, 456, 4, F, 8 +10, 456, 4, F, 8 +{ +[11] = F +} +8 +8 +[a=3, b=match-on-type-of-tbl_c_of_c] +8 +8 +8, hello +8, hello again +220 +220 +220 +220 +10000 +220 +220 +6.0 +220 +220 diff --git a/testing/btest/opt/AST-side-effects.zeek b/testing/btest/opt/AST-side-effects.zeek new file mode 100644 index 0000000000..bb204c40f7 --- /dev/null +++ b/testing/btest/opt/AST-side-effects.zeek @@ -0,0 +1,405 @@ +# @TEST-DOC: Stress tests for the AST optimizer dealing with side effects. +# @TEST-REQUIRES: test "${ZEEK_ZAM}" == "1" +# +# See below for an explanation of this convoluted invocation line. +# @TEST-EXEC: zeek -b -O ZAM -O dump-xform --optimize-func='AST_opt_test_.*' %INPUT >output +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output + +# This is a subtle & extensive test of the AST optimizer used by ZAM, in +# particular its CSE = Common Subexpression Elimination functionality, whereby +# it reuses previously computed values rather than recomputing them. This test +# in particular focuses on that functionality in the context of side-effects +# that arise due to either implicit or explicit function calls. The implicit +# ones (such as table &default functions) are the trickiest, because they +# can in principle access or modify state that leads to *other* implicit +# function calls. +# +# Unlike nearly all Zeek BTests, the heart of the output is intermediary +# information produced by "-O dump-xform", not the final output. The dump +# implicitly reveals where the AST optimizer reuses expressions previously +# computed (into temporaries) and where it refrains from doing so because +# in principle the re-use is unsafe due to possible modifications to the +# original expression. Many of the tests print the same set of values twice in +# a row, often to see whether the first "print" requires correctly re-computing +# an expression or inheriting its value from a previous computation. The +# second "print" generally ensures that given the lack of potential +# side-effects, the values from the first "print" are re-used, although for +# some tests they need to instead be re-computed. +# +# If the dumped intermediary information agrees but there's a difference in +# the final output as the Zeek script executes, that reflects a bug subsequent +# to the AST optimizer (typically, this will be in ZAM's low-level optimizer). +# +# Changes to the AST processing used by script optimization can introduce +# benign differences to the intermediary information, such as introducing +# or removing some temporary variables. These require hand inspection to +# determine whether they reflect a problem. +# +# We divide the tests up into a number of different event handlers. This +# keeps the AST for each group of tests from getting too complex (especially +# if later tests wind up reusing values from much earlier tests, which can +# be hard to manually verify for correctness). We enable ZAM optimization +# for *only* those handlers to avoid complications in the output unrelated +# to these tests. We use distinct event handlers, rather than multiple +# handlers for the same event, to avoid coalescence of the handlers. Doing +# this explicitly rather than using "-O no-inline" is preferrable because +# it means the testing still includes any issues that the inliner might +# introduce. +# +# Any time you make a change to the final output the script produces, confirm +# that running Zeek *without* script optimization produces that same output. + +######################################################################## + +# A global that in some contexts is directly modified as a side-effect. +global g1 = 4; + +# A global that is never directly modified as a side-effect. However, some +# side effects have Unknown consequences - for those, the optimizer should +# assume that any expression derived from a global value needs to re-computed. +global g2 = 44; + +# A table with a safe &default. However, in some contexts the tests will +# introduce other tables with the same type signature whose &default's have +# side effects. Due to the complexity of potential aliasing among Zeek +# container objects of like types, those side effects should be presumed +# to potentially modify this global. +global tbl_c_of_c = table([2] = 3, [3] = 5) &default=456; + +# A function with no side effects. +function benign(a: count): count + { + return a + 3; + } + +# Calls a BiF that should be known to the optimizer as having no side effects. +function safe_bif(): string + { + return fmt("%s", g1 * 7); + } + +# Calls a BiF that the optimizer doesn't know as having no side effects, +# and which should therefore be treated as having Unknown side effects. +function dangerous_bif(): count + { + local l = tbl_c_of_c[2]; + clear_table(tbl_c_of_c); + return l; + } + +event AST_opt_test_1() + { + # For the following, the optimizer should reuse "g1 * 2" and + # "tbl_c_of_c[2]" for the later print's ... + print g1 * 2, tbl_c_of_c[2]; + print benign(4); + print g1 * 2, tbl_c_of_c[2]; + print safe_bif(); + print g1 * 2, tbl_c_of_c[2]; + + # ... but *not* after this call. In addition, even though the call + # doesn't modify "g1", it should still be reloaded because + # the optimizer only knows that the BiF is unsafe = it has Unknown + # effects. + print dangerous_bif(); + print g1 * 2, tbl_c_of_c[2]; + print g1 * 2, tbl_c_of_c[2]; + } + +######################################################################## + +# A function that modifies our global-of-interest. +function mod_g1(a: addr): count + { + return ++g1; + } + +global tbl_addr_of_count = table([1.2.3.4] = 1001, [2.3.4.5] = 10002) &default=mod_g1; + +event AST_opt_test_2() + { + # The optimizer should reload "g1" after each tbl_addr_of_count + # reference, but reuse "g2 * 3" and "tbl_c_of_c[2]", since those + # can't be affected by mod_g1(). + print g1 * 2, g2 * 3, tbl_c_of_c[2]; + print tbl_addr_of_count[1.2.3.4]; + print g1 * 2, g2 * 3, tbl_c_of_c[2]; + print tbl_addr_of_count[127.0.0.1]; + print g1 * 2, g2 * 3, tbl_c_of_c[2]; + } + +######################################################################## + +# A global that controls whether the following functions change an entry +# in one of our global tables. +global mess_is_active = F; + +# We use a separate function for actually changing the global to make sure +# that the AST analysis follows the effects of function calls. +function do_the_messing() + { + tbl_c_of_c[2] = 999; + } + +# An &on_change handler that potentially alters the value of the table. +function mess_with_tbl_c_of_c(tbl: table[bool] of count, tc: TableChange, ind: bool, val: count) + { + if ( mess_is_active ) + do_the_messing(); + } + +global tcc_mod = table([F] = 44, [T] = 55) &on_change=mess_with_tbl_c_of_c; + +event AST_opt_test_3() + { + # The optimizer should re-use "tbl_c_of_c[2]" in each of the secondary + # print's, but it should recompute it after each modification to + # "tcc_mod", even though the first one won't lead to a potential change. + print tbl_c_of_c[2]; + print tbl_c_of_c[2]; + + tcc_mod[F] = 33; + + print tbl_c_of_c[2]; + print tbl_c_of_c[2]; + + mess_is_active = T; + tcc_mod[T] = 66; + + print tbl_c_of_c[2]; + print tbl_c_of_c[2]; + } + +######################################################################## + +# Analogous to tbl_c_of_c but has a function call for its &default. +global tbl_c_of_c2 = table([1] = 4, [4] = 8) &default=benign; + +event AST_opt_test_4() + { + # In the following, for the duplicated prints the optimizer should + # reuse the previous values. That includes for the accesses to + # local_tbl_c_of_c3 (which associates a more complex &default function + # to "table[count] of count" types). + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_c2[10]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_c2[10]; + + local local_tbl_c_of_c3 = table([4] = 1, [12] = 0) + &default=function(c: count): count { return benign(c+7) - 2; }; + + # We print at this separately to make sure it occurs prior to + # potentially computing the other elements in the print. + print local_tbl_c_of_c3[12]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12]; + + # Same with separate printing here. + print local_tbl_c_of_c3[10]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[10]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[10]; + + # This BiF should lead to recomputing of all values, including + # the local local_tbl_c_of_c3. + print dangerous_bif(); + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], local_tbl_c_of_c3[12], local_tbl_c_of_c3[10]; + } + +######################################################################## + +# Used to introduce another global as having side effects, this time in +# the context of a local table. +global my_exponential = 2.0; + +event AST_opt_test_5() + { + # A similar test, but this time the second local has side effects, + # and aliases type-wise with the first local, so expressions with + # the first local should be recomputed for every access. + # + # The notion of aggregate aliases is important because in general + # with static analysis it can be extremely difficult to tell whether + # two instances of an aggregate, each with the same type, might in + # fact refer to the same underlying aggregate value. The optimizer + # thus needs to play it safe and refrain from only focusing on the + # instance that directly has the attribute. + local side_effect_free = table([0] = 0.5, [1] = 1.5); + + print side_effect_free[1]; + print side_effect_free[1]; + + local has_side_effects = table([0] = -0.5, [1] = -1.5) + &default_insert=function(c: count): double + { + my_exponential = my_exponential * my_exponential; + return my_exponential; + }; + + print side_effect_free[1], has_side_effects[2]; + print side_effect_free[1], has_side_effects[2]; + } + +######################################################################## + +global tbl_c_of_b = table([100] = F, [101] = T); +global tbl_s_of_s = table(["1"] = "4", ["4"] = "8") &default="no-side-effects"; +global tbl_s_of_s2 = table(["1"] = "4", ["4"] = "8") &default="ultimately-side-effects"; + +event AST_opt_test_6() + { + # Without the final statement of this handler, the second one of these + # prints would re-use all of the values. However, it instead can only + # reuse the first three, as explained below. + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_b[100], tbl_s_of_s["4"]; + print g1 * 2, tbl_c_of_c[2], tbl_c_of_c2[1], tbl_c_of_b[100], tbl_s_of_s["4"]; + + # This should force recomputation of tbl_c_of_b above (because its + # type aliases with one that has side effects on loads, so loads + # must not be optimized away) AND THEREFORE also tbl_s_of_s (because + # those loads affect an alias of its type). + print table([11] = F) + &default=function(c: count): bool + { tbl_s_of_s2["2"] = "two"; return |tbl_s_of_s2| < 9; }; + } + +######################################################################## + +# Here we test &default side-effects of record constructors. We also use +# this as an opportunity to stress-test propagation of side effects +# when one side effect induces another, and whether AST analysis correctly +# accounts for potential type aliasing. + +# Reads a local table - but that should be enough to raise the specter of +# any "table[count] of bool" access side-effects. +function local_table_read(): count + { + local l = table([99] = T); + return l[99] ? 3 : 2; + } + +# The same, but modifies the table instead. +function local_table_mod(): count + { + local l = table([99] = T); + l[99] = F; + return |l|; + } + +# Tickles "table[count] of bool" access side-effects. +type R1: record { + a: count &default = local_table_read(); + b: string; +}; + +# Similar, but for modification side-effects. NOTE: type-compatible with R1. +type R2: record { + a: count &default = local_table_mod(); + b: string; +}; + +# Just like R2 but NOT type-compatible with R1. +type R3: record { + a: count &default = local_table_mod(); + b: string; + c: int &default = +5; +}; + +event AST_opt_test_7() + { + # In this test, it's not the mere presence of "R1" that affects + # earlier optimization (like in the previous test), but the execution + # of its constructor. So the following should proceed with the second + # completely re-using from the first. + print tbl_s_of_s["4"]; + print tbl_s_of_s["4"]; + + # Here constructing R1 potentially invokes the &default for $a, + # which reads from a "table[count] of bool", which the previous + # test (AST_opt_test_6) has set up as potentially affecting values + # of type "table[string] of string" such as tbl_s_of_s. + local force_tbl_c_of_c_str_reload = R1($b="match-on-type-of-tbl_c_of_c"); + print force_tbl_c_of_c_str_reload; + print tbl_s_of_s["4"]; + print tbl_s_of_s["4"]; + + # Here's a similar sequence but using a record that *modifies* + # a "table[count] of bool" - which ordinarily should NOT thwart + # optimization since there's no potential &on_change attribute for + # such tables ... HOWEVER R2 is type-compatible with R1, so it + # inherits R1's side-effects, thus this WILL require a reload of + # tbl_s_of_s["4"]. + local problem_r2 = R2($b="hello"); + print tbl_s_of_s["4"], problem_r2$b; + + # ... THIS however won't, because R3 is not type-compatible with R1, + # even though it has the same attributes as R2. + local problem_r3 = R3($b="hello again"); + print tbl_s_of_s["4"], problem_r3$b; + } + +######################################################################## + +event AST_opt_test_8() + { + # This last one is a hum-dinger. The two locals we define introduce + # mutually recursive side-effects, type-wise: the first sets up that + # a "table[count, count] of count" can access a "table[double, double] + # of double", and the latter establishes the converse. First, this + # setup requires that the profiler doesn't wind up in infinite + # recursion trying to follow dependencies. Second, it should both + # spot those *and* turn them into Unknown side-effects ... which + # means that reuse of expression involving the global g2 - which is + # never altered anywhere - that crosses an access to such a table + # should be recomputed, rather than reusing the previous + # result, because "all bets are off" for Unknown side-effects. + + # The second of these should reuse the previous temporary ... + print g2 * 5; + print g2 * 5; + + local l1 = table([1, 3] = 4, [2, 5] = 6) + &default = function(i1: count, i2: count): count + { + local my_tbl = table([1.0, 3.0] = 1e4); + return double_to_count(my_tbl[1.0, 3.0]); + }; + + local l2 = table([1.0, 3.0] = 4.0, [2.0, 5.0] = 6.0) + &default = function(d1: double, d2: double): double + { + local my_tbl = table([1, 3] = 1000); + return my_tbl[1, 3]; + }; + + # ... as should both of these, since we haven't yet done an *access* + # to one of the tables. + print g2 * 5; + print g2 * 5; + + # Here's an access. + print l1[3, 8]; + + # The first of these should recompute, the second should re-use. + print g2 * 5; + print g2 * 5; + + # Here's an access to the other. + print l2[2.0, 5.0]; + + # Again, the first of these should recompute, the second should re-use. + print g2 * 5; + print g2 * 5; + } + +######################################################################## + +event zeek_init() + { + event AST_opt_test_1(); + event AST_opt_test_2(); + event AST_opt_test_3(); + event AST_opt_test_4(); + event AST_opt_test_5(); + event AST_opt_test_6(); + event AST_opt_test_7(); + event AST_opt_test_8(); + }