Skip to content

Commit

Permalink
compiler: introduce size threshold for nil checks
Browse files Browse the repository at this point in the history
    
    Add a new control variable to the Gogo class that stores the size
    threshold for nil checks. This value can be used to control the
    policy for deciding when a given deference operation needs a check and
    when it does not. A size threshold of -1 means that every potentially
    faulting dereference needs an explicit check (and branch to error
    call). A size threshold of K (where K > 0) means that if the size of
    the object being dereferenced is >= K, then we need a check.
    
    Reviewed-on: https://go-review.googlesource.com/80996

	* go-c.h (go_create_gogo_args): Add nil_check_size_threshold
	field.
	* go-lang.c (go_langhook_init): Set nil_check_size_threshold.


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@255340 138bc75d-0d04-0410-961f-82ee72b054a4
  • Loading branch information
ian committed Dec 1, 2017
1 parent 8812925 commit f614ea8
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 81 deletions.
6 changes: 6 additions & 0 deletions gcc/go/ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
2017-12-01 Than McIntosh <[email protected]>

* go-c.h (go_create_gogo_args): Add nil_check_size_threshold
field.
* go-lang.c (go_langhook_init): Set nil_check_size_threshold.

2017-11-28 Jakub Jelinek <[email protected]>

* go-gcc.cc (Gcc_backend::switch_statement): Build SWITCH_EXPR using
Expand Down
1 change: 1 addition & 0 deletions gcc/go/go-c.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct go_create_gogo_args
bool check_divide_overflow;
bool compiling_runtime;
int debug_escape_level;
int64_t nil_check_size_threshold;
};

extern void go_create_gogo (const struct go_create_gogo_args*);
Expand Down
1 change: 1 addition & 0 deletions gcc/go/go-lang.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ go_langhook_init (void)
args.check_divide_overflow = go_check_divide_overflow;
args.compiling_runtime = go_compiling_runtime;
args.debug_escape_level = go_debug_escape_level;
args.nil_check_size_threshold = 4096;
args.linemap = go_get_linemap();
args.backend = go_get_backend();
go_create_gogo (&args);
Expand Down
2 changes: 1 addition & 1 deletion gcc/go/gofrontend/MERGE
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
0d6b3abcbfe04949db947081651a503ceb12fe6e
8cd42a3e9e0e618bb09e67be73f7d2f2477a0faa

The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
158 changes: 105 additions & 53 deletions gcc/go/gofrontend/expressions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ Expression::get_interface_type_descriptor(Expression* rhs)
Expression::make_interface_info(rhs, INTERFACE_INFO_METHODS, location);

Expression* descriptor =
Expression::make_unary(OPERATOR_MULT, mtable, location);
Expression::make_dereference(mtable, NIL_CHECK_NOT_NEEDED, location);
descriptor = Expression::make_field_reference(descriptor, 0, location);
Expression* nil = Expression::make_nil(location);

Expand Down Expand Up @@ -393,7 +393,8 @@ Expression::convert_interface_to_type(Type *lhs_type, Expression* rhs,
{
obj = Expression::make_unsafe_cast(Type::make_pointer_type(lhs_type), obj,
location);
obj = Expression::make_unary(OPERATOR_MULT, obj, location);
obj = Expression::make_dereference(obj, NIL_CHECK_NOT_NEEDED,
location);
}
return Expression::make_compound(check_iface, obj, location);
}
Expand Down Expand Up @@ -3842,24 +3843,20 @@ Unary_expression::do_flatten(Gogo* gogo, Named_object*,
&& !this->expr_->is_variable())
{
go_assert(this->expr_->type()->points_to() != NULL);
Type* ptype = this->expr_->type()->points_to();
if (!ptype->is_void_type())
switch (this->requires_nil_check(gogo))
{
int64_t s;
bool ok = ptype->backend_type_size(gogo, &s);
if (!ok)
case NIL_CHECK_ERROR_ENCOUNTERED:
{
go_assert(saw_errors());
return Expression::make_error(this->location());
}
if (s >= 4096 || this->issue_nil_check_)
{
Temporary_statement* temp =
Statement::make_temporary(NULL, this->expr_, location);
inserter->insert(temp);
this->expr_ =
Expression::make_temporary_reference(temp, location);
}
case NIL_CHECK_NOT_NEEDED:
break;
case NIL_CHECK_NEEDED:
this->create_temp_ = true;
break;
case NIL_CHECK_DEFAULT:
go_unreachable();
}
}

