Var: pass IntrusivePtr to add_global(), add_local() etc.

Those functions don't have a well-defined reference passing API, and
we had lots of memory leaks here.  By using IntrusivePtr, reference
ownership is well-defined.
This commit is contained in:
Max Kellermann 2020-02-26 05:28:21 +01:00
parent 17a2f1a3ab
commit 2c0ece7376
5 changed files with 85 additions and 101 deletions

View file

@ -1119,8 +1119,8 @@ ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr)
else
{
delete add_local((*loop_vars)[i],
ind_type->Ref(), INIT_NONE,
add_local({NewRef{}, (*loop_vars)[i]},
{NewRef{}, ind_type}, INIT_NONE,
0, 0, VAR_REGULAR);
}
}
@ -1136,7 +1136,7 @@ ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr)
BroType* t = (*loop_vars)[0]->Type();
if ( ! t )
delete add_local((*loop_vars)[0], base_type(TYPE_COUNT),
add_local({NewRef{}, (*loop_vars)[0]}, {AdoptRef{}, base_type(TYPE_COUNT)},
INIT_NONE, 0, 0, VAR_REGULAR);
else if ( ! IsIntegral(t->Tag()) )
@ -1156,8 +1156,8 @@ ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr)
BroType* t = (*loop_vars)[0]->Type();
if ( ! t )
delete add_local((*loop_vars)[0],
base_type(TYPE_STRING),
add_local({NewRef{}, (*loop_vars)[0]},
{AdoptRef{}, base_type(TYPE_STRING)},
INIT_NONE, 0, 0, VAR_REGULAR);
else if ( t->Tag() != TYPE_STRING )
@ -1187,7 +1187,7 @@ ForStmt::ForStmt(id_list* arg_loop_vars, Expr* loop_expr, ID* val_var)
}
else
{
delete add_local(value_var, yield_type->Ref(), INIT_NONE,
add_local({NewRef{}, value_var}, {NewRef{}, yield_type}, INIT_NONE,
0, 0, VAR_REGULAR);
}
}

View file

