From 290c2a0b4df2db38ade684cf386a5c9b6b271d9e Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 13 Dec 2012 15:05:29 -0600 Subject: [PATCH] Make const variables actually constant. Addresses #922. Both local and global variables declared with "const" could be modified, but now expressions that would modify them should generate an error message at parse-time. --- scripts/base/frameworks/notice/cluster.bro | 6 +- src/Expr.cc | 15 +++- src/Expr.h | 3 +- src/Var.cc | 4 +- .../Baseline/language.const/invalid.stderr | 13 +++ .../Baseline/language.const/invalid.stdout | 0 .../Baseline/language.const/valid.stderr | 0 .../Baseline/language.const/valid.stdout | 10 +++ testing/btest/language/const.bro | 79 +++++++++++++++++++ 9 files changed, 120 insertions(+), 10 deletions(-) create mode 100644 testing/btest/Baseline/language.const/invalid.stderr create mode 100644 testing/btest/Baseline/language.const/invalid.stdout create mode 100644 testing/btest/Baseline/language.const/valid.stderr create mode 100644 testing/btest/Baseline/language.const/valid.stdout create mode 100644 testing/btest/language/const.bro diff --git a/scripts/base/frameworks/notice/cluster.bro b/scripts/base/frameworks/notice/cluster.bro index 3ee113acf3..e812c3fdca 100644 --- a/scripts/base/frameworks/notice/cluster.bro +++ b/scripts/base/frameworks/notice/cluster.bro @@ -21,12 +21,10 @@ redef Cluster::manager2worker_events += /Notice::begin_suppression/; redef Cluster::worker2manager_events += /Notice::cluster_notice/; @if ( Cluster::local_node_type() != Cluster::MANAGER ) + # The notice policy is completely handled by the manager and shouldn't be # done by workers or proxies to save time for packet processing. -event bro_init() &priority=11 - { - Notice::policy = table(); - } +redef Notice::policy = table(); event Notice::begin_suppression(n: Notice::Info) { diff --git a/src/Expr.cc b/src/Expr.cc index 7995d5d495..3a4e8add70 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -229,9 +229,10 @@ bool Expr::DoUnserialize(UnserialInfo* info) } -NameExpr::NameExpr(ID* arg_id) : Expr(EXPR_NAME) +NameExpr::NameExpr(ID* arg_id, bool const_init) : Expr(EXPR_NAME) { id = arg_id; + in_const_init = const_init; SetType(id->Type()->Ref()); EventHandler* h = event_registry->Lookup(id->Name()); @@ -287,6 +288,9 @@ Expr* NameExpr::MakeLvalue() if ( id->AsType() ) ExprError("Type name is not an lvalue"); + if ( id->IsConst() && ! in_const_init ) + ExprError("const is not a modifiable lvalue"); + return new RefExpr(this); } @@ -337,9 +341,11 @@ bool NameExpr::DoSerialize(SerialInfo* info) const // Write out just the name of the function if requested. if ( info->globals_as_names && id->IsGlobal() ) - return SERIALIZE('n') && SERIALIZE(id->Name()); + return SERIALIZE('n') && SERIALIZE(id->Name()) && + SERIALIZE(in_const_init); else - return SERIALIZE('f') && id->Serialize(info); + return SERIALIZE('f') && id->Serialize(info) && + SERIALIZE(in_const_init); } bool NameExpr::DoUnserialize(UnserialInfo* info) @@ -370,6 +376,9 @@ bool NameExpr::DoUnserialize(UnserialInfo* info) if ( ! id ) return false; + if ( ! UNSERIALIZE(&in_const_init) ) + return false; + return true; } diff --git a/src/Expr.h b/src/Expr.h index afdf02c124..ea17c735b5 100644 --- a/src/Expr.h +++ b/src/Expr.h @@ -198,7 +198,7 @@ protected: class NameExpr : public Expr { public: - NameExpr(ID* id); + NameExpr(ID* id, bool const_init = false); ~NameExpr(); ID* Id() const { return id; } @@ -220,6 +220,7 @@ protected: DECLARE_SERIAL(NameExpr); ID* id; + bool in_const_init; }; class ConstExpr : public Expr { diff --git a/src/Var.cc b/src/Var.cc index 2e9fdbe946..b4d76097d3 100644 --- a/src/Var.cc +++ b/src/Var.cc @@ -210,7 +210,6 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init, // defined. Func* f = new BroFunc(id, 0, 0, 0, 0); id->SetVal(new Val(f)); - id->SetConst(); } } @@ -233,8 +232,9 @@ Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init, Ref(id); + Expr* name_expr = new NameExpr(id, dt == VAR_CONST); Stmt* stmt = - new ExprStmt(new AssignExpr(new NameExpr(id), init, 0, 0, + new ExprStmt(new AssignExpr(name_expr, init, 0, 0, id->Attrs() ? id->Attrs()->Attrs() : 0 )); stmt->SetLocationInfo(init->GetLocationInfo()); diff --git a/testing/btest/Baseline/language.const/invalid.stderr b/testing/btest/Baseline/language.const/invalid.stderr new file mode 100644 index 0000000000..b08c472708 --- /dev/null +++ b/testing/btest/Baseline/language.const/invalid.stderr @@ -0,0 +1,13 @@ +error in ./invalid.bro, line 15: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 16: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 17: const is not a modifiable lvalue (bar) +error in ./invalid.bro, line 17: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 18: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 19: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 20: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 22: const is not a modifiable lvalue (foo) +error in ./invalid.bro, line 25: const is not a modifiable lvalue (bar) +error in ./invalid.bro, line 26: const is not a modifiable lvalue (baz) +error in ./invalid.bro, line 27: const is not a modifiable lvalue (bar) +error in ./invalid.bro, line 28: const is not a modifiable lvalue (baz) +error in ./invalid.bro, line 33: const is not a modifiable lvalue (foo) diff --git a/testing/btest/Baseline/language.const/invalid.stdout b/testing/btest/Baseline/language.const/invalid.stdout new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/Baseline/language.const/valid.stderr b/testing/btest/Baseline/language.const/valid.stderr new file mode 100644 index 0000000000..e69de29bb2 diff --git a/testing/btest/Baseline/language.const/valid.stdout b/testing/btest/Baseline/language.const/valid.stdout new file mode 100644 index 0000000000..5e3a76f060 --- /dev/null +++ b/testing/btest/Baseline/language.const/valid.stdout @@ -0,0 +1,10 @@ +40 +enter f, 10 +exit f, 110 +enter f, 9 +exit f, 109 +enter f, 7 +exit f, 107 +foo, 10 +bar, 9 +baz, 7 diff --git a/testing/btest/language/const.bro b/testing/btest/language/const.bro new file mode 100644 index 0000000000..ee938e8d45 --- /dev/null +++ b/testing/btest/language/const.bro @@ -0,0 +1,79 @@ +# @TEST-EXEC: bro -b valid.bro 2>valid.stderr 1>valid.stdout +# @TEST-EXEC: btest-diff valid.stderr +# @TEST-EXEC: btest-diff valid.stdout + +# @TEST-EXEC-FAIL: bro -b invalid.bro 2>invalid.stderr 1>invalid.stdout +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff invalid.stderr +# @TEST-EXEC: btest-diff invalid.stdout + +@TEST-START-FILE valid.bro +# First some simple code that should be valid and error-free. + +function f(c: count) + { + print "enter f", c; + c = c + 100; + print "exit f", c; + } + +const foo = 0 &redef; +redef foo = 10; + +const bar = 9; + +event bro_init() + { + const baz = 7; + local i = foo; + i = i + bar + 2; + i = i + baz + 11; + ++i; + print i; + --i; + f(foo); + f(bar); + f(baz); + print "foo", foo; + print "bar", bar; + print "baz", baz; + } + +@TEST-END-FILE + +@TEST-START-FILE invalid.bro +# Now some const assignments that should generate errors at parse-time. + +const foo = 0 &redef; +redef foo = 10; + +const bar = 9; + +event bro_init() + { + const baz = 7; + local s = 0; + + print "nope"; + + foo = 100; + foo = bar; + foo = bar = baz; + foo = s; + ++foo; + s = foo = bar; + + if ( foo = 0 ) + print "nope"; + + bar = 1 + 1; + baz = s; + ++bar; + --baz; + + print "foo", foo; + print "bar", bar; + print "baz", baz; + print "foo=foo", foo = foo; + } + +@TEST-END-FILE