Skip to content

Commit

Permalink
fix various code issues: some formatting cleanup, some missing gc roo…
Browse files Browse the repository at this point in the history 
…ts, some potential alignment issues, and a strcpy buffer overflow at startup
  • Loading branch information
@vtjnash
vtjnash committed Jan 14, 2015
1 parent 8938e3a commit 69b84e9
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 216 deletions.
35 changes: 23 additions & 12 deletions src/alloc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -186,11 +186,17 @@ DLLEXPORT jl_value_t *jl_pointerref(jl_value_t *p, jl_value_t *i)
JL_TYPECHK(pointerref, pointer, p);
JL_TYPECHK(pointerref, long, i);
jl_value_t *ety = jl_tparam0(jl_typeof(p));
if (!jl_is_datatype(ety))
jl_error("pointerref: invalid pointer");
size_t nb = jl_datatype_size(ety);
char *pp = (char*)jl_unbox_long(p) + (jl_unbox_long(i)-1)*nb;
return jl_new_bits(ety, pp);
if (ety == (jl_value_t*)jl_any_type) {
jl_value_t **pp = (jl_value_t**)(jl_unbox_long(p) + (jl_unbox_long(i)-1)*sizeof(void*));
return *pp;
}
else {
if (!jl_is_datatype(ety))
jl_error("pointerref: invalid pointer");
size_t nb = LLT_ALIGN(jl_datatype_size(ety), ((jl_datatype_t*)ety)->alignment);
char *pp = (char*)jl_unbox_long(p) + (jl_unbox_long(i)-1)*nb;
return jl_new_bits(ety, pp);
}
}

void jl_assign_bits(void *dest, jl_value_t *bits)
Expand All @@ -213,13 +219,18 @@ DLLEXPORT void jl_pointerset(jl_value_t *p, jl_value_t *x, jl_value_t *i)
JL_TYPECHK(pointerset, pointer, p);
JL_TYPECHK(pointerset, long, i);
jl_value_t *ety = jl_tparam0(jl_typeof(p));
if (!jl_is_datatype(ety))
jl_error("pointerset: invalid pointer");
size_t nb = jl_datatype_size(ety);
char *pp = (char*)jl_unbox_long(p) + (jl_unbox_long(i)-1)*nb;
if (jl_typeof(x) != ety)
jl_error("pointerset: type mismatch in assign");
jl_assign_bits(pp, x);
if (ety == (jl_value_t*)jl_any_type) {
jl_value_t **pp = (jl_value_t**)(jl_unbox_long(p) + (jl_unbox_long(i)-1)*sizeof(void*));
*pp = x;
} else {
if (!jl_is_datatype(ety))
jl_error("pointerset: invalid pointer");
size_t nb = LLT_ALIGN(jl_datatype_size(ety), ((jl_datatype_t*)ety)->alignment);
char *pp = (char*)jl_unbox_long(p) + (jl_unbox_long(i)-1)*nb;
if (jl_typeof(x) != ety)
jl_error("pointerset: type mismatch in assign");
jl_assign_bits(pp, x);
}
}