@ -27,22 +27,21 @@ static Val* init_val(Expr* init, const BroType* t, Val* aggr)
}
}
static void make_var(ID* id, BroType* t, init_class c, Expr* init,
static void make_var(ID* id, IntrusivePtr<BroType> t, init_class c, IntrusivePtr<Expr> init,
attr_list* attr, decl_type dt, int do_init)
{
if ( id->Type() )
{
if ( id->IsRedefinable() || (! init && attr) )
{
BroObj* redef_obj = init ? (BroObj*) init : (BroObj*) t;
BroObj* redef_obj = init ? (BroObj*) init.get() : (BroObj*) t.get();
if ( dt != VAR_REDEF )
id->Warn("redefinition requires \"redef\"", redef_obj, 1);
}
else if ( dt != VAR_REDEF || init || ! attr )
{
id->Error("already defined", init);
Unref(init);
id->Error("already defined", init.get());
return;
}
}
@ -52,29 +51,26 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
if ( ! id->Type() )
{
id->Error("\"redef\" used but not previously defined");
Unref(init);
return;
}
if ( ! t )
t = id->Type();
t = {NewRef{}, id->Type()};
}
if ( id->Type() && id->Type()->Tag() != TYPE_ERROR )
{
if ( dt != VAR_REDEF &&
(! init || ! do_init || (! t && ! (t = init_type(init)))) )
(! init || ! do_init || (! t && ! (t = {AdoptRef{}, init_type(init.get())}))) )
{
id->Error("already defined", init);
Unref(init);
id->Error("already defined", init.get());
return;
}
// Allow redeclaration in order to initialize.
if ( ! same_type(t, id->Type()) )
if ( ! same_type(t.get(), id->Type()) )
{
id->Error("redefinition changes type", init);
Unref(init);
id->Error("redefinition changes type", init.get());
return;
}
}
@ -88,13 +84,11 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
{
if ( init )
{
id->Error("double initialization", init);
Unref(init);
id->Error("double initialization", init.get());
return;
}
Ref(elements);
init = elements;
init = {NewRef{}, elements};
}
}
@ -103,32 +97,28 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
if ( ! init )
{
id->Error("no type given");
Unref(init);
return;
}
t = init_type(init);
t = {AdoptRef{}, init_type(init.get())};
if ( ! t )
{
id->SetType(error_type());
Unref(init);
return;
}
}
else
Ref(t);
id->SetType(t);
id->SetType(t->Ref());
if ( attr )
id->AddAttrs(new Attributes(attr, t, false, id->IsGlobal()));
id->AddAttrs(new Attributes(attr, t.get(), false, id->IsGlobal()));
if ( init )
{
switch ( init->Tag() ) {
case EXPR_TABLE_CONSTRUCTOR:
{
TableConstructorExpr* ctor = (TableConstructorExpr*) init;
TableConstructorExpr* ctor = (TableConstructorExpr*) init.get();
if ( ctor->Attrs() )
{
::Ref(ctor->Attrs());
@ -139,7 +129,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
case EXPR_SET_CONSTRUCTOR:
{
SetConstructorExpr* ctor = (SetConstructorExpr*) init;
SetConstructorExpr* ctor = (SetConstructorExpr*) init.get();
if ( ctor->Attrs() )
{
::Ref(ctor->Attrs());
@ -164,7 +154,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
if ( init && ((c == INIT_EXTRA && id->FindAttr(ATTR_ADD_FUNC)) ||
(c == INIT_REMOVE && id->FindAttr(ATTR_DEL_FUNC)) ))
// Just apply the function.
id->SetVal(init, c);
id->SetVal(init->Ref(), c);
else if ( dt != VAR_REDEF || init || ! attr )
{
@ -175,7 +165,7 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
if ( init && t )
// Have an initialization and type is not deduced.
init = new RecordCoerceExpr(init, t->AsRecordType());
init = make_intrusive<RecordCoerceExpr>(init.release(), t->AsRecordType());
}
else if ( t->Tag() == TYPE_TABLE )
@ -190,13 +180,10 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
Val* v = 0;
if ( init )
{
v = init_val(init, t, aggr);
v = init_val(init.get(), t.get(), aggr);
if ( ! v )
{
Unref(init);
return;
}
}
if ( aggr )
id->SetVal(aggr, c);
@ -221,8 +208,6 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
id->SetOption();
}
Unref(init);
id->UpdateValAttrs();
if ( t && t->Tag() == TYPE_FUNC &&
@ -238,73 +223,71 @@ static void make_var(ID* id, BroType* t, init_class c, Expr* init,
}
void add_global(ID* id, BroType* t, init_class c, Expr* init,
void add_global(ID* id, IntrusivePtr<BroType> t, init_class c, IntrusivePtr<Expr> init,
attr_list* attr, decl_type dt)
{
make_var(id, t, c, init, attr, dt, 1);
make_var(id, t, c, std::move(init), attr, dt, 1);
}
Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init,
IntrusivePtr<Stmt> add_local(IntrusivePtr<ID> id, IntrusivePtr<BroType> t, init_class c, IntrusivePtr<Expr> init,
attr_list* attr, decl_type dt)
{
make_var(id, t, c, init ? init->Ref() : init, attr, dt, 0);
make_var(id.get(), t, c, init, attr, dt, 0);
if ( init )
{
if ( c != INIT_FULL )
id->Error("can't use += / -= for initializations of local variables");
Ref(id);
Expr* name_expr = new NameExpr(id, dt == VAR_CONST);
Stmt* stmt =
new ExprStmt(new AssignExpr(name_expr, init, 0, 0,
const Location* location = init->GetLocationInfo();
Expr* name_expr = new NameExpr(IntrusivePtr{id}.release(), dt == VAR_CONST);
auto stmt =
make_intrusive<ExprStmt>(new AssignExpr(name_expr, init.release(), 0, 0,
id->Attrs() ? id->Attrs()->Attrs() : 0 ));
stmt->SetLocationInfo(init->GetLocationInfo());
stmt->SetLocationInfo(location);
return stmt;
}
else
{
current_scope()->AddInit(id);
return new NullStmt;
current_scope()->AddInit(id.release());
return make_intrusive<NullStmt>();
}
}
extern Expr* add_and_assign_local(ID* id, Expr* init, Val* val)
extern IntrusivePtr<Expr> add_and_assign_local(IntrusivePtr<ID> id, IntrusivePtr<Expr> init, IntrusivePtr<Val> val)
{
make_var(id, 0, INIT_FULL, init->Ref(), 0, VAR_REGULAR, 0);
Ref(id);
return new AssignExpr(new NameExpr(id), init, 0, val);
make_var(id.get(), 0, INIT_FULL, IntrusivePtr{init}, 0, VAR_REGULAR, 0);
return make_intrusive<AssignExpr>(new NameExpr(id.release()), init.release(), 0, val.release());
}
void add_type(ID* id, BroType* t, attr_list* attr)
void add_type(ID* id, IntrusivePtr<BroType> t, attr_list* attr)
{
string new_type_name = id->Name();
string old_type_name = t->GetName();
BroType* tnew = 0;
IntrusivePtr<BroType> tnew;
if ( (t->Tag() == TYPE_RECORD || t->Tag() == TYPE_ENUM) &&
old_type_name.empty() )
// An extensible type (record/enum) being declared for first time.
tnew = t;
tnew = std::move(t);
else
// Clone the type to preserve type name aliasing.
tnew = t->ShallowClone();
tnew = {AdoptRef{}, t->ShallowClone()};
BroType::AddAlias(new_type_name, tnew);
BroType::AddAlias(new_type_name, tnew.get());
if ( new_type_name != old_type_name && ! old_type_name.empty() )
BroType::AddAlias(old_type_name, tnew);
BroType::AddAlias(old_type_name, tnew.get());
tnew->SetName(id->Name());
id->SetType(tnew);
id->SetType(IntrusivePtr{tnew}.release());
id->MakeType();
if ( attr )
id->SetAttrs(new Attributes(attr, tnew, false, false));
id->SetAttrs(new Attributes(attr, tnew.get(), false, false));
}
static void transfer_arg_defaults(RecordType* args, RecordType* recv)
@ -348,22 +331,23 @@ static bool has_attr(const attr_list* al, attr_tag tag)
}
void begin_func(ID* id, const char* module_name, function_flavor flavor,
int is_redef, FuncType* t, attr_list* attrs)
int is_redef, IntrusivePtr<FuncType> t, attr_list* attrs)
{
if ( flavor == FUNC_FLAVOR_EVENT )
{
const BroType* yt = t->YieldType();
if ( yt && yt->Tag() != TYPE_VOID )
id->Error("event cannot yield a value", t);
id->Error("event cannot yield a value", t.get());
t->ClearYieldType(flavor);
}
if ( id->Type() )
{
if ( ! same_type(id->Type(), t) )
id->Type()->Error("incompatible types", t);
if ( ! same_type(id->Type(), t.get()) )
id->Type()->Error("incompatible types", t.get());
else
// If a previous declaration of the function had &default params,
// automatically transfer any that are missing (convenience so that
@ -379,7 +363,7 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor,
function_flavor id_flavor = id->ID_Val()->AsFunc()->Flavor();
if ( id_flavor != flavor )
id->Error("inconsistent function flavor", t);
id->Error("inconsistent function flavor", t.get());
switch ( id_flavor ) {
@ -401,7 +385,7 @@ void begin_func(ID* id, const char* module_name, function_flavor flavor,
}
}
else
id->SetType(t);
id->SetType(IntrusivePtr{t}.release());
push_scope(id, attrs);
@ -482,9 +466,9 @@ TraversalCode OuterIDBindingFinder::PostExpr(const Expr* expr)
return TC_CONTINUE;
}
void end_func(Stmt* body)
void end_func(IntrusivePtr<Stmt> body)
{
auto ingredients = std::make_unique<function_ingredients>(pop_scope(), body);
auto ingredients = std::make_unique<function_ingredients>(pop_scope(), body.release());
if ( streq(ingredients->id->Name(), "anonymous-function") )
{

View file

@ -2,6 +2,7 @@
#pragma once
#include "IntrusivePtr.h"
#include "ID.h"
#include "Type.h"
@ -16,17 +17,17 @@ class ListVal;
typedef enum { VAR_REGULAR, VAR_CONST, VAR_REDEF, VAR_OPTION, } decl_type;
extern void add_global(ID* id, BroType* t, init_class c, Expr* init,
extern void add_global(ID* id, IntrusivePtr<BroType> t, init_class c, IntrusivePtr<Expr> init,
attr_list* attr, decl_type dt);
extern Stmt* add_local(ID* id, BroType* t, init_class c, Expr* init,
extern IntrusivePtr<Stmt> add_local(IntrusivePtr<ID> id, IntrusivePtr<BroType> t, init_class c, IntrusivePtr<Expr> init,
attr_list* attr, decl_type dt);
extern Expr* add_and_assign_local(ID* id, Expr* init, Val* val = 0);
extern IntrusivePtr<Expr> add_and_assign_local(IntrusivePtr<ID> id, IntrusivePtr<Expr> init, IntrusivePtr<Val> val = nullptr);
extern void add_type(ID* id, BroType* t, attr_list* attr);
extern void add_type(ID* id, IntrusivePtr<BroType> t, attr_list* attr);
extern void begin_func(ID* id, const char* module_name, function_flavor flavor,
int is_redef, FuncType* t, attr_list* attrs = nullptr);
extern void end_func(Stmt* body);
int is_redef, IntrusivePtr<FuncType> t, attr_list* attrs = nullptr);
extern void end_func(IntrusivePtr<Stmt> body);
// Gather all IDs referenced inside a body that aren't part of a given scope.
extern id_list gather_outer_ids(Scope* scope, Stmt* body);

View file

@ -479,7 +479,7 @@ expr:
| TOK_LOCAL local_id '=' expr
{
set_location(@2, @4);
$$ = add_and_assign_local($2, $4, val_mgr->GetBool(1));
$$ = add_and_assign_local({AdoptRef{}, $2}, {AdoptRef{}, $4}, {AdoptRef{}, val_mgr->GetBool(1)}).release();
}
| expr '[' expr_list ']'
@ -511,7 +511,7 @@ expr:
current_module.c_str(),
FUNC_FLAVOR_FUNCTION,
0,
$3);
{AdoptRef{}, $3});
}
func_body
{
@ -1089,27 +1089,27 @@ decl:
| TOK_GLOBAL def_global_id opt_type init_class opt_init opt_attr ';'
{
add_global($2, $3, $4, $5, $6, VAR_REGULAR);
add_global($2, {AdoptRef{}, $3}, $4, {AdoptRef{}, $5}, $6, VAR_REGULAR);
zeekygen_mgr->Identifier($2);
}
| TOK_OPTION def_global_id opt_type init_class opt_init opt_attr ';'
{
add_global($2, $3, $4, $5, $6, VAR_OPTION);
add_global($2, {AdoptRef{}, $3}, $4, {AdoptRef{}, $5}, $6, VAR_OPTION);
zeekygen_mgr->Identifier($2);
}
| TOK_CONST def_global_id opt_type init_class opt_init opt_attr ';'
{
add_global($2, $3, $4, $5, $6, VAR_CONST);
add_global($2, {AdoptRef{}, $3}, $4, {AdoptRef{}, $5}, $6, VAR_CONST);
zeekygen_mgr->Identifier($2);
}
| TOK_REDEF global_id opt_type init_class opt_init opt_attr ';'
{
IntrusivePtr<Expr> e{NewRef{}, $5};
add_global($2, $3, $4, $5, $6, VAR_REDEF);
zeekygen_mgr->Redef($2, ::filename, $4, $5);
IntrusivePtr<Expr> init{AdoptRef{}, $5};
add_global($2, {AdoptRef{}, $3}, $4, init, $6, VAR_REDEF);
zeekygen_mgr->Redef($2, ::filename, $4, init.release());
}
| TOK_REDEF TOK_ENUM global_id TOK_ADD_TO '{'
@ -1140,7 +1140,7 @@ decl:
type opt_attr ';'
{
cur_decl_type_id = 0;
add_type($2, $5, $6);
add_type($2, {AdoptRef{}, $5}, $6);
zeekygen_mgr->Identifier($2);
}
@ -1172,7 +1172,7 @@ func_hdr:
TOK_FUNCTION def_global_id func_params opt_attr
{
begin_func($2, current_module.c_str(),
FUNC_FLAVOR_FUNCTION, 0, $3, $4);
FUNC_FLAVOR_FUNCTION, 0, {NewRef{}, $3}, $4);
$$ = $3;
zeekygen_mgr->Identifier($2);
}
@ -1186,7 +1186,7 @@ func_hdr:
}
begin_func($2, current_module.c_str(),
FUNC_FLAVOR_EVENT, 0, $3, $4);
FUNC_FLAVOR_EVENT, 0, {NewRef{}, $3}, $4);
$$ = $3;
}
| TOK_HOOK def_global_id func_params opt_attr
@ -1194,13 +1194,13 @@ func_hdr:
$3->ClearYieldType(FUNC_FLAVOR_HOOK);
$3->SetYieldType(base_type(TYPE_BOOL));
begin_func($2, current_module.c_str(),
FUNC_FLAVOR_HOOK, 0, $3, $4);
FUNC_FLAVOR_HOOK, 0, {NewRef{}, $3}, $4);
$$ = $3;
}
| TOK_REDEF TOK_EVENT event_id func_params opt_attr
{
begin_func($3, current_module.c_str(),
FUNC_FLAVOR_EVENT, 1, $4, $5);
FUNC_FLAVOR_EVENT, 1, {NewRef{}, $4}, $5);
$$ = $4;
}
;
@ -1221,7 +1221,7 @@ func_body:
'}'
{
set_location(func_hdr_location, @5);
end_func($3);
end_func({AdoptRef{}, $3});
}
;
@ -1260,7 +1260,7 @@ begin_func:
func_params
{
$$ = current_scope()->GenerateTemporary("anonymous-function");
begin_func($$, current_module.c_str(), FUNC_FLAVOR_FUNCTION, 0, $1);
begin_func($$, current_module.c_str(), FUNC_FLAVOR_FUNCTION, 0, {AdoptRef{}, $1});
}
;
@ -1492,7 +1492,7 @@ stmt:
| TOK_LOCAL local_id opt_type init_class opt_init opt_attr ';' opt_no_test
{
set_location(@1, @7);
$$ = add_local($2, $3, $4, $5, $6, VAR_REGULAR);
$$ = add_local({AdoptRef{}, $2}, {AdoptRef{}, $3}, $4, {AdoptRef{}, $5}, $6, VAR_REGULAR).release();
if ( ! $8 )
brofiler.AddStmt($$);
}
@ -1500,7 +1500,7 @@ stmt:
| TOK_CONST local_id opt_type init_class opt_init opt_attr ';' opt_no_test
{
set_location(@1, @6);
$$ = add_local($2, $3, $4, $5, $6, VAR_CONST);
$$ = add_local({AdoptRef{}, $2}, {AdoptRef{}, $3}, $4, {AdoptRef{}, $5}, $6, VAR_CONST).release();
if ( ! $8 )
brofiler.AddStmt($$);
}
@ -1634,19 +1634,18 @@ case_type:
| TOK_TYPE type TOK_AS TOK_ID
{
const char* name = $4;
BroType* type = $2;
ID* case_var = lookup_ID(name, current_module.c_str());
IntrusivePtr<BroType> type{AdoptRef{}, $2};
IntrusivePtr<ID> case_var{AdoptRef{}, lookup_ID(name, current_module.c_str())};
if ( case_var && case_var->IsGlobal() )
case_var->Error("already a global identifier");
else
{
Unref(case_var);
case_var = install_ID(name, current_module.c_str(), false, false);
case_var = {NewRef{}, install_ID(name, current_module.c_str(), false, false)};
}
add_local(case_var, type, INIT_NONE, 0, 0, VAR_REGULAR);
$$ = case_var;
add_local(case_var, std::move(type), INIT_NONE, 0, 0, VAR_REGULAR);
$$ = case_var.release();
}
for_head:

View file

@ -134,7 +134,7 @@ ComponentManager<T, C>::ComponentManager(const string& arg_module, const string&
tag_enum_type(make_intrusive<EnumType>(module + "::" + local_id))
{
::ID* id = install_ID(local_id.c_str(), module.c_str(), true, true);
add_type(id, tag_enum_type.get(), 0);
add_type(id, tag_enum_type, 0);
zeekygen_mgr->Identifier(id);
}