From 0a69b87f03b18f6c5b4e6952912b5390c9e698b1 Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@ncsa.illinois.edu>
Date: Thu, 17 Jan 2013 15:21:50 -0600
Subject: [PATCH] Fix uninitialized locals in event/hook handlers from having a
 value.

Since values for local variables are referenced by offset within a Frame
(not by identifier name), and event/hook handler bodies share a common
Frame, the value offsets for local variables in different handlers may
overlap.  This meant locals in a handler without an initialization may
actually end up referring to the value of a previous handler's local
that has the same Frame offset.  When executing the body, that can
possibly result in a type-conflict error or give give unexpected
results instead of a "use of uninitialized value" error.

This patch makes it so uninitialized locals do always refer to a null
value before executing the body of a event/hook handler, so that using
them without assigning a value within the body will connsistently give
a "use of uninitialized value" error.

Addresses #932.
---
 src/Stmt.cc                                   | 16 +++++++++----
 src/Var.cc                                    |  5 +---
 .../Baseline/language.uninitialized-local/out |  2 ++
 .../btest/language/uninitialized-local.bro    | 23 +++++++++++++++++++
 4 files changed, 38 insertions(+), 8 deletions(-)
 create mode 100644 testing/btest/Baseline/language.uninitialized-local/out
 create mode 100644 testing/btest/language/uninitialized-local.bro

diff --git a/src/Stmt.cc b/src/Stmt.cc
index 2cd7117ddb..d7052d0b66 100644
--- a/src/Stmt.cc
+++ b/src/Stmt.cc
@@ -1789,13 +1789,21 @@ Val* InitStmt::Exec(Frame* f, stmt_flow_type& flow) const
 		ID* aggr = (*inits)[i];
 		BroType* t = aggr->Type();
 
-		Val* v;
-		if ( t->Tag() == TYPE_RECORD )
+		Val* v = 0;
+
+		switch ( t->Tag() ) {
+		case TYPE_RECORD:
 			v = new RecordVal(t->AsRecordType());
-		else if ( aggr->Type()->Tag() == TYPE_VECTOR )
+			break;
+		case TYPE_VECTOR:
 			v = new VectorVal(t->AsVectorType());
-		else
+			break;
+		case TYPE_TABLE:
 			v = new TableVal(t->AsTableType(), aggr->Attrs());
+			break;
+		default:
+			break;
+		}
 
 		f->SetElement(aggr->Offset(), v);
 		}
diff --git a/src/Var.cc b/src/Var.cc
index b4d76097d3..0aadd93e92 100644
--- a/src/Var.cc
+++ b/src/Var.cc
@@ -243,10 +243,7 @@ Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init,
 
 	else
 		{
-		if ( t->Tag() == TYPE_RECORD || t->Tag() == TYPE_TABLE ||
-		     t->Tag() == TYPE_VECTOR )
-			current_scope()->AddInit(id);
-
+		current_scope()->AddInit(id);
 		return new NullStmt;
 		}
 	}
diff --git a/testing/btest/Baseline/language.uninitialized-local/out b/testing/btest/Baseline/language.uninitialized-local/out
new file mode 100644
index 0000000000..f803415fe6
--- /dev/null
+++ b/testing/btest/Baseline/language.uninitialized-local/out
@@ -0,0 +1,2 @@
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.uninitialized-local/uninitialized-local.bro, line 16: value used but not set (my_string)
+Continuing
diff --git a/testing/btest/language/uninitialized-local.bro b/testing/btest/language/uninitialized-local.bro
new file mode 100644
index 0000000000..e1cc178c0a
--- /dev/null
+++ b/testing/btest/language/uninitialized-local.bro
@@ -0,0 +1,23 @@
+# @TEST-EXEC: bro -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+event testit() &priority=10
+	{
+	local my_count: count = 10;
+	}
+
+event testit()
+	{
+	# my_string's value occupies same Frame offset as my_count's from above
+	# handler, but execution of this handler body should still "initialize"
+	# it to a null value instead of referring to a left-over value of my_count.
+	local my_string: string;
+	local my_vector: vector of string;
+	my_vector[0] = my_string;
+	print "Continuing";
+	}
+
+event bro_init()
+	{
+	event testit();
+	}