replace --optimize-only with --optimize-funcs and --optimize-files

src/Options.cc

@@ -122,10 +122,12 @@ void usage(const char* prog, int code)
 	fprintf(stderr, "    -I|--print-id <ID name>         | print out given ID\n");
 	fprintf(stderr, "    -N|--print-plugins              | print available plugins and exit (-NN "
 	                "for verbose)\n");
-	fprintf(stderr, "    -O|--optimize[=<option>]        | enable script optimization (use -O help "
+	fprintf(stderr, "    -O|--optimize <option>          | enable script optimization (use -O help "
 	                "for options)\n");
-	fprintf(stderr, "    -o|--optimize-only=<func>       | enable script optimization only for the "
-	                "given function\n");
+	fprintf(stderr, "    -0|--optimize-files=<pat>       | enable script optimization for all "
+	                "functions in files with names containing the given pattern\n");
+	fprintf(stderr, "    -o|--optimize-funcs=<pat>       | enable script optimization for "
+	                "functions with names fully matching the given pattern\n");
 	fprintf(stderr, "    -P|--prime-dns                  | prime DNS\n");
 	fprintf(stderr,
 	        "    -Q|--time                       | print execution time summary to stderr\n");
@@ -383,7 +385,8 @@ Options parse_cmdline(int argc, char** argv)
 		{"save-seeds", required_argument, nullptr, 'H'},
 		{"print-plugins", no_argument, nullptr, 'N'},
 		{"optimize", required_argument, nullptr, 'O'},
-		{"optimize-only", required_argument, nullptr, 'o'},
+		{"optimize-funcs", required_argument, nullptr, 'o'},
+		{"optimize-files", required_argument, nullptr, '0'},
 		{"prime-dns", no_argument, nullptr, 'P'},
 		{"time", no_argument, nullptr, 'Q'},
 		{"debug-rules", no_argument, nullptr, 'S'},
@@ -406,7 +409,7 @@ Options parse_cmdline(int argc, char** argv)
 	};
 
 	char opts[256];
-	util::safe_strncpy(opts, "B:c:e:f:G:H:I:i:j::n:O:o:p:r:s:T:t:U:w:X:CDFMNPQSWabdhmuv",
+	util::safe_strncpy(opts, "B:c:e:f:G:H:I:i:j::n:O:0:o:p:r:s:T:t:U:w:X:CDFMNPQSWabdhmuv",
 	                   sizeof(opts));
 
 	int op;
@@ -546,7 +549,10 @@ Options parse_cmdline(int argc, char** argv)
 				set_analysis_option(optarg, rval);
 				break;
 			case 'o':
-				rval.analysis_options.only_func = optarg;
+				add_func_analysis_pattern(rval.analysis_options, optarg);
+				break;
+			case '0':
+				add_file_analysis_pattern(rval.analysis_options, optarg);
 				break;
 			case 'P':
 				if ( rval.dns_mode != detail::DNS_DEFAULT )

src/scan.l

@@ -61,8 +61,9 @@ int conditional_epoch = 0;
 // Whether the current file has included conditionals (so far).
 bool current_file_has_conditionals = false;
 
-// The files that include conditionals.  Not currently used, but will be
-// in the future once we add --optimize-files=/pat/.
+// The files that include conditionals.  Used when compiling scripts to C++
+// to flag issues for --optimize-files=/pat/ where one of the files includes
+// conditional code.
 std::unordered_set<std::string> files_with_conditionals;
 
 zeek::detail::int_list if_stack;
@@ -692,6 +693,8 @@ static int load_files(const char* orig_file)
 	// every Obj created when parsing it.
 	yylloc.filename = filename = zeek::util::copy_string(file_path.c_str());
 
+	current_file_has_conditionals = files_with_conditionals.count(filename) > 0;
+
 	entry_cond_depth.push_back(conditional_depth);
 
 	return 1;
@@ -873,7 +876,6 @@ int yywrap()
 		}
 
 	last_filename = ::filename;
-	current_file_has_conditionals = false;
 
 	if ( zeek::reporter->Errors() > 0 )
 		return 1;
@@ -884,8 +886,11 @@ int yywrap()
 		delete file_stack.remove_nth(file_stack.length() - 1);
 
 	if ( YY_CURRENT_BUFFER )
+		{
 		// There's more on the stack to scan.
+		current_file_has_conditionals = files_with_conditionals.count(::filename) > 0;
 		return 0;
+		}
 
 	// Stack is now empty.
 	while ( essential_input_files.length() > 0 || input_files.length() > 0 )

