Merge remote-tracking branch 'origin/topic/jsiwek/gh-1122'

* origin/topic/jsiwek/gh-1122:
  GH-165: Fix global initializations that indirectly use builtin types
  Improve how primary/top-level BIFs get initialized
  GH-1122: Allow initializing globals with calls to subdir BIFs
  GH-1122: Improve error for global record initialization exceptions

src/Func.cc

@@ -74,6 +74,7 @@ extern	RETSIGTYPE sig_handler(int signo);
 namespace zeek::detail {
 std::vector<CallInfo> call_stack;
 bool did_builtin_init = false;
+std::vector<void (*)()> bif_initializers;
 static const std::pair<bool, zeek::ValPtr> empty_hook_result(false, nullptr);
 } // namespace zeek::detail
 
@@ -901,6 +902,38 @@ void emit_builtin_exception(const char* msg, Obj* arg)
 	emit_builtin_error_common(msg, arg, true);
 	}
 
+void init_primary_bifs()
+	{
+	if ( did_builtin_init )
+		return;
+
+	ProcStats = id::find_type<RecordType>("ProcStats");
+	NetStats = id::find_type<RecordType>("NetStats");
+	MatcherStats = id::find_type<RecordType>("MatcherStats");
+	ConnStats = id::find_type<RecordType>("ConnStats");
+	ReassemblerStats = id::find_type<RecordType>("ReassemblerStats");
+	DNSStats = id::find_type<RecordType>("DNSStats");
+	GapStats = id::find_type<RecordType>("GapStats");
+	EventStats = id::find_type<RecordType>("EventStats");
+	TimerStats = id::find_type<RecordType>("TimerStats");
+	FileAnalysisStats = id::find_type<RecordType>("FileAnalysisStats");
+	ThreadStats = id::find_type<RecordType>("ThreadStats");
+	BrokerStats = id::find_type<RecordType>("BrokerStats");
+	ReporterStats = id::find_type<RecordType>("ReporterStats");
+
+	var_sizes = id::find_type("var_sizes")->AsTableType();
+
+#include "zeek.bif.func_init"
+#include "stats.bif.func_init"
+#include "reporter.bif.func_init"
+#include "strings.bif.func_init"
+#include "option.bif.func_init"
+#include "supervisor.bif.func_init"
+
+	init_builtin_types();
+	did_builtin_init = true;
+	}
+
 } // namespace detail
 
 
@@ -935,39 +968,3 @@ void builtin_error(const char* msg, zeek::Obj* arg)
 	{
 	zeek::emit_builtin_error(msg, arg);
 	}
-
-#include "__all__.bif.cc" // Autogenerated for compiling in the bif_target() code.
-#include "__all__.bif.register.cc" // Autogenerated for compiling in the bif_target() code.
-
-void init_builtin_funcs()
-	{
-	ProcStats = zeek::id::find_type<zeek::RecordType>("ProcStats");
-	NetStats = zeek::id::find_type<zeek::RecordType>("NetStats");
-	MatcherStats = zeek::id::find_type<zeek::RecordType>("MatcherStats");
-	ConnStats = zeek::id::find_type<zeek::RecordType>("ConnStats");
-	ReassemblerStats = zeek::id::find_type<zeek::RecordType>("ReassemblerStats");
-	DNSStats = zeek::id::find_type<zeek::RecordType>("DNSStats");
-	GapStats = zeek::id::find_type<zeek::RecordType>("GapStats");
-	EventStats = zeek::id::find_type<zeek::RecordType>("EventStats");
-	TimerStats = zeek::id::find_type<zeek::RecordType>("TimerStats");
-	FileAnalysisStats = zeek::id::find_type<zeek::RecordType>("FileAnalysisStats");
-	ThreadStats = zeek::id::find_type<zeek::RecordType>("ThreadStats");
-	BrokerStats = zeek::id::find_type<zeek::RecordType>("BrokerStats");
-	ReporterStats = zeek::id::find_type<zeek::RecordType>("ReporterStats");
-
-	var_sizes = zeek::id::find_type("var_sizes")->AsTableType();
-
-#include "zeek.bif.func_init"
-#include "stats.bif.func_init"
-#include "reporter.bif.func_init"
-#include "strings.bif.func_init"
-#include "option.bif.func_init"
-#include "supervisor.bif.func_init"
-
-	zeek::detail::did_builtin_init = true;
-	}
-
-void init_builtin_funcs_subdirs()
-	{
-#include "__all__.bif.init.cc" // Autogenerated for compiling in the bif_target() code.
-	}

