Merge remote-tracking branch 'origin/topic/jsiwek/bit-1909'

I've moved the resizing loop into a static method of RecordVal.

* origin/topic/jsiwek/bit-1909:
  BIT-1909: fix invalid redef'd record field accesses

aux/binpac

@@ -1 +1 @@
-Subproject commit e1d8b72fa24406e832c610d752d299f9c460d4e7
+Subproject commit a4bd60ec2e6958506b4a07cf0a57f006254ee4f2

aux/bro-aux

@@ -1 +1 @@
-Subproject commit 6db18be3e1a5cc6edbe83abb79ca3ed2cd623b41
+Subproject commit f7cc11d97b3298bbfc5d01ff544e871b67f0fc5a

aux/broccoli

@@ -1 +1 @@
-Subproject commit 64904c9f6c93d815fbe8f6226d3e1a9f0aa7c1cf
+Subproject commit 22a35f33786b2ede5f3efe7e2b24237a4fe974fb

aux/broctl

@@ -1 +1 @@
-Subproject commit 7991b77bf4e2f80d20152e87f492c59070305031
+Subproject commit 60eeea9f98455ea613bcfc02e09f2c9665c97f58

aux/broker

@@ -1 +1 @@
-Subproject commit 338b65a84da66d0059276814de8807ab9ee9b105
+Subproject commit 066fb75d1ea46e919a2d9c56faff1e605c64756c

cmake

@@ -1 +1 @@
-Subproject commit 70fc7406f4baa87da9362c2b80ee6bbcdb242e0a
+Subproject commit c40d8f9831b69259bfa930a64662d91cfdbf97bf

src/Net.cc

@@ -61,6 +61,7 @@ double bro_start_time = 0.0; // time Bro started.
 double bro_start_network_time;	// timestamp of first packet
 double last_watchdog_proc_time = 0.0;	// value of above during last watchdog
 bool terminating = false;	// whether we're done reading and finishing up
+bool is_parsing = false;
 
 const Packet *current_pkt = 0;
 int current_dispatched = 0;

src/Net.h

@@ -70,6 +70,9 @@ extern bool terminating;
 // True if the remote serializer is to be activated.
 extern bool using_communication;
 
+// True if Bro is currently parsing scripts.
+extern bool is_parsing;
+
 extern const Packet* current_pkt;
 extern int current_dispatched;
 extern double current_timestamp;

src/Val.cc

@@ -2722,6 +2722,8 @@ unsigned int TableVal::MemoryAllocation() const
 		+ table_hash->MemoryAllocation();
 	}
 
+vector<RecordVal*> RecordVal::parse_time_records;
+
 RecordVal::RecordVal(RecordType* t) : MutableVal(t)
 	{
 	origin = 0;
@@ -2767,6 +2769,12 @@ RecordVal::RecordVal(RecordType* t) : MutableVal(t)
 		vl->append(def ? def->Ref() : 0);
 
 		Unref(def);
+
+		if ( is_parsing )
+			{
+			parse_time_records.emplace_back(this);
+			Ref();
+			}
 		}
 	}
 
@@ -2832,6 +2840,29 @@ Val* RecordVal::LookupWithDefault(int field) const
 	return record_type->FieldDefault(field);
 	}
 
+void RecordVal::ResizeParseTimeRecords()
+	{
+	for ( auto& rv : parse_time_records )
+		{
+		auto vs = rv->val.val_list_val;
+		auto rt = rv->record_type;
+		auto current_length = vs->length();
+		auto required_length = rt->NumFields();
+
+		if ( required_length > current_length )
+			{
+			vs->resize(required_length);
+
+			for ( auto i = current_length; i < required_length; ++i )
+				vs->replace(i, nullptr);
+			}
+
+		Unref(rv);
+		}
+
+	parse_time_records.clear();
+	}
+
 Val* RecordVal::Lookup(const char* field, bool with_default) const
 	{
 	int idx = record_type->FieldOffset(field);

src/Val.h

@@ -965,6 +965,11 @@ public:
 	unsigned int MemoryAllocation() const override;
 	void DescribeReST(ODesc* d) const override;
 
+	// Extend the underlying arrays of record instances created during
+	// parsing to match the number of fields in the record type (they may
+	// mismatch as a result of parse-time record type redefinitions.
+	static void ResizeParseTimeRecords();
+
 protected:
 	friend class Val;
 	RecordVal()	{}
@@ -976,6 +981,8 @@ protected:
 
 	RecordType* record_type;
 	BroObj* origin;
+
+	static vector<RecordVal*> parse_time_records;
 };
 
 class EnumVal : public Val {

src/main.cc

@@ -854,7 +854,11 @@ int main(int argc, char** argv)
 	HeapLeakChecker::Disabler disabler;
 #endif
 
+	is_parsing = true;
 	yyparse();
+	is_parsing = false;
+
+	RecordVal::ResizeParseTimeRecords();
 
 	init_general_global_var();
 	init_net_var();

testing/btest/Baseline/language.record-redef-after-init/output

@@ -0,0 +1,10 @@
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+[a=runtime, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=OPTQ]
+[a=local, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=OPTQ]
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=<uninitialized>, q=<uninitialized>]
+newp
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=newp, q=<uninitialized>]
+OPTQ
+our value
+[a=redef, d=<uninitialized>, e=<uninitialized>, f=<uninitialized>, g=<uninitialized>, h=<uninitialized>, i=<uninitialized>, j=<uninitialized>, k=<uninitialized>, l=<uninitialized>, m=<uninitialized>, n=<uninitialized>, o=<uninitialized>, p=newp, q=our value]

testing/btest/language/record-redef-after-init.bro

@@ -0,0 +1,52 @@
+# @TEST-EXEC: bro -b %INPUT >output
+# @TEST-EXEC: btest-diff output
+
+type myrec: record {
+    a: string;
+};
+
+const mr = myrec($a = "init") &redef;
+
+redef mr = myrec($a = "redef");
+
+# Many fields may help ensure out-of-bounds reference failures
+redef record myrec += {
+    d: string &optional;
+    e: string &optional;
+    f: string &optional;
+    g: string &optional;
+    h: string &optional;
+    i: string &optional;
+    j: string &optional;
+    k: string &optional;
+    l: string &optional;
+    m: string &optional;
+    n: string &optional;
+    o: string &optional;
+    p: string &optional;
+    q: string &default="OPTQ";
+};
+
+print mr;                        # original 'myrec' type with updated a value
+print myrec($a = "runtime");     # check we get new defaults
+
+local mr2 = myrec($a = "local");
+print mr2;
+
+mr2 = mr;      # Copying should do the right thing
+print mr2;
+
+local mr3: myrec = mr; # Initializing should do the right thing
+print mr3;
+
+if ( mr?$q )    # the test that did not work properly
+    {
+    print mr$q; # accessed invalid memory location
+    }
+mr$p = "newp";  # Assignment updates mr as much as needed
+print mr$p;
+print mr;
+print mr$q;
+mr$q = "our value";
+print mr$q;
+print mr;