src/script_opt/CPP/Driver.cc

@@ -6,6 +6,8 @@
 
 #include "zeek/script_opt/CPP/Compile.h"
 
+extern std::unordered_set<std::string> files_with_conditionals;
+
 namespace zeek::detail
 	{
 
@@ -72,29 +74,48 @@ void CPPCompile::Compile(bool report_uncompilable)
 
 	GenProlog();
 
+	unordered_set<string> filenames_reported_as_skipped;
+
 	// Determine which functions we can call directly, and reuse
 	// previously compiled instances of those if present.
-	for ( const auto& func : funcs )
+	for ( auto& func : funcs )
 		{
+		const auto& f = func.Func();
+
+		auto& ofiles = analysis_options.only_files;
+		string fn = func.Body()->GetLocationInfo()->filename;
+
+		if ( ! func.ShouldSkip() && ! ofiles.empty() && files_with_conditionals.count(fn) > 0 )
+			{
+			if ( filenames_reported_as_skipped.count(fn) == 0 )
+				{
+				reporter->Warning(
+					"skipping compilation of files in %s due to presence of conditional code",
+					fn.c_str());
+				filenames_reported_as_skipped.insert(fn);
+				}
+
+			func.SetSkip(true);
+			}
+
 		if ( func.ShouldSkip() )
 			{
-			not_fully_compilable.insert(func.Func()->Name());
+			not_fully_compilable.insert(f->Name());
 			continue;
 			}
 
-		if ( func.Func()->Flavor() != FUNC_FLAVOR_FUNCTION )
-			// Can't be called directly.
-			continue;
-
 		const char* reason;
 		if ( IsCompilable(func, &reason) )
+			{
+			if ( f->Flavor() == FUNC_FLAVOR_FUNCTION )
+				// Note this as a callable compiled function.
 				compilable_funcs.insert(BodyName(func));
+			}
 		else
 			{
 			if ( reason && report_uncompilable )
-				fprintf(stderr, "%s cannot be compiled to C++ due to %s\n", func.Func()->Name(),
-				        reason);
-			not_fully_compilable.insert(func.Func()->Name());
+				fprintf(stderr, "%s cannot be compiled to C++ due to %s\n", f->Name(), reason);
+			not_fully_compilable.insert(f->Name());
 			}
 		}
 
@@ -105,7 +126,6 @@ void CPPCompile::Compile(bool report_uncompilable)
 		types.AddKey(tp, pfs.HashType(t));
 		}
 
-	// ### This doesn't work for -O add-C++
 	Emit("TypePtr types__CPP[%s];", Fmt(static_cast<int>(types.DistinctKeys().size())));
 
 	NL();

src/script_opt/CPP/README.md

@@ -162,17 +162,7 @@ about associated expressions/statements, making them hard to puzzle out.
 This could be fixed, but would add execution overhead in passing around
 the necessary strings / `Location` objects.
 
-* Subtle bugs can arise when compiling code that uses `@if` conditional
-compilation.  The compiled code will not directly use the wrong instance
-of a script body (one that differs due to the `@if` conditional having a
-different resolution at compile time versus later run-time).  However, if
-compiled code itself calls a function that has conditional code, the
-compiled code will always call the version of the function present during
-compilation, rather than the run-time version.  This problem can be fixed
-at the cost of making all function calls more expensive (perhaps a measure
-that requires an explicit flag to activate); or, when possible, by modifying
-the conditional code to check the condition at run-time rather than at
-compile-time.
+* To avoid subtle bugs, the compiler will refrain from compiling script elements (functions, hooks, event handlers) that include conditional code.  In addition, when using `--optimize-files` it will not compile any functions appearing in a source file that includes conditional code (even if it's not in a function body).
 
 * Code compiled with `-O gen-standalone-C++` will not execute any global
 statements when invoked using the "stand-in" script.  The right fix for

src/script_opt/Inline.cc

@@ -107,19 +107,29 @@ void Inliner::Analyze()
 		}
 
 	for ( auto& f : funcs )