Expand Down Expand Up @@ -3960,6 +3957,41 @@ Unary_expression::base_is_static_initializer(Expression* expr)
return false;
}

// Return whether this dereference expression requires an explicit nil
// check. If we are dereferencing the pointer to a large struct
// (greater than the specified size threshold), we need to check for
// nil. We don't bother to check for small structs because we expect
// the system to crash on a nil pointer dereference. However, if we
// know the address of this expression is being taken, we must always
// check for nil.
Unary_expression::Nil_check_classification
Unary_expression::requires_nil_check(Gogo* gogo)
{
go_assert(this->op_ == OPERATOR_MULT);
go_assert(this->expr_->type()->points_to() != NULL);

if (this->issue_nil_check_ == NIL_CHECK_NEEDED)
return NIL_CHECK_NEEDED;
else if (this->issue_nil_check_ == NIL_CHECK_NOT_NEEDED)
return NIL_CHECK_NOT_NEEDED;

Type* ptype = this->expr_->type()->points_to();
int64_t type_size = -1;
if (!ptype->is_void_type())
{
bool ok = ptype->backend_type_size(gogo, &type_size);
if (!ok)
return NIL_CHECK_ERROR_ENCOUNTERED;
}

int64_t size_cutoff = gogo->nil_check_size_threshold();
if (size_cutoff == -1 || (type_size != -1 && type_size >= size_cutoff))
this->issue_nil_check_ = NIL_CHECK_NEEDED;
else
this->issue_nil_check_ = NIL_CHECK_NOT_NEEDED;
return this->issue_nil_check_;
}

// Apply unary opcode OP to UNC, setting NC. Return true if this
// could be done, false if not. On overflow, issues an error and sets
// *ISSUED_ERROR.
Expand Down Expand Up @@ -4408,43 +4440,42 @@ Unary_expression::do_get_backend(Translate_context* context)
{
go_assert(this->expr_->type()->points_to() != NULL);

// If we are dereferencing the pointer to a large struct, we
// need to check for nil. We don't bother to check for small
// structs because we expect the system to crash on a nil
// pointer dereference. However, if we know the address of this
// expression is being taken, we must always check for nil.

bool known_valid = false;
Type* ptype = this->expr_->type()->points_to();
Btype* pbtype = ptype->get_backend(gogo);
if (!ptype->is_void_type())
{
int64_t s;
bool ok = ptype->backend_type_size(gogo, &s);
if (!ok)
switch (this->requires_nil_check(gogo))
{
case NIL_CHECK_NOT_NEEDED:
break;
case NIL_CHECK_ERROR_ENCOUNTERED:
{
go_assert(saw_errors());
return gogo->backend()->error_expression();
}
if (s >= 4096 || this->issue_nil_check_)
{
case NIL_CHECK_NEEDED:
{
go_assert(this->expr_->is_variable());
Bexpression* nil =
Expression::make_nil(loc)->get_backend(context);
Expression::make_nil(loc)->get_backend(context);
Bexpression* compare =
gogo->backend()->binary_expression(OPERATOR_EQEQ, bexpr,
nil, loc);
Bexpression* crash =
gogo->runtime_error(RUNTIME_ERROR_NIL_DEREFERENCE,
loc)->get_backend(context);
gogo->runtime_error(RUNTIME_ERROR_NIL_DEREFERENCE,
loc)->get_backend(context);
Bfunction* bfn = context->function()->func_value()->get_decl();
bexpr = gogo->backend()->conditional_expression(bfn, btype,
compare,
crash, bexpr,
loc);

}
}
ret = gogo->backend()->indirect_expression(pbtype, bexpr, false, loc);
known_valid = true;
break;
}
case NIL_CHECK_DEFAULT:
go_unreachable();
}
ret = gogo->backend()->indirect_expression(pbtype, bexpr,
known_valid, loc);
}
break;

