From cdbe459f206e42a784fd8df2617e9eabe231c77c Mon Sep 17 00:00:00 2001
From: Jon Siwek <jsiwek@illinois.edu>
Date: Tue, 2 Dec 2014 12:30:46 -0600
Subject: [PATCH] Make using local IDs in @if directives an error.

Addresses BIT-1296.
---
 src/scan.l                                    | 40 +++++++++++++++++-
 .../btest/Baseline/language.at-if-invalid/out |  4 ++
 testing/btest/language/at-if-invalid.bro      | 41 +++++++++++++++++++
 testing/btest/language/at-if.bro              | 12 +++++-
 4 files changed, 94 insertions(+), 3 deletions(-)
 create mode 100644 testing/btest/Baseline/language.at-if-invalid/out
 create mode 100644 testing/btest/language/at-if-invalid.bro

diff --git a/src/scan.l b/src/scan.l
index 9ab830c4f1..a804004d66 100644
--- a/src/scan.l
+++ b/src/scan.l
@@ -26,6 +26,7 @@
 #include "Reporter.h"
 #include "RE.h"
 #include "Net.h"
+#include "Traverse.h"
 
 #include "analyzer/Analyzer.h"
 #include "broxygen/Manager.h"
@@ -615,11 +616,48 @@ void end_RE()
 	BEGIN(INITIAL);
 	}
 
+struct LocalNameFinder : public TraversalCallback {
+
+	LocalNameFinder()
+		{}
+
+	virtual TraversalCode PreExpr(const Expr* expr)
+		{
+		if ( expr->Tag() != EXPR_NAME )
+			return TC_CONTINUE;
+
+		const NameExpr* name_expr = static_cast<const NameExpr*>(expr);
+
+		if ( name_expr->Id()->IsGlobal() )
+			return TC_CONTINUE;
+
+		local_names.push_back(name_expr);
+		return TC_CONTINUE;
+		}
+
+	std::vector<const NameExpr*> local_names;
+};
+
 void do_atif(Expr* expr)
 	{
 	++current_depth;
 
-	Val* val = expr->Eval(0);
+	LocalNameFinder cb;
+	expr->Traverse(&cb);
+	Val* val = 0;
+
+	if ( cb.local_names.empty() )
+		val = expr->Eval(0);
+	else
+		for ( size_t i = 0; i < cb.local_names.size(); ++i )
+			cb.local_names[i]->Error("referencing a local name in @if");
+
+	if ( ! val )
+		{
+		expr->Error("invalid expression in @if");
+		return;
+		}
+
 	if ( ! val->AsBool() )
 		{
 		if_stack.append(current_depth);
diff --git a/testing/btest/Baseline/language.at-if-invalid/out b/testing/btest/Baseline/language.at-if-invalid/out
new file mode 100644
index 0000000000..63b93a3cf8
--- /dev/null
+++ b/testing/btest/Baseline/language.at-if-invalid/out
@@ -0,0 +1,4 @@
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.at-if-invalid/at-if-invalid.bro, line 28: referencing a local name in @if (xyz)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.at-if-invalid/at-if-invalid.bro, line 28: invalid expression in @if (F && foo(xyz))
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.at-if-invalid/at-if-invalid.bro, line 36: referencing a local name in @if (local_true_condition)
+error in /Users/jsiwek/Projects/bro/bro/testing/btest/.tmp/language.at-if-invalid/at-if-invalid.bro, line 36: invalid expression in @if (T && TRUE_CONDITION && local_true_condition)
diff --git a/testing/btest/language/at-if-invalid.bro b/testing/btest/language/at-if-invalid.bro
new file mode 100644
index 0000000000..1be2b94304
--- /dev/null
+++ b/testing/btest/language/at-if-invalid.bro
@@ -0,0 +1,41 @@
+# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1
+# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out
+
+function foo(c: count): bool
+	{ return c == 42 ? T : F; }
+
+global TRUE_CONDITION = T;
+
+event bro_init()
+	{
+	local xyz = 0;
+	local local_true_condition = T;
+
+	@if ( F )
+		xyz += 1;
+	@endif
+
+	@if ( foo(0) )
+		xyz += 1;
+	@endif
+
+	@if ( T && foo(42) )
+		xyz += 2;
+	@endif
+
+	xyz = 0;
+
+	@if ( F && foo(xyz) )
+		xyz += 1;
+	@else
+		xyz += 2;
+	@endif
+
+	xyz = 0;
+
+	@if ( T && TRUE_CONDITION && local_true_condition )
+		xyz += 1;
+	@else
+		xyz += 2;
+	@endif
+	}
diff --git a/testing/btest/language/at-if.bro b/testing/btest/language/at-if.bro
index 0a3e87adfa..dddf9a22a5 100644
--- a/testing/btest/language/at-if.bro
+++ b/testing/btest/language/at-if.bro
@@ -6,6 +6,10 @@ function test_case(msg: string, expect: bool)
         print fmt("%s (%s)", msg, expect ? "PASS" : "FAIL");
         }
 
+function foo(c: count): bool
+	{ return c == 42 ? T : F; }
+
+global TRUE_CONDITION = T;
 
 event bro_init()
 {
@@ -17,7 +21,11 @@ event bro_init()
 		xyz += 1;
 	@endif
 
-	@if ( T )
+	@if ( foo(0) )
+		xyz += 1;
+	@endif
+
+	@if ( T && foo(42) )
 		xyz += 2;
 	@endif
 
@@ -37,7 +45,7 @@ event bro_init()
 
 	xyz = 0;
 
-	@if ( T )
+	@if ( T && TRUE_CONDITION )
 		xyz += 1;
 	@else
 		xyz += 2;