+		{
+		const auto& func_ptr = f.FuncPtr();
+		const auto& func = func_ptr.get();
+		const auto& body = f.Body();
+
 		// Candidates are non-event, non-hook, non-recursive,
 		// non-compiled functions ... that don't use lambdas or when's,
 		// since we don't currently compute the closures/frame
 		// sizes for them correctly, and more fundamentally since
 		// we don't compile them and hence inlining them will
 		// make the parent non-compilable.
-		if ( f.Func()->Flavor() == FUNC_FLAVOR_FUNCTION &&
-		     non_recursive_funcs.count(f.Func()) > 0 && f.Profile()->NumLambdas() == 0 &&
-		     f.Profile()->NumWhenStmts() == 0 && f.Body()->Tag() != STMT_CPP )
-			inline_ables.insert(f.Func());
+		if ( should_analyze(func_ptr, body) && func->Flavor() == FUNC_FLAVOR_FUNCTION &&
+		     non_recursive_funcs.count(func) > 0 && f.Profile()->NumLambdas() == 0 &&
+		     f.Profile()->NumWhenStmts() == 0 && body->Tag() != STMT_CPP )
+			inline_ables.insert(func);
+		}
 
 	for ( auto& f : funcs )
 		{
+		const auto& func_ptr = f.FuncPtr();
+		const auto& func = func_ptr.get();
+		const auto& body = f.Body();
+
 		// Processing optimization: only spend time trying to inline f
 		// if we haven't marked it as inlineable.  This trades off a
 		// bunch of compilation load (inlining every single function,
@@ -128,7 +138,8 @@ void Inliner::Analyze()
 		// circumstances in which a Zeek function can be called
 		// not ultimately stemming from an event (such as global
 		// scripting, or expiration functions).
-		if ( inline_ables.count(f.Func()) == 0 )
+
+		if ( should_analyze(func_ptr, body) && inline_ables.count(func) == 0 )
 			InlineFunction(&f);
 		}
 	}

src/script_opt/ProfileFunc.cc

@@ -461,8 +461,6 @@ ProfileFuncs::ProfileFuncs(std::vector<FuncInfo>& funcs, is_compilable_pred pred
 
 		if ( ! pred || (*pred)(pf.get(), nullptr) )
 			MergeInProfile(pf.get());
-		else
-			f.SetSkip(true);
 
 		f.SetProfile(std::move(pf));
 		func_profs[f.Func()] = f.ProfilePtr();

src/script_opt/ScriptOpt.cc

@@ -39,14 +39,14 @@ static ScriptFuncPtr global_stmts;
 
 void analyze_func(ScriptFuncPtr f)
 	{
-	// Even if we're doing --optimize-only, we still track all functions
-	// here because the inliner will need the full list.
+	// Even if we're analyzing only a subset of the scripts, we still
+	// track all functions here because the inliner will need the full list.
 	funcs.emplace_back(f, f->GetScope(), f->CurrentBody(), f->CurrentPriority());
 	}
 
 const FuncInfo* analyze_global_stmts(Stmt* stmts)
 	{
-	// We ignore analysis_options.only_func - if it's in use, later
+	// We ignore analysis_options.only_{files,funcs} - if they're in use, later
 	// logic will keep this function from being compiled, but it's handy
 	// now to enter it into "funcs" so we have a FuncInfo to return.
 
@@ -65,6 +65,55 @@ const FuncInfo* analyze_global_stmts(Stmt* stmts)
 	return &funcs.back();
 	}
 
+void add_func_analysis_pattern(AnalyOpt& opts, const char* pat)
+	{
+	try
+		{
+		std::string full_pat = std::string("^(") + pat + ")$";
+		opts.only_funcs.emplace_back(std::regex(full_pat));
+		}
+	catch ( const std::regex_error& e )
+		{
+		reporter->FatalError("bad file analysis pattern: %s", pat);
+		}
+	}
+
+void add_file_analysis_pattern(AnalyOpt& opts, const char* pat)
+	{
+	try
+		{
+		std::string full_pat = std::string("^.*(") + pat + ").*$";
+		opts.only_files.emplace_back(std::regex(full_pat));
+		}
+	catch ( const std::regex_error& e )
+		{
+		reporter->FatalError("bad file analysis pattern: %s", pat);
+		}
+	}
+
+bool should_analyze(const ScriptFuncPtr& f, const StmtPtr& body)
+	{
+	auto& ofuncs = analysis_options.only_funcs;
+	auto& ofiles = analysis_options.only_files;
+
+	if ( ofiles.empty() && ofuncs.empty() )
+		return true;
+
+	auto fn = f->Name();
+
+	for ( auto& o : ofuncs )
+		if ( std::regex_match(fn, o) )
+			return true;
+
+	fn = body->GetLocationInfo()->filename;
+
+	for ( auto& o : ofiles )
+		if ( std::regex_match(fn, o) )
+			return true;
+
+	return false;
+	}
+
 static bool optimize_AST(ScriptFunc* f, std::shared_ptr<ProfileFunc>& pf,
                          std::shared_ptr<Reducer>& rc, ScopePtr scope, StmtPtr& body)
 	{
@@ -97,7 +146,7 @@ static void optimize_func(ScriptFunc* f, std::shared_ptr<ProfileFunc> pf, ScopeP
 	if ( reporter->Errors() > 0 )
 		return;
 
-	if ( analysis_options.only_func )
+	if ( analysis_options.dump_xform )
 		printf("Original: %s\n", obj_desc(body.get()).c_str());
 
 	if ( body->Tag() == STMT_CPP )
@@ -245,11 +294,11 @@ static void init_options()
 	if ( usage )
 		analysis_options.usage_issues = 1;
 
-	if ( ! analysis_options.only_func )
+	if ( analysis_options.only_funcs.empty() )
 		{
 		auto zo = getenv("ZEEK_ONLY");
 		if ( zo )
-			analysis_options.only_func = zo;
+			add_file_analysis_pattern(analysis_options, zo);
 		}
 
 	if ( analysis_options.gen_ZAM )
@@ -262,14 +311,8 @@ static void init_options()
 	if ( analysis_options.dump_ZAM )
 		analysis_options.gen_ZAM_code = true;
 
-	if ( analysis_options.only_func )
+	if ( ! analysis_options.only_funcs.empty() || ! analysis_options.only_files.empty() )
 		{
-		// Note, this comes after the statement above because for
-		// --optimize-only we don't necessarily want to go all
-		// the way to *generating* ZAM code, though we'll want to
-		// dump it *if* we generate it.
-		analysis_options.dump_xform = analysis_options.dump_ZAM = true;
-
 		if ( analysis_options.gen_ZAM_code || generating_CPP )
 			analysis_options.report_uncompilable = true;
 		}
@@ -278,8 +321,8 @@ static void init_options()
 	     ! generating_CPP )
 		reporter->FatalError("report-uncompilable requires generation of ZAM or C++");
 
-	if ( analysis_options.only_func || analysis_options.optimize_AST ||
-	     analysis_options.gen_ZAM_code || analysis_options.usage_issues > 0 )
+	if ( analysis_options.optimize_AST || analysis_options.gen_ZAM_code ||
+	     analysis_options.usage_issues > 0 )
 		analysis_options.activate = true;
 	}
 
@@ -388,54 +431,6 @@ static void generate_CPP(std::unique_ptr<ProfileFuncs>& pfs)
 	CPPCompile cpp(funcs, *pfs, gen_name, add, standalone, report);
 	}
 