src/Func.h

@@ -276,6 +276,16 @@ extern std::vector<CallInfo> call_stack;
 
 // This is set to true after the built-in functions have been initialized.
 extern bool did_builtin_init;
+extern std::vector<void (*)()> bif_initializers;
+extern void init_primary_bifs();
+
+inline void run_bif_initializers()
+	{
+	for ( const auto& bi : bif_initializers )
+		bi();
+
+	bif_initializers = {};
+	}
 
 extern void emit_builtin_exception(const char* msg);
 extern void emit_builtin_exception(const char* msg, const ValPtr& arg);
@@ -301,12 +311,6 @@ using function_ingredients [[deprecated("Remove in v4.1. Use zeek::detail::funct
 constexpr auto check_built_in_call [[deprecated("Remove in v4.1. Use zeek::detail::check_built_in_call.")]] = zeek::detail::check_built_in_call;
 constexpr auto render_call_stack [[deprecated("Remove in v4.1. Use zeek::render_call_stack.")]] = zeek::render_call_stack;
 
-// TODO: these are still here because of how all of the bif code gets included in Func.c. There could be a
-// renamed version inside the namespace, but the way that the code gets included complicates the matter. It
-// might need to be revisited after everything is namespaced everywhere else.
-void init_builtin_funcs();
-void init_builtin_funcs_subdirs();
-
 // TODO: do call_stack and did_builtin_init need to be aliased?
 
 // These have to be implemented as actual methods due to function overloading breaking the use of aliases.

src/ID.cc

@@ -90,7 +90,7 @@ FuncPtr id::find_func(std::string_view name)
 	return v->AsFuncPtr();
 	}
 
-void id::detail::init()
+void id::detail::init_types()
 	{
 	conn_id = id::find_type<RecordType>("conn_id");
 	endpoint = id::find_type<RecordType>("endpoint");

src/ID.h

@@ -272,7 +272,7 @@ extern VectorTypePtr index_vec;
 
 namespace detail {
 
-void init();
+void init_types();
 
 } // namespace detail
 } // namespace zeek::id

src/NetVar.cc

@@ -207,11 +207,15 @@ static void bif_init_event_handlers()
 static void bif_init_net_var()
 	{
 #include "const.bif.netvar_init"
-#include "types.bif.netvar_init"
 #include "reporter.bif.netvar_init"
 #include "supervisor.bif.netvar_init"
 	}
 
+static void init_bif_types()
+	{
+#include "types.bif.netvar_init"
+	}
+
 #include "const.bif.netvar_def"
 #include "types.bif.netvar_def"
 #include "event.bif.netvar_def"
@@ -240,11 +244,16 @@ void init_general_global_var()
 
 extern void zeek_legacy_netvar_init();
 
+void init_builtin_types()
+	{
+	init_bif_types();
+	id::detail::init_types();
+	}
+
 void init_net_var()
 	{
 	bif_init_net_var();
 
-	id::detail::init();
 	zeek_legacy_netvar_init();
 
 	ignore_checksums = id::find_val("ignore_checksums")->AsBool();

src/NetVar.h

@@ -103,6 +103,7 @@ extern void init_general_global_var();
 
 extern void init_event_handlers();
 extern void init_net_var();
+extern void init_builtin_types();
 
 } // namespace zeek::detail
 

src/Val.cc

@@ -2886,7 +2886,22 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields) : Val(std::move(t))
 		{
 		detail::Attributes* a = rt->FieldDecl(i)->attrs.get();
 		detail::Attr* def_attr = a ? a->Find(detail::ATTR_DEFAULT).get() : nullptr;
-		auto def = def_attr ? def_attr->GetExpr()->Eval(nullptr) : nullptr;
+		ValPtr def;
+
+		if ( def_attr )
+			try
+				{
+				def = def_attr->GetExpr()->Eval(nullptr);
+				}
+			catch ( InterpreterException& )
+				{
+				if ( run_state::is_parsing )
+					parse_time_records[rt].pop_back();
+
+				delete AsNonConstRecord();
+				throw;
+				}
+
 		const auto& type = rt->FieldDecl(i)->type;
 
 		if ( def && type->Tag() == TYPE_RECORD &&

src/Var.cc

@@ -252,7 +252,15 @@ static void make_var(const IDPtr& id, TypePtr t, InitClass c, ExprPtr init,
 
 			if ( t->Tag() == TYPE_RECORD )
 				{
-				aggr = make_intrusive<RecordVal>(cast_intrusive<RecordType>(t));
+				try
+					{
+					aggr = make_intrusive<RecordVal>(cast_intrusive<RecordType>(t));
+					}
+				catch ( InterpreterException& )
+					{
+					id->Error("initialization failed");
+					return;
+					}
 
 				if ( init && t )
 					// Have an initialization and type is not deduced.

src/scan.l

@@ -842,16 +842,6 @@ int yywrap()
 	if ( zeek::reporter->Errors() > 0 )
 		return 1;
 
-	if ( ! did_builtin_init && file_stack.length() == 1 )
-		{
-		// ### This is a gross hack - we know that the first file
-		// we parse is init-bare.zeek, and after it it's safe to initialize
-		// the built-ins.  Furthermore, we want to initialize the
-		// built-in's *right* after parsing bro.init, so that other
-		// source files can use built-in's when initializing globals.
-		init_builtin_funcs();
-		}
-
 	yy_delete_buffer(YY_CURRENT_BUFFER);
 
 	if ( file_stack.length() > 0 )

src/zeek-setup.cc

@@ -62,6 +62,7 @@ extern "C" {
 #include "broker/Manager.h"
 
 #include "binpac_zeek.h"
+#include "module_util.h"
 
 #include "3rdparty/sqlite3.h"
 
@@ -388,8 +389,7 @@ static std::vector<std::string> get_script_signature_files()
 	return rval;
 	}
 
-SetupResult setup(int argc, char** argv,
-                                Options* zopts)
+SetupResult setup(int argc, char** argv, Options* zopts)
 	{
 	ZEEK_LSAN_DISABLE();
 	std::set_new_handler(bro_new_handler);
@@ -631,6 +631,19 @@ SetupResult setup(int argc, char** argv,
 	HeapLeakChecker::Disabler disabler;
 #endif
 
+	auto ipbid = install_ID("__init_primary_bifs", GLOBAL_MODULE_NAME,
+	                        true, true);
+	auto ipbft = make_intrusive<FuncType>(make_intrusive<RecordType>(nullptr),
+	                                      base_type(TYPE_BOOL),
+	                                      FUNC_FLAVOR_FUNCTION);
+	ipbid->SetType(std::move(ipbft));
+	auto init_bifs = [](Frame* frame, const Args* args) -> BifReturnVal
+		{
+		init_primary_bifs();
+		return val_mgr->True();
+		};
+	auto ipbb = make_intrusive<BuiltinFunc>(init_bifs, ipbid->Name(), false);
+
 	run_state::is_parsing = true;
 	yyparse();
 	run_state::is_parsing = false;
@@ -640,7 +653,7 @@ SetupResult setup(int argc, char** argv,
 
 	init_general_global_var();
 	init_net_var();
-	init_builtin_funcs_subdirs();
+	run_bif_initializers();
 
 	// Must come after plugin activation (and also after hash
 	// initialization).

src/zeek.bif

@@ -5046,6 +5046,24 @@ function match_signatures%(c: connection, pattern_type: int, s: string,
 	return zeek::val_mgr->True();
 	%}
 
+%%{
+// Autogenerated from CMake bif_target()
+#include "__all__.bif.cc"
+#include "__all__.bif.register.cc"
+
+static void init_secondary_bifs()
+	{
+	#include "__all__.bif.init.cc"
+	}
+%%}
+
+## An internal function that helps initialize BIFs.
+function __init_secondary_bifs%(%): bool
+	%{
+	init_secondary_bifs();
+	return zeek::val_mgr->True();
+	%}
+
 # ===========================================================================
 #
 #                            Anonymization Functions