int jl_field_index(jl_datatype_t *t, jl_sym_t *fld, int err)
Expand Down
17 changes: 8 additions & 9 deletions src/builtins.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,10 +405,6 @@ JL_CALLABLE(jl_f_apply)
return jl_apply(f, &jl_tupleref(args[1],0), jl_tuple_len(args[1]));
}
}
jl_value_t *argarr = NULL;
JL_GC_PUSH1(&argarr);
jl_value_t *result;
jl_value_t **newargs;
size_t n=0, i, j;
for(i=1; i < nargs; i++) {
if (jl_is_tuple(args[i])) {
Expand All @@ -426,20 +422,23 @@ JL_CALLABLE(jl_f_apply)
JL_TYPECHK(apply, tuple, args[i]);
}
}
argarr = jl_apply(jl_append_any_func, &args[1], nargs-1);
jl_value_t *argarr = jl_apply(jl_append_any_func, &args[1], nargs-1);
assert(jl_typeis(argarr, jl_array_any_type));
result = jl_apply(f, jl_cell_data(argarr), jl_array_len(argarr));
JL_GC_PUSH1(&argarr);
jl_value_t *result = jl_apply(f, jl_cell_data(argarr), jl_array_len(argarr));
JL_GC_POP();
return result;
}
}
jl_value_t **newargs;
if (n > jl_page_size/sizeof(jl_value_t*)) {
// put arguments on the heap if there are too many
argarr = (jl_value_t*)jl_alloc_cell_1d(n);
jl_value_t *argarr = (jl_value_t*)jl_alloc_cell_1d(n);
JL_GC_PUSH1(&argarr);
newargs = jl_cell_data(argarr);
}
else {
newargs = (jl_value_t**)alloca(n * sizeof(jl_value_t*));
JL_GC_PUSHARGS(newargs, n);
}
n = 0;
for(i=1; i < nargs; i++) {
Expand All @@ -455,7 +454,7 @@ JL_CALLABLE(jl_f_apply)
newargs[n++] = jl_cellref(args[i], j);
}
}
result = jl_apply(f, newargs, n);
jl_value_t *result = jl_apply(f, newargs, n);
JL_GC_POP();
return result;
}
Expand Down
2 changes: 1 addition & 1 deletion src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,7 @@ void jl_restore_system_image(const char *fname)
if (jl_is_debugbuild()) build_mode = 1;
#endif
if (!build_mode) {
char *fname_shlib = (char*)alloca(strlen(fname));
char *fname_shlib = (char*)alloca(strlen(fname)+1);
strcpy(fname_shlib, fname);
char *fname_shlib_dot = strrchr(fname_shlib, '.');
if (fname_shlib_dot != NULL) *fname_shlib_dot = 0;
Expand Down
181 changes: 0 additions & 181 deletions src/flisp/basename.c
View file

This file was deleted.

3 changes: 2 additions & 1 deletion src/init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -806,7 +806,7 @@ int isabspath(const char *in)
}
}
#else
if (jl_compileropts.image_file[0] == '/') return 1; // absolute path
if (in[0] == '/') return 1; // absolute path
#endif
return 0; // relative path
}
Expand Down Expand Up @@ -834,6 +834,7 @@ static char *abspath(const char *in)
path[path_size-1] = PATHSEPSTRING[0];
memcpy(path+path_size, in, len+1);
out = strdup(path);
free(path);
}
}
#else
Expand Down
9 changes: 6 additions & 3 deletions src/intrinsics.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,14 +620,15 @@ static Value *emit_pointerref(jl_value_t *e, jl_value_t *i, jl_codectx_t *ctx)
return NULL;
}
assert(jl_is_datatype(ety));
uint64_t size = ((jl_datatype_t*)ety)->size;
uint64_t size = jl_datatype_size(ety);
Value *strct =
builder.CreateCall(prepare_call(jlallocobj_func),
ConstantInt::get(T_size,
sizeof(void*)+size));
builder.CreateStore(literal_pointer_val((jl_value_t*)ety),
emit_nthptr_addr(strct, (size_t)0));
im1 = builder.CreateMul(im1, ConstantInt::get(T_size, size));
im1 = builder.CreateMul(im1, ConstantInt::get(T_size,
LLT_ALIGN(size, ((jl_datatype_t*)ety)->alignment)));
thePtr = builder.CreateGEP(builder.CreateBitCast(thePtr, T_pint8), im1);
builder.CreateMemCpy(builder.CreateBitCast(emit_nthptr_addr(strct, (size_t)1), T_pint8),
thePtr, size, 1);
Expand Down Expand Up @@ -682,8 +683,10 @@ static Value *emit_pointerset(jl_value_t *e, jl_value_t *x, jl_value_t *i, jl_co
assert(val->getType() == jl_pvalue_llvmt); //Boxed
assert(jl_is_datatype(ety));
uint64_t size = ((jl_datatype_t*)ety)->size;
im1 = builder.CreateMul(im1, ConstantInt::get(T_size,
LLT_ALIGN(size, ((jl_datatype_t*)ety)->alignment)));
builder.CreateMemCpy(builder.CreateGEP(builder.CreateBitCast(thePtr, T_pint8), im1),
builder.CreateBitCast(emit_nthptr_addr(val, (size_t)1),T_pint8), size, 1);
builder.CreateBitCast(emit_nthptr_addr(val, (size_t)1), T_pint8), size, 1);
}
else {
if (val == NULL) {
Expand Down
2 changes: 1 addition & 1 deletion src/jl_uv.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ DLLEXPORT void jl_uv_writecb_task(uv_write_t *req, int status)
DLLEXPORT int jl_write_copy(uv_stream_t *stream, const char *str, size_t n, uv_write_t *uvw, void *writecb)
{
JL_SIGATOMIC_BEGIN();
char *data = (char*)(uvw+1);
char *data = (char*)uvw+sizeof(*uvw);
memcpy(data,str,n);
uv_buf_t buf[1];
buf[0].base = data;
Expand Down
9 changes: 7 additions & 2 deletions src/jltypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2684,8 +2684,10 @@ static jl_value_t *type_match_(jl_value_t *child, jl_value_t *parent,
if (jl_is_uniontype(child)) {
jl_tuple_t *t = ((jl_uniontype_t*)child)->types;
if (morespecific) {
cenv_t tenv;
tenv.data = (jl_value_t**)alloca(MAX_CENV_SIZE*sizeof(void*));
jl_value_t **rts;
JL_GC_PUSHARGS(rts, MAX_CENV_SIZE);
cenv_t tenv; tenv.data = rts;
memset(tenv.data, 0, MAX_CENV_SIZE*sizeof(void*));
for(i=0; i < jl_tuple_len(t); i++) {
int n = env->n;
tmp = type_match_(jl_tupleref(t,i), parent, env, 1, invariant);
Expand All @@ -2703,16 +2705,19 @@ static jl_value_t *type_match_(jl_value_t *child, jl_value_t *parent,
type_match_(jl_tupleref(t,j), parent,
env, 1, invariant) == jl_false) {
env->n = n;
JL_GC_POP();
return jl_false;
}
}
JL_GC_POP();
return jl_true;
}
}
else {
env->n = n;
}
}
JL_GC_POP();
return jl_false;
}
else {
Expand Down
5 changes: 2 additions & 3 deletions src/support/ios.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,11 +847,10 @@ ios_t *ios_file(ios_t *s, const char *fname, int rd, int wr, int create, int tru
if (create) flags |= O_CREAT;
if (trunc) flags |= O_TRUNC;
#if defined(_OS_WINDOWS_)
size_t len = strlen(fname)+1;
size_t wlen = MultiByteToWideChar(CP_UTF8, 0, fname, len, NULL, 0);
size_t wlen = MultiByteToWideChar(CP_UTF8, 0, fname, -1, NULL, 0);
if (!wlen) goto open_file_err;
wchar_t *fname_w = (wchar_t*)alloca(wlen*sizeof(wchar_t));
if (!MultiByteToWideChar(CP_UTF8, 0, fname, len, fname_w, wlen)) goto open_file_err;
if (!MultiByteToWideChar(CP_UTF8, 0, fname, -1, fname_w, wlen)) goto open_file_err;
fd = _wopen(fname_w, flags | O_BINARY, _S_IREAD | _S_IWRITE);
#else
fd = open(fname, flags, S_IRUSR | S_IWUSR /* 0600 */ | S_IRGRP | S_IROTH /* 0644 */);
Expand Down
Loading

2 comments on commit 69b84e9

@timholy
Copy link
Member

Choose a reason for hiding this comment

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

I read this through in some detail. I don't know enough to evaluate some of the changes (esp in I/O, intrinsics.cpp), but most looked really good.

However, I'd strongly urge you to split potentially-important "grab bag" commits like this one into something like 5 separate commits, each of which is targeted at a single issue. git bisect is too powerful of a tool to throw under the bus this way.

@timholy
Copy link
Member

Choose a reason for hiding this comment

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

But, of course, thanks for doing this!

Please sign in to comment.