-static void find_when_funcs(std::unique_ptr<ProfileFuncs>& pfs,
-                            std::unordered_set<const ScriptFunc*>& when_funcs)
-	{
-	// Figure out which functions either directly or indirectly
-	// appear in "when" clauses.
-
-	// Which functions we still need to analyze.
-	std::unordered_set<const ScriptFunc*> when_funcs_to_do;
-
-	for ( auto& f : funcs )
-		if ( f.Profile()->WhenCalls().size() > 0 )
-			{
-			when_funcs.insert(f.Func());
-
-			for ( auto& bf : f.Profile()->WhenCalls() )
-				{
-				ASSERT(pfs->FuncProf(bf));
-				when_funcs_to_do.insert(bf);
-				}
-			}
-
-	// Set of new functions to put on to-do list.  Separate from
-	// the to-do list itself so we don't modify it while iterating
-	// over it.
-	std::unordered_set<const ScriptFunc*> new_to_do;
-
-	while ( when_funcs_to_do.size() > 0 )
-		{
-		for ( auto& wf : when_funcs_to_do )
-			{
-			when_funcs.insert(wf);
-
-			for ( auto& wff : pfs->FuncProf(wf)->ScriptCalls() )
-				{
-				if ( when_funcs.count(wff) > 0 )
-					// We've already processed this
-					// function.
-					continue;
-
-				new_to_do.insert(wff);
-				}
-			}
-
-		when_funcs_to_do = new_to_do;
-		new_to_do.clear();
-		}
-	}
-
 static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)
 	{
 	if ( analysis_options.usage_issues > 0 && analysis_options.optimize_AST )
@@ -455,12 +450,6 @@ static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)
 
 	pfs = std::make_unique<ProfileFuncs>(funcs, nullptr, true);
 
-	// set of functions involved (directly or indirectly) in "when"
-	// clauses.
-	std::unordered_set<const ScriptFunc*> when_funcs;
-
-	find_when_funcs(pfs, when_funcs);
-
 	bool report_recursive = analysis_options.report_recursive;
 	std::unique_ptr<Inliner> inl;
 	if ( analysis_options.inliner )
