suzanne.soy/racket
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix potential bad interaction of module caching and JIT assumption

Browse Source 
The JIT wants to detect calls to functions bound to module-level
variables that are always instantiated to a procedure of a particular
arity, in which case it can avoid some checks.  The problem is that
bytecode and JITted code can be shared via the module cache across
namespaces that use different modules to implement a particular module
path or that enable the JIT differently.  In particular, starting with
one that has a procedure binding and then using one (in a different
namespace) that has a non-procedure binding could lead to a crash.

Defend against this possibility by never treating imported variables
as constant in that sense. The JIT detects imported variables through
a new import map in the prefix.

This change may slow code. My guess is that it will have no
performance impact in practice, due to cross-module inlining,
although I can construct a microbenchmark that slows by 20%.
This commit is contained in:
Matthew Flatt 2012-06-27 09:16:38 -06:00
parent e0bf863b21
commit 0d1056dbed
10 changed files with 138 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

58
collects/tests/racket/module.rktl
View File

@ -819,6 +819,64 @@
(compile-eval m1-expr)
(compile-eval m2-expr)))
;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Check JIT treatement of seemingly constant imports
(let ()
(define (a-expr mut?)
`(module a racket/base
,(if mut?
`(define a 5)
`(define (a x)
;; long enough to not be inlined:
(list x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x x)))
(provide a)))
(define b-expr
`(module b racket/base
(require 'a)
(define (b q) (a q))
(provide b)))
(define (compile-m e strs)
(parameterize ([current-namespace (make-base-namespace)])
(for ([str (in-list strs)])
(parameterize ([read-accept-compiled #t])
(eval (read (open-input-bytes str)))))
(define o (open-output-bytes))
(write (compile e) o)
(define s (get-output-bytes o))
(define vlen (bytes-ref s 2))
;; Add a hash, so that loading this module in two contexts tries to
;; use the same loaded bytecode and same JIT-generated code:
(bytes-copy! s (+ 4 vlen)
(subbytes
(bytes-append (string->bytes/utf-8 (format "~s" (bytes-length s)))
(make-bytes 20 0))
0
20))
s))
(define a-s (compile-m (a-expr #f) '()))
(define am-s (compile-m (a-expr #t) '()))
(define b-s (compile-m b-expr (list a-s)))
(define (go a-s)
(parameterize ([current-namespace (make-base-namespace)])
(parameterize ([read-accept-compiled #t])
(eval (read (open-input-bytes a-s)))
(define temp-dir (find-system-path 'temp-dir))
(define dir (build-path temp-dir "compiled"))
(make-directory* dir)
(with-output-to-file (build-path dir "check-gen_rkt.zo")
#:exists 'truncate
(lambda () (write-bytes b-s)))
((dynamic-require (build-path temp-dir "check-gen.rkt") 'b) 10)
(delete-file (build-path dir "check-gen_rkt.zo")))))
;; Triger JIT generation with constant function as `a':
(go a-s)
;; Check that we don't crash when trying to use a different `a':
(err/rt-test (go am-s)))
;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(report-errs)

33
src/racket/src/eval.c
View File

@ -786,7 +786,8 @@ static Scheme_Object *link_module_variable(Scheme_Object *modidx,
int check_access, Scheme_Object *insp,
int pos, int mod_phase,
Scheme_Env *env,
Scheme_Object **exprs, int which)
Scheme_Object **exprs, int which,
char *import_map)
{
Scheme_Object *modname;
Scheme_Env *menv;
@ -856,6 +857,9 @@ static Scheme_Object *link_module_variable(Scheme_Object *modidx,
if (!(((Scheme_Bucket_With_Flags *)bkt)->flags & (GLOB_IS_IMMUTATED | GLOB_IS_LINKED)))
((Scheme_Bucket_With_Flags *)bkt)->flags |= GLOB_IS_LINKED;
}
if (!self && !(import_map[which >> 3] & (1 << (which & 0x7))))
import_map[which >> 3] |= (1 << (which & 0x7));
return (Scheme_Object *)bkt;
}
@ -863,7 +867,8 @@ static Scheme_Object *link_module_variable(Scheme_Object *modidx,
static Scheme_Object *link_toplevel(Scheme_Object **exprs, int which, Scheme_Env *env,
Scheme_Object *src_modidx,
Scheme_Object *dest_modidx,
Scheme_Object *insp)
Scheme_Object *insp,
char *import_map)
{
Scheme_Object *expr = exprs[which];
@ -896,14 +901,15 @@ static Scheme_Object *link_toplevel(Scheme_Object **exprs, int which, Scheme_Env
if (SCHEME_PAIRP(modname)) {
mod_phase = SCHEME_INT_VAL(SCHEME_CDR(modname));
modname = SCHEME_CAR(modname);
}
}
}
return link_module_variable(modname,
varname,
0, NULL,
-1, mod_phase,
env,
NULL, 0);
NULL, 0,
import_map);
} else if (SAME_TYPE(SCHEME_TYPE(expr), scheme_variable_type)) {
Scheme_Bucket *b = (Scheme_Bucket *)expr;
Scheme_Env *home;
@ -918,7 +924,8 @@ static Scheme_Object *link_toplevel(Scheme_Object **exprs, int which, Scheme_Env
1, home->module->insp,
-1, home->mod_phase,
env,
exprs, which);
exprs, which,
import_map);
} else {
Module_Variable *mv = (Module_Variable *)expr;
@ -931,7 +938,8 @@ static Scheme_Object *link_toplevel(Scheme_Object **exprs, int which, Scheme_Env
mv->sym, 1, (mv->insp ? mv->insp : insp),
mv->pos, mv->mod_phase,
env,
exprs, which);
exprs, which,
import_map);
}
}
@ -5437,7 +5445,8 @@ Scheme_Object **scheme_push_prefix(Scheme_Env *genv, Resolve_Prefix *rp,
{
Scheme_Object **rs_save, **rs, *v;
Scheme_Prefix *pf;
int i, j, tl_map_len;
char *import_map;
int i, j, tl_map_len, import_map_len;
rs_save = rs = MZ_RUNSTACK;
@ -5458,6 +5467,13 @@ Scheme_Object **scheme_push_prefix(Scheme_Env *genv, Resolve_Prefix *rp,
i += rp->num_lifts;
tl_map_len = ((rp->num_toplevels + rp->num_lifts) + 31) / 32;
import_map_len = (rp->num_toplevels + 7) / 8;
if (import_map_len) {
import_map = GC_malloc_atomic(import_map_len);
memset(import_map, 0, import_map_len);
} else
import_map = NULL;
pf = scheme_malloc_tagged(sizeof(Scheme_Prefix)
+ ((i-mzFLEX_DELTA) * sizeof(Scheme_Object *))
@ -5466,6 +5482,7 @@ Scheme_Object **scheme_push_prefix(Scheme_Env *genv, Resolve_Prefix *rp,
pf->num_slots = i;
pf->num_toplevels = rp->num_toplevels;
pf->num_stxes = rp->num_stxes;
pf->import_map = import_map;
--rs;
MZ_RUNSTACK = rs;
rs[0] = (Scheme_Object *)pf;
@ -5473,7 +5490,7 @@ Scheme_Object **scheme_push_prefix(Scheme_Env *genv, Resolve_Prefix *rp,
for (i = 0; i < rp->num_toplevels; i++) {
v = rp->toplevels[i];
if (genv || SCHEME_FALSEP(v))
v = link_toplevel(rp->toplevels, i, genv ? genv : dummy_env, src_modidx, now_modidx, insp);
v = link_toplevel(rp->toplevels, i, genv ? genv : dummy_env, src_modidx, now_modidx, insp, import_map);
pf->a[i] = v;
}

50
src/racket/src/jit.c
View File

@ -458,14 +458,28 @@ static int no_sync_change(Scheme_Object *obj, int fuel)
}
}
Scheme_Object *scheme_extract_global(Scheme_Object *o, Scheme_Native_Closure *nc)
Scheme_Object *scheme_extract_global(Scheme_Object *o, Scheme_Native_Closure *nc, int local_only)
{
/* GLOBAL ASSUMPTION: we assume that globals are the last thing
in the closure; grep for "GLOBAL ASSUMPTION" in fun.c. */
Scheme_Prefix *globs;
int pos;
globs = (Scheme_Prefix *)nc->vals[nc->code->u2.orig_code->closure_size - 1];
return globs->a[SCHEME_TOPLEVEL_POS(o)];
pos = SCHEME_TOPLEVEL_POS(o);
if (local_only) {
/* Usually, we look for local bindings only, because module caching means
that JIT-generated code can be linked to different other modules that
may have different bindings, even though we expect them binding to be
consistent. */
if (pos < globs->num_toplevels) {
if (globs->import_map[pos >> 3] & (1 << (pos & 7)))
return NULL;
}
}
return globs->a[pos];
}
Scheme_Object *scheme_extract_closure_local(Scheme_Object *obj, mz_jit_state *jitter, int extra_push)
@ -511,17 +525,19 @@ int scheme_is_noncm(Scheme_Object *a, mz_jit_state *jitter, int depth, int stack
&& SAME_TYPE(SCHEME_TYPE(a), scheme_toplevel_type)
&& ((SCHEME_TOPLEVEL_FLAGS(a) & SCHEME_TOPLEVEL_FLAGS_MASK) >= SCHEME_TOPLEVEL_CONST)) {
Scheme_Object *p;
p = scheme_extract_global(a, jitter->nc);
p = ((Scheme_Bucket *)p)->val;
if (p && SAME_TYPE(SCHEME_TYPE(p), scheme_native_closure_type)) {
Scheme_Native_Closure_Data *ndata = ((Scheme_Native_Closure *)p)->code;
if (ndata->closure_size >= 0) { /* not case-lambda */
if (lambda_has_been_jitted(ndata)) {
if (SCHEME_NATIVE_CLOSURE_DATA_FLAGS(ndata) & NATIVE_PRESERVES_MARKS)
return 1;
} else {
if (SCHEME_CLOSURE_DATA_FLAGS(ndata->u2.orig_code) & CLOS_PRESERVES_MARKS)
return 1;
p = scheme_extract_global(a, jitter->nc, 1);
if (p) {
p = ((Scheme_Bucket *)p)->val;
if (p && SAME_TYPE(SCHEME_TYPE(p), scheme_native_closure_type)) {
Scheme_Native_Closure_Data *ndata = ((Scheme_Native_Closure *)p)->code;
if (ndata->closure_size >= 0) { /* not case-lambda */
if (lambda_has_been_jitted(ndata)) {
if (SCHEME_NATIVE_CLOSURE_DATA_FLAGS(ndata) & NATIVE_PRESERVES_MARKS)
return 1;
} else {
if (SCHEME_CLOSURE_DATA_FLAGS(ndata->u2.orig_code) & CLOS_PRESERVES_MARKS)
return 1;
}
}
}
}
@ -731,9 +747,11 @@ static int is_a_procedure(Scheme_Object *v, mz_jit_state *jitter)
if (jitter->nc) {
Scheme_Object *p;
p = scheme_extract_global(v, jitter->nc);
p = ((Scheme_Bucket *)p)->val;
return SAME_TYPE(SCHEME_TYPE(p), scheme_native_closure_type);
p = scheme_extract_global(v, jitter->nc, 1);
if (p) {
p = ((Scheme_Bucket *)p)->val;
return SAME_TYPE(SCHEME_TYPE(p), scheme_native_closure_type);
}
}
}
}

2
src/racket/src/jit.h
View File

@ -1282,7 +1282,7 @@ int scheme_jit_check_closure_flonum_bit(Scheme_Closure_Data *data, int pos, int
# define CLOSURE_CONTENT_IS_FLONUM(data, pos) scheme_jit_check_closure_flonum_bit(data, pos, data->num_params)
#endif
Scheme_Object *scheme_extract_global(Scheme_Object *o, Scheme_Native_Closure *nc);
Scheme_Object *scheme_extract_global(Scheme_Object *o, Scheme_Native_Closure *nc, int local_only);
Scheme_Object *scheme_extract_closure_local(Scheme_Object *obj, mz_jit_state *jitter, int extra_push);
void scheme_jit_register_traversers(void);

34
src/racket/src/jitcall.c
View File

@ -1468,36 +1468,30 @@ int scheme_generate_app(Scheme_App_Rec *app, Scheme_Object **alt_rands, int num_
}
}
} else if (t == scheme_toplevel_type) {
if (0 && (SCHEME_TOPLEVEL_FLAGS(rator) & SCHEME_TOPLEVEL_FLAGS_MASK) >= SCHEME_TOPLEVEL_FIXED) {
if ((SCHEME_TOPLEVEL_FLAGS(rator) & SCHEME_TOPLEVEL_FLAGS_MASK) >= SCHEME_TOPLEVEL_FIXED) {
/* We can re-order evaluation of the rator. */
reorder_ok = 1;
if (jitter->nc && 0
if (jitter->nc
&& ((SCHEME_TOPLEVEL_FLAGS(rator) & SCHEME_TOPLEVEL_FLAGS_MASK) >= SCHEME_TOPLEVEL_CONST)) {
Scheme_Object *p;
p = scheme_extract_global(rator, jitter->nc);
p = ((Scheme_Bucket *)p)->val;
if (can_direct_native(p, num_rands, &extract_case)) {
direct_native = 1;
p = scheme_extract_global(rator, jitter->nc, 1);
if (p) {
p = ((Scheme_Bucket *)p)->val;
if (can_direct_native(p, num_rands, &extract_case)) {
direct_native = 1;
if ((SCHEME_TOPLEVEL_POS(rator) == jitter->self_toplevel_pos)
&& (num_rands < MAX_SHARED_CALL_RANDS)) {
if (is_tail)
direct_self = 1;
else if (jitter->self_nontail_code)
nontail_self = 1;
if ((SCHEME_TOPLEVEL_POS(rator) == jitter->self_toplevel_pos)
&& (num_rands < MAX_SHARED_CALL_RANDS)) {
if (is_tail)
direct_self = 1;
else if (jitter->self_nontail_code)
nontail_self = 1;
}
}
}
}
} else if (jitter->nc) {
Scheme_Object *p;
p = scheme_extract_global(rator, jitter->nc);
if (((Scheme_Bucket_With_Flags *)p)->flags & GLOB_IS_CONSISTENT) {
if (can_direct_native(((Scheme_Bucket *)p)->val, num_rands, &extract_case))
direct_native = 1;
}
}
} else if (SAME_TYPE(t, scheme_closure_type)) {
Scheme_Closure_Data *data;

2
src/racket/src/jitinline.c
View File

@ -147,7 +147,7 @@ static int inlineable_struct_prim(Scheme_Object *o, mz_jit_state *jitter, int ex
if (jitter->nc) {
if (SAME_TYPE(SCHEME_TYPE(o), scheme_toplevel_type)) {
Scheme_Object *p;
p = scheme_extract_global(o, jitter->nc);
p = scheme_extract_global(o, jitter->nc, 0);
p = ((Scheme_Bucket *)p)->val;
return check_val_struct_prim(p, arity);
} else if (SAME_TYPE(SCHEME_TYPE(o), scheme_local_type)) {

1
src/racket/src/mzclpf_post.inc
View File

@ -18,6 +18,7 @@
if (!pf->next_final) {
/* We're the first to look at this prefix... */
gcMARK2(pf->import_map, gc);
if (pf->num_stxes) {
/* Mark all syntax-object references */
for (i = pf->num_stxes+1; i--;) {

2
src/racket/src/mzmark_type.inc
View File

@ -2356,6 +2356,7 @@ static int prefix_val_MARK(void *p, struct NewGC *gc) {
int i;
for (i = pf->num_slots; i--; )
gcMARK2(pf->a[i], gc);
gcMARK2(pf->import_map, gc);
return
gcBYTES_TO_WORDS((sizeof(Scheme_Prefix)
+ ((pf->num_slots-mzFLEX_DELTA) * sizeof(Scheme_Object *))
@ -2368,6 +2369,7 @@ static int prefix_val_FIXUP(void *p, struct NewGC *gc) {
int i;
for (i = pf->num_slots; i--; )
gcFIXUP2(pf->a[i], gc);
gcFIXUP2(pf->import_map, gc);
return
gcBYTES_TO_WORDS((sizeof(Scheme_Prefix)
+ ((pf->num_slots-mzFLEX_DELTA) * sizeof(Scheme_Object *))

1
src/racket/src/mzmarksrc.c
View File

@ -946,6 +946,7 @@ prefix_val {
int i;
for (i = pf->num_slots; i--; )
gcMARK2(pf->a[i], gc);
gcMARK2(pf->import_map, gc);
size:
gcBYTES_TO_WORDS((sizeof(Scheme_Prefix)
+ ((pf->num_slots-mzFLEX_DELTA) * sizeof(Scheme_Object *))

1
src/racket/src/schpriv.h
View File

@ -2127,6 +2127,7 @@ typedef struct Scheme_Prefix
Scheme_Object so; /* scheme_prefix_type */
int num_slots, num_toplevels, num_stxes;
struct Scheme_Prefix *next_final; /* for special GC handling */
char *import_map; /* bitmap indicating which toplevels are imported */
Scheme_Object *a[mzFLEX_ARRAY_DECL]; /* array of objects */
/* followed by an array of `int's for tl_map uses */
} Scheme_Prefix;