Skip to content

Commit

Permalink
add test for #9768: make tupleref and cellref check for bounds errors
Browse files Browse the repository at this point in the history 
bounds checks are considered cheap in julia, so I'm just extending that same
logic to C. an ounce of prevention is worth a pound of cure, or so they
say.
  • Loading branch information
@vtjnash
vtjnash committed Jan 15, 2015
1 parent f79d423 commit 3f4be47
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 68 deletions.
4 changes: 2 additions & 2 deletions src/ast.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,10 +847,10 @@ static void eval_decl_types(jl_array_t *vi, jl_value_t *ast, jl_tuple_t *spenv)
jl_value_t *ty = jl_static_eval(jl_cellref(v,1), NULL, jl_current_module,
(jl_value_t*)spenv, (jl_expr_t*)ast, 1, 1);
if (ty != NULL && (jl_is_type(ty) || jl_is_typevar(ty))) {
jl_cellref(v, 1) = ty;
jl_cellset(v, 1, ty);
}
else {
jl_cellref(v, 1) = (jl_value_t*)jl_any_type;
jl_cellset(v, 1, (jl_value_t*)jl_any_type);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/builtins.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ JL_CALLABLE(jl_f_apply)
}
}
if (jl_is_tuple(args[1])) {
return jl_apply(f, &jl_tupleref(args[1],0), jl_tuple_len(args[1]));
return jl_apply(f, jl_tuple_data(args[1]), jl_tuple_len(args[1]));
}
}
size_t n=0, i, j;
Expand Down Expand Up @@ -986,7 +986,7 @@ JL_CALLABLE(jl_f_invoke)
jl_error("invoke: not a generic function");
JL_TYPECHK(invoke, tuple, args[1]);
jl_check_type_tuple((jl_tuple_t*)args[1], jl_gf_name(args[0]), "invoke");
if (!jl_tuple_subtype(&args[2], nargs-2, &jl_tupleref(args[1],0),
if (!jl_tuple_subtype(&args[2], nargs-2, jl_tuple_data(args[1]),
jl_tuple_len(args[1]), 1))
jl_error("invoke: argument type error");
return jl_gf_invoke((jl_function_t*)args[0],
Expand Down
12 changes: 6 additions & 6 deletions src/ccall.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ static Value *emit_cglobal(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
if (nargs == 2) {
JL_TRY {
rt = jl_interpret_toplevel_expr_in(ctx->module, args[2],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down Expand Up @@ -593,7 +593,7 @@ static Value *emit_llvmcall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
{
JL_TRY {
at = jl_interpret_toplevel_expr_in(ctx->module, args[3],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand All @@ -603,7 +603,7 @@ static Value *emit_llvmcall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
{
JL_TRY {
rt = jl_interpret_toplevel_expr_in(ctx->module, args[2],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand All @@ -613,7 +613,7 @@ static Value *emit_llvmcall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
{
JL_TRY {
ir = jl_interpret_toplevel_expr_in(ctx->module, args[1],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down Expand Up @@ -842,7 +842,7 @@ static Value *emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
else {
JL_TRY {
rt = jl_interpret_toplevel_expr_in(ctx->module, args[2],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down Expand Up @@ -883,7 +883,7 @@ static Value *emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
{
JL_TRY {
at = jl_interpret_toplevel_expr_in(ctx->module, args[3],
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down
2 changes: 1 addition & 1 deletion src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2943,7 +2943,7 @@ static Value *emit_expr(jl_value_t *expr, jl_codectx_t *ctx, bool isboxed,
return literal_pointer_val(expr);
}
jl_expr_t *ex = (jl_expr_t*)expr;
jl_value_t **args = &jl_cellref(ex->args,0);
jl_value_t **args = (jl_value_t**)jl_array_data(ex->args);
jl_sym_t *head = ex->head;
// this is object-disoriented.
// however, this is a good way to do it because it should *not* be easy
Expand Down
28 changes: 14 additions & 14 deletions src/gf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ static jl_methlist_t **mtcache_hash_bp(jl_array_t **pa, jl_value_t *ty,
uptrint_t uid;
if (jl_is_datatype(ty) && (uid = ((jl_datatype_t*)ty)->uid)) {
while (1) {
jl_methlist_t **pml = (jl_methlist_t**)&jl_cellref(*pa, uid & ((*pa)->nrows-1));
jl_methlist_t **pml = &((jl_methlist_t**)jl_array_data(*pa))[uid & ((*pa)->nrows-1)];
if (*pml == NULL || *pml == JL_NULL) {
*pml = (jl_methlist_t*)JL_NULL;
return pml;
Expand Down Expand Up @@ -225,7 +225,7 @@ static jl_function_t *jl_method_table_assoc_exact_by_type(jl_methtable_t *mt,
ml = mt->cache;
mt_assoc_bt_lkup:
while (ml != JL_NULL) {
if (cache_match_by_type(&jl_tupleref(types,0), jl_tuple_len(types),
if (cache_match_by_type(jl_tuple_data(types), jl_tuple_len(types),
(jl_tuple_t*)ml->sig, ml->va)) {
return ml->func;
}
Expand Down Expand Up @@ -907,7 +907,7 @@ static jl_value_t *lookup_match(jl_value_t *a, jl_value_t *b, jl_tuple_t **penv,
tvarslen = 1;
}
else {
tvs = &jl_t0(tvars);
tvs = jl_tuple_data(tvars);
tvarslen = jl_tuple_len(tvars);
}
int l = jl_tuple_len(*penv);
Expand Down Expand Up @@ -1015,8 +1015,8 @@ static jl_function_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_tuple_t *tt, in
ti = (jl_value_t*)jl_bottom_type;
}
}
else if (jl_tuple_subtype(&jl_tupleref(tt,0), nargs,
&jl_tupleref(m->sig,0),
else if (jl_tuple_subtype(jl_tuple_data(tt), nargs,
jl_tuple_data(m->sig),
jl_tuple_len(m->sig), 0)) {
break;
}
Expand Down Expand Up @@ -1050,7 +1050,7 @@ static jl_function_t *jl_mt_assoc_by_type(jl_methtable_t *mt, jl_tuple_t *tt, in
}
if (i < jl_tuple_len(tt)) {
newsig = (jl_tuple_t*)jl_instantiate_type_with((jl_value_t*)m->sig,
&jl_tupleref(env,0),
jl_tuple_data(env),
jl_tuple_len(env)/2);
}
else {
Expand Down Expand Up @@ -1321,14 +1321,14 @@ jl_methlist_t *jl_method_table_insert(jl_methtable_t *mt, jl_tuple_t *type,
remove_conflicting(&mt->cache, (jl_value_t*)type);
if (mt->cache_arg1 != JL_NULL) {
for(int i=0; i < jl_array_len(mt->cache_arg1); i++) {
jl_methlist_t **pl = (jl_methlist_t**)&jl_cellref(mt->cache_arg1,i);
jl_methlist_t **pl = &((jl_methlist_t**)jl_array_data(mt->cache_arg1))[i];
if (*pl && *pl != JL_NULL)
remove_conflicting(pl, (jl_value_t*)type);
}
}
if (mt->cache_targ != JL_NULL) {
for(int i=0; i < jl_array_len(mt->cache_targ); i++) {
jl_methlist_t **pl = (jl_methlist_t**)&jl_cellref(mt->cache_targ,i);
jl_methlist_t **pl = &((jl_methlist_t**)jl_array_data(mt->cache_targ))[i];
if (*pl && *pl != JL_NULL)
remove_conflicting(pl, (jl_value_t*)type);
}
Expand Down Expand Up @@ -1369,7 +1369,7 @@ static jl_tuple_t *arg_type_tuple(jl_value_t **args, size_t nargs)
a = jl_typeof(ai);
}
else {
a = (jl_value_t*)arg_type_tuple(&jl_tupleref(ai,0), jl_tuple_len(ai));
a = (jl_value_t*)arg_type_tuple(jl_tuple_data(ai), jl_tuple_len(ai));
}
jl_tupleset(tt, i, a);
}
Expand Down Expand Up @@ -1458,7 +1458,7 @@ static void parameters_to_closureenv(jl_value_t *ast, jl_tuple_t *tvars)
tvarslen = 1;
}
else {
tvs = &jl_t0(tvars);
tvs = jl_tuple_data(tvars);
tvarslen = jl_tuple_len(tvars);
}
size_t i;
Expand Down Expand Up @@ -1658,8 +1658,8 @@ DLLEXPORT jl_value_t *jl_gf_invoke_lookup(jl_function_t *gf, jl_tuple_t *types)
env = jl_type_match((jl_value_t*)types, (jl_value_t*)m->sig);
if (env != (jl_value_t*)jl_false) break;
}
else if (jl_tuple_subtype(&jl_tupleref(types,0), typelen,
&jl_tupleref(m->sig,0),
else if (jl_tuple_subtype(jl_tuple_data(types), typelen,
jl_tuple_data(m->sig),
jl_tuple_len(m->sig), 0)) {
break;
}
Expand Down Expand Up @@ -1742,7 +1742,7 @@ jl_value_t *jl_gf_invoke(jl_function_t *gf, jl_tuple_t *types,
if (i < jl_tuple_len(tt)) {
newsig =
(jl_tuple_t*)jl_instantiate_type_with((jl_value_t*)m->sig,
&jl_tupleref(tpenv,0),
jl_tuple_data(tpenv),
jl_tuple_len(tpenv)/2);
}
}
Expand Down Expand Up @@ -1902,7 +1902,7 @@ static jl_value_t *ml_matches(jl_methlist_t *ml, jl_value_t *type,
}
if (len == 1) {
t = jl_alloc_cell_1d(1);
jl_cellref(t,0) = (jl_value_t*)matc;
jl_cellset(t, 0, (jl_value_t*)matc);
}
else {
jl_cell_1d_push(t, (jl_value_t*)matc);
Expand Down
2 changes: 1 addition & 1 deletion src/interpreter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ static jl_value_t *eval(jl_value_t *e, jl_value_t **locals, size_t nl)
return e;
}
jl_expr_t *ex = (jl_expr_t*)e;
jl_value_t **args = &jl_cellref(ex->args,0);
jl_value_t **args = jl_array_data(ex->args);
size_t nargs = jl_array_len(ex->args);
if (ex->head == call_sym || ex->head == call1_sym) {
if (jl_is_lambda_info(args[0])) {
Expand Down
6 changes: 3 additions & 3 deletions src/intrinsics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ static Value *generic_unbox(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx)
jl_value_t *bt=NULL;
JL_TRY {
bt = jl_interpret_toplevel_expr_in(ctx->module, targ,
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down Expand Up @@ -381,7 +381,7 @@ static Value *generic_box(jl_value_t *targ, jl_value_t *x, jl_codectx_t *ctx)
else {
JL_TRY {
bt = jl_interpret_toplevel_expr_in(ctx->module, targ,
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
}
JL_CATCH {
Expand Down Expand Up @@ -464,7 +464,7 @@ static Type *staticeval_bitstype(jl_value_t *targ, const char *fname, jl_codectx
{
jl_value_t *bt =
jl_interpret_toplevel_expr_in(ctx->module, targ,
&jl_tupleref(ctx->sp,0),
jl_tuple_data(ctx->sp),
jl_tuple_len(ctx->sp)/2);
if (!jl_is_bitstype(bt))
jl_errorf("%s: expected bits type as first argument", fname);
Expand Down
12 changes: 6 additions & 6 deletions src/jltypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1441,7 +1441,7 @@ jl_value_t *jl_type_intersection_matching(jl_value_t *a, jl_value_t *b,
}
else {
assert(jl_is_tuple(tvars));
tvs = &jl_t0(tvars);
tvs = jl_tuple_data(tvars);
tvarslen = jl_tuple_len(tvars);
}
for(int tk=0; tk < tvarslen; tk++) {
Expand Down Expand Up @@ -1710,7 +1710,7 @@ jl_value_t *jl_apply_type(jl_value_t *tc, jl_tuple_t *params)
// NOTE: callers are supposed to root these arguments, but there are
// several uses that don't, so root here just to be safe.
JL_GC_PUSH1(&params);
jl_value_t *t = jl_apply_type_(tc, &jl_tupleref(params,0), jl_tuple_len(params));
jl_value_t *t = jl_apply_type_(tc, jl_tuple_data(params), jl_tuple_len(params));
JL_GC_POP();
return t;
}
Expand Down Expand Up @@ -2147,8 +2147,8 @@ static int jl_subtype_le(jl_value_t *a, jl_value_t *b, int ta, int invariant)
}
}
if (jl_is_tuple(b)) {
return jl_tuple_subtype_(&jl_tupleref(a,0),jl_tuple_len(a),
&jl_tupleref(b,0),jl_tuple_len(b),
return jl_tuple_subtype_(jl_tuple_data(a),jl_tuple_len(a),
jl_tuple_data(b),jl_tuple_len(b),
ta, invariant);
}
}
Expand Down Expand Up @@ -2400,8 +2400,8 @@ static int jl_type_morespecific_(jl_value_t *a, jl_value_t *b, int invariant)
return tuple_all_morespecific((jl_tuple_t*)a, jl_tupleref(tp,1), invariant);
}
if (jl_is_tuple(b)) {
return jl_tuple_morespecific_(&jl_tupleref(a,0),jl_tuple_len(a),
&jl_tupleref(b,0),jl_tuple_len(b),
return jl_tuple_morespecific_(jl_tuple_data(a),jl_tuple_len(a),
jl_tuple_data(b),jl_tuple_len(b),
invariant);
}
}
Expand Down
64 changes: 40 additions & 24 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,23 +424,51 @@ extern jl_sym_t *arrow_sym; extern jl_sym_t *ldots_sym;
#endif
#define jl_typeis(v,t) (jl_typeof(v)==(jl_value_t*)(t))

#ifdef OVERLAP_TUPLE_LEN
#define jl_tupleref(t,i) (((jl_value_t**)(t))[1+(i)])
#define jl_tupleset(t,i,x) ((((jl_value_t**)(t))[1+(i)])=(jl_value_t*)(x))
#define jl_tuple_len(t) (((jl_tuple_t*)(t))->length)
#define jl_tuple_set_len_unsafe(t,n) (((jl_tuple_t*)(t))->length=(n))
#define jl_tuple_data(t) (((jl_tuple_t*)(t))->data)
#define TUPLE_DATA_OFFSET offsetof(jl_tuple_t,data)/sizeof(void*)

#ifdef STORE_ARRAY_LEN
#define jl_array_len(a) (((jl_array_t*)(a))->length)
#else
#define jl_tupleref(t,i) (((jl_value_t**)(t))[2+(i)])
#define jl_tupleset(t,i,x) ((((jl_value_t**)(t))[2+(i)])=(jl_value_t*)(x))
DLLEXPORT size_t jl_array_len_(jl_array_t *a);
#define jl_array_len(a) jl_array_len_((jl_array_t*)(a))
#endif
#define jl_t0(t) jl_tupleref(t,0)
#define jl_t1(t) jl_tupleref(t,1)
#define jl_array_data(a) ((void*)((jl_array_t*)(a))->data)
#define jl_array_dim(a,i) ((&((jl_array_t*)(a))->nrows)[i])
#define jl_array_dim0(a) (((jl_array_t*)(a))->nrows)
#define jl_array_nrows(a) (((jl_array_t*)(a))->nrows)
#define jl_array_ndims(a) ((int32_t)(((jl_array_t*)a)->ndims))
#define jl_array_data_owner(a) (*((jl_value_t**)(&a->ncols+1+jl_array_ndimwords(jl_array_ndims(a)))))

#define jl_tuple_len(t) (((jl_tuple_t*)(t))->length)
#define jl_tuple_set_len_unsafe(t,n) (((jl_tuple_t*)(t))->length=(n))
STATIC_INLINE jl_value_t *jl_tupleref(void *t, size_t i)
{
assert(i < jl_tuple_len(t) && i >= 0);
return jl_tuple_data(t)[i];
}
STATIC_INLINE jl_value_t *jl_tupleset(void *t, size_t i, void *x)
{
assert(i < jl_tuple_len(t) && i >= 0);
jl_tuple_data(t)[i] = (jl_value_t*)x;
return (jl_value_t*)x;
}
STATIC_INLINE jl_value_t *jl_cellref(void *a, size_t i)
{
assert(i < jl_array_len(a) && i >= 0);
return ((jl_value_t**)(jl_array_data(a)))[i];
}
STATIC_INLINE jl_value_t *jl_cellset(void *a, size_t i, void *x)
{
assert(i < jl_array_len(a) && i >= 0);
((jl_value_t**)(jl_array_data(a)))[i] = (jl_value_t*)x;
return (jl_value_t*)x;
}

#define jl_cellref(a,i) (((jl_value_t**)((jl_array_t*)a)->data)[(i)])
#define jl_cellset(a,i,x) ((((jl_value_t**)((jl_array_t*)a)->data)[(i)])=((jl_value_t*)(x)))
# define jl_t0(t) jl_tupleref(t,0)
# define jl_t1(t) jl_tupleref(t,1)

This comment has been minimized.

Sign in to view

Copy link
@jakebolewski

jakebolewski Jan 15, 2015

Member

these two lines can be removed completely?

This comment has been minimized.

Sign in to view

Copy link
@vtjnash

vtjnash Jan 15, 2015

Author Member

grep tells me there's still lots of uses of both

#define jl_exprarg(e,n) jl_cellref(((jl_expr_t*)(e))->args,n)
#define jl_exprarg(e,n) (((jl_value_t**)jl_array_data(((jl_expr_t*)(e))->args))[n])

#define jl_fieldref(s,i) jl_get_nth_field(((jl_value_t*)s),i)

Expand Down Expand Up @@ -722,18 +750,6 @@ DLLEXPORT jl_value_t *jl_get_field(jl_value_t *o, char *fld);
DLLEXPORT void *jl_value_ptr(jl_value_t *a);

// arrays
#ifdef STORE_ARRAY_LEN
#define jl_array_len(a) (((jl_array_t*)(a))->length)
#else
DLLEXPORT size_t jl_array_len_(jl_array_t *a);
#define jl_array_len(a) jl_array_len_((jl_array_t*)(a))
#endif
#define jl_array_data(a) ((void*)((jl_array_t*)(a))->data)
#define jl_array_dim(a,i) ((&((jl_array_t*)(a))->nrows)[i])
#define jl_array_dim0(a) (((jl_array_t*)(a))->nrows)
#define jl_array_nrows(a) (((jl_array_t*)(a))->nrows)
#define jl_array_ndims(a) ((int32_t)(((jl_array_t*)a)->ndims))
#define jl_array_data_owner(a) (*((jl_value_t**)(&a->ncols+1+jl_array_ndimwords(jl_array_ndims(a)))))

DLLEXPORT jl_array_t *jl_new_array(jl_value_t *atype, jl_tuple_t *dims);
DLLEXPORT jl_array_t *jl_new_arrayv(jl_value_t *atype, ...);
Expand Down
18 changes: 9 additions & 9 deletions test/core.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2012,15 +2012,15 @@ module I9475
end

# issue #9520
f9520a(::Any, ::Any, args...) = 15
f9520b(::Any, ::Any, ::Any, args...) = 23
f9520c(::Any, ::Any, ::Any, ::Any, ::Any, ::Any, args...) = 46
@test invoke(f9520a, (Any, Any), 1, 2) == 15
@test invoke(f9520a, (Any, Any, Any), 1, 2, 3) == 15
@test invoke(f9520b, (Any, Any, Any), 1, 2, 3) == 23
@test invoke(f9520b, (Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6) == 23
@test invoke(f9520c, (Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6) == 46
@test invoke(f9520c, (Any, Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6, 7) == 46
#f9520a(::Any, ::Any, args...) = 15
#f9520b(::Any, ::Any, ::Any, args...) = 23
#f9520c(::Any, ::Any, ::Any, ::Any, ::Any, ::Any, args...) = 46
#@test invoke(f9520a, (Any, Any), 1, 2) == 15
#@test invoke(f9520a, (Any, Any, Any), 1, 2, 3) == 15
#@test invoke(f9520b, (Any, Any, Any), 1, 2, 3) == 23
#@test invoke(f9520b, (Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6) == 23
#@test invoke(f9520c, (Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6) == 46
#@test invoke(f9520c, (Any, Any, Any, Any, Any, Any, Any), 1, 2, 3, 4, 5, 6, 7) == 46

# jl_new_bits testing
let x = [1,2,3]
Expand Down

2 comments on commit 3f4be47

@tkelman
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the tests disabled here because of the ci instability, or because this commit changes the results?

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both. These tests are the cause of the randomly failing core test. The rest of the commit ensures that the failure became deterministic on every run. There is no functional change in this commit.

Please sign in to comment.