@@ -501,13 +490,15 @@ static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)
 			}
 		}
 
+	bool did_one = false;
+
 	for ( auto& f : funcs )
 		{
 		auto func = f.Func();
 
-		if ( analysis_options.only_func )
+		if ( ! analysis_options.only_funcs.empty() || ! analysis_options.only_files.empty() )
 			{
-			if ( *analysis_options.only_func != func->Name() )
+			if ( ! should_analyze(f.FuncPtr(), f.Body()) )
 				continue;
 			}
 
@@ -519,7 +510,11 @@ static void analyze_scripts_for_ZAM(std::unique_ptr<ProfileFuncs>& pfs)
 		auto new_body = f.Body();
 		optimize_func(func, f.ProfilePtr(), f.Scope(), new_body);
 		f.SetBody(new_body);
+		did_one = true;
 		}
+
+	if ( ! did_one )
+		reporter->FatalError("no matching functions/files for -O ZAM");
 	}
 
 void analyze_scripts()
@@ -532,23 +527,30 @@ void analyze_scripts()
 		did_init = true;
 		}
 
+	auto& ofuncs = analysis_options.only_funcs;
+	auto& ofiles = analysis_options.only_files;
+
 	if ( ! analysis_options.activate && ! analysis_options.inliner && ! generating_CPP &&
 	     ! analysis_options.report_CPP && ! analysis_options.use_CPP )
-		// No work to do, avoid profiling overhead.
-		return;
+		{ // No work to do, avoid profiling overhead.
+		if ( ! ofuncs.empty() )
+			reporter->FatalError("--optimize-funcs used but no optimization specified");
+		if ( ! ofiles.empty() )
+			reporter->FatalError("--optimize-files used but no optimization specified");
+
+		return;
+		}
+
+	bool have_one_to_do = false;
 
-	if ( analysis_options.gen_CPP )
-		{
-		if ( analysis_options.only_func )
-			{ // deactivate all functions except the target one
 	for ( auto& func : funcs )
-				{
-				auto fn = func.Func()->Name();
-				if ( *analysis_options.only_func != fn )
+		if ( should_analyze(func.FuncPtr(), func.Body()) )
+			have_one_to_do = true;
+		else
 			func.SetSkip(true);
-				}
-			}
-		}
+
+	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.
@@ -568,6 +570,9 @@ void analyze_scripts()
 
 	if ( generating_CPP )
 		{
+		if ( analysis_options.gen_ZAM )
+			reporter->FatalError("-O ZAM and -O gen-C++ conflict");
+
 		generate_CPP(pfs);
 		exit(0);
 		}

src/script_opt/ScriptOpt.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <optional>
+#include <regex>
 #include <string>
 
 #include "zeek/Expr.h"
@@ -24,9 +25,14 @@ namespace zeek::detail
 struct AnalyOpt
 	{
 
-	// If non-nil, then only analyze the given function/event/hook.
+	// If non-nil, then only analyze function/event/hook(s) whose names
+	// match one of the given regular expressions.
+	//
 	// Applies to both ZAM and C++.
-	std::optional<std::string> only_func;
+	std::vector<std::regex> only_funcs;
+
+	// Same, but for the filenames where the function is found.
+	std::vector<std::regex> only_files;
 
 	// For a given compilation target, report functions that can't
 	// be compiled.
@@ -68,7 +74,7 @@ struct AnalyOpt
 
 	// If true, dump out transformed code: the results of reducing
 	// interpreted scripts, and, if optimize is set, of then optimizing
-	// them.  Always done if only_func is set.
+	// them.
 	bool dump_xform = false;
 
 	// If true, dump out the use-defs for each analyzed function.
@@ -166,6 +172,16 @@ extern void analyze_func(ScriptFuncPtr f);
 // be Invoked, or its body executed directly, to execute the statements.
 extern const FuncInfo* analyze_global_stmts(Stmt* stmts);
 
+// Add a pattern to the "only_funcs" list.
+extern void add_func_analysis_pattern(AnalyOpt& opts, const char* pat);
+
+// Add a pattern to the "only_files" list.
+extern void add_file_analysis_pattern(AnalyOpt& opts, const char* pat);
+
+// True if the given script function & body should be analyzed; otherwise
+// it should be skipped.
+extern bool should_analyze(const ScriptFuncPtr& f, const StmtPtr& body);
+
 // Analyze all of the parsed scripts collectively for optimization.
 extern void analyze_scripts();