Expand Down Expand Up @@ -4529,6 +4560,19 @@ Expression::make_unary(Operator op, Expression* expr, Location location)
return new Unary_expression(op, expr, location);
}

Expression*
Expression::make_dereference(Expression* ptr,
Nil_check_classification docheck,
Location location)
{
Expression* deref = Expression::make_unary(OPERATOR_MULT, ptr, location);
if (docheck == NIL_CHECK_NEEDED)
deref->unary_expression()->set_requires_nil_check(true);
else if (docheck == NIL_CHECK_NOT_NEEDED)
deref->unary_expression()->set_requires_nil_check(false);
return deref;
}

// If this is an indirection through a pointer, return the expression
// being pointed through. Otherwise return this.

Expand Down Expand Up @@ -6829,7 +6873,7 @@ Bound_method_expression::create_thunk(Gogo* gogo, const Method* method,
// Field 0 of the closure is the function code pointer, field 1 is
// the value on which to invoke the method.
Expression* arg = Expression::make_var_reference(cp, loc);
arg = Expression::make_unary(OPERATOR_MULT, arg, loc);
arg = Expression::make_dereference(arg, NIL_CHECK_NOT_NEEDED, loc);
arg = Expression::make_field_reference(arg, 1, loc);

Expression* bme = Expression::make_bound_method(arg, method, fn, loc);
Expand Down Expand Up @@ -6893,7 +6937,8 @@ bme_check_nil(const Method::Field_indexes* field_indexes, Location loc,
Expression::make_nil(loc),
loc);
cond = Expression::make_binary(OPERATOR_OROR, cond, n, loc);
*ref = Expression::make_unary(OPERATOR_MULT, *ref, loc);
*ref = Expression::make_dereference(*ref, Expression::NIL_CHECK_DEFAULT,
loc);
go_assert((*ref)->type()->struct_type() == stype);
}
*ref = Expression::make_field_reference(*ref, field_indexes->field_index,
Expand Down Expand Up @@ -6948,7 +6993,7 @@ Bound_method_expression::do_flatten(Gogo* gogo, Named_object*,
Expression* val = expr;
if (fntype->receiver()->type()->points_to() == NULL
&& val->type()->points_to() != NULL)
val = Expression::make_unary(OPERATOR_MULT, val, loc);
val = Expression::make_dereference(val, NIL_CHECK_DEFAULT, loc);

// Note that we are ignoring this->expr_type_ here. The thunk will
// expect a closure whose second field has type this->expr_type_ (if
Expand Down Expand Up @@ -8919,7 +8964,8 @@ Builtin_call_expression::do_get_backend(Translate_context* context)
arg_type = arg_type->points_to();
go_assert(arg_type->array_type() != NULL
&& !arg_type->is_slice_type());
arg = Expression::make_unary(OPERATOR_MULT, arg, location);
arg = Expression::make_dereference(arg, NIL_CHECK_DEFAULT,
location);
}

Type* int_type = Type::lookup_integer_type("int");
Expand Down Expand Up @@ -8953,8 +8999,9 @@ Builtin_call_expression::do_get_backend(Translate_context* context)
arg, nil, location);
Expression* zero = Expression::make_integer_ul(0, int_type,
location);
Expression* indir = Expression::make_unary(OPERATOR_MULT,
arg, location);
Expression* indir =
Expression::make_dereference(arg, NIL_CHECK_NOT_NEEDED,
location);
val = Expression::make_conditional(cmp, zero, indir, location);
}
else
Expand Down Expand Up @@ -8995,8 +9042,9 @@ Builtin_call_expression::do_get_backend(Translate_context* context)
arg, nil, location);
Expression* zero = Expression::make_integer_ul(0, int_type,
location);
Expression* indir = Expression::make_unary(OPERATOR_MULT,
parg, location);
Expression* indir =
Expression::make_dereference(parg, NIL_CHECK_NOT_NEEDED,
location);
val = Expression::make_conditional(cmp, zero, indir, location);
}
else
Expand Down Expand Up @@ -10274,7 +10322,7 @@ Call_expression::do_get_backend(Translate_context* context)
Type::make_pointer_type(
Type::make_pointer_type(Type::make_void_type()));
fn = Expression::make_unsafe_cast(pfntype, this->fn_, location);
fn = Expression::make_unary(OPERATOR_MULT, fn, location);
fn = Expression::make_dereference(fn, NIL_CHECK_NOT_NEEDED, location);
}
else
{
Expand Down Expand Up @@ -10532,8 +10580,8 @@ Index_expression::do_lower(Gogo*, Named_object*, Statement_inserter*, int)
&& type->points_to()->array_type() != NULL
&& !type->points_to()->is_slice_type())
{
Expression* deref = Expression::make_unary(OPERATOR_MULT, left,
location);
Expression* deref =
Expression::make_dereference(left, NIL_CHECK_DEFAULT, location);

// For an ordinary index into the array, the pointer will be
// dereferenced. For a slice it will not--the resulting slice
Expand Down Expand Up @@ -11297,7 +11345,8 @@ String_index_expression::do_get_backend(Translate_context* context)
Location loc = this->location();
Expression* string_arg = this->string_;
if (this->string_->type()->points_to() != NULL)
string_arg = Expression::make_unary(OPERATOR_MULT, this->string_, loc);
string_arg = Expression::make_dereference(this->string_,
NIL_CHECK_NOT_NEEDED, loc);

Expression* bad_index = Expression::check_bounds(this->start_, loc);

Expand Down Expand Up @@ -11531,8 +11580,9 @@ Map_index_expression::do_get_backend(Translate_context* context)
go_assert(this->value_pointer_ != NULL
&& this->value_pointer_->is_variable());

Expression* val = Expression::make_unary(OPERATOR_MULT, this->value_pointer_,
this->location());
Expression* val = Expression::make_dereference(this->value_pointer_,
NIL_CHECK_NOT_NEEDED,
this->location());
return val->get_backend(context);
}

Expand Down Expand Up @@ -11768,7 +11818,7 @@ Interface_field_reference_expression::get_function()
Expression* ref = this->expr_;
Location loc = this->location();
if (ref->type()->points_to() != NULL)
ref = Expression::make_unary(OPERATOR_MULT, ref, loc);
ref = Expression::make_dereference(ref, NIL_CHECK_DEFAULT, loc);

Expression* mtable =
Expression::make_interface_info(ref, INTERFACE_INFO_METHODS, loc);
Expand All @@ -11778,7 +11828,8 @@ Interface_field_reference_expression::get_function()
unsigned int index;
const Struct_field* field = mtable_type->find_local_field(name, &index);
go_assert(field != NULL);
mtable = Expression::make_unary(OPERATOR_MULT, mtable, loc);

mtable = Expression::make_dereference(mtable, NIL_CHECK_NOT_NEEDED, loc);
return Expression::make_field_reference(mtable, index, loc);
}

Expand All @@ -11790,7 +11841,8 @@ Interface_field_reference_expression::get_underlying_object()
{
Expression* expr = this->expr_;
if (expr->type()->points_to() != NULL)
expr = Expression::make_unary(OPERATOR_MULT, expr, this->location());
expr = Expression::make_dereference(expr, NIL_CHECK_DEFAULT,
this->location());
return Expression::make_interface_info(expr, INTERFACE_INFO_OBJECT,
this->location());
}
Expand Down Expand Up @@ -11963,7 +12015,7 @@ Interface_field_reference_expression::create_thunk(Gogo* gogo,
// Field 0 of the closure is the function code pointer, field 1 is
// the value on which to invoke the method.
Expression* arg = Expression::make_var_reference(cp, loc);
arg = Expression::make_unary(OPERATOR_MULT, arg, loc);
arg = Expression::make_dereference(arg, NIL_CHECK_NOT_NEEDED, loc);
arg = Expression::make_field_reference(arg, 1, loc);

Expression *ifre = Expression::make_interface_field_reference(arg, name,
Expand Down
Loading

0 comments on commit f614ea8

Please sign in to comment.