From 1d274002e59962a62d9132f6da22b77dafe3da23 Mon Sep 17 00:00:00 2001 From: Yichao Yu Date: Tue, 1 Mar 2016 21:11:52 -0500 Subject: [PATCH] Fix missing GC root in table.c (cherry picked from commit 27faae47a6b17c5c1c4ee6b0ea25cb5845fc008d) ref #15318 --- src/dump.c | 1 + src/julia_internal.h | 2 ++ src/table.c | 25 ++++++++++++++++++------- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/dump.c b/src/dump.c index e888f980aec09..7529b7aaeedc7 100644 --- a/src/dump.c +++ b/src/dump.c @@ -1694,6 +1694,7 @@ static void jl_reinit_item(ios_t *f, jl_value_t *v, int how) { switch (how) { case 1: { // rehash ObjectIdDict jl_array_t **a = (jl_array_t**)jl_data_ptr(v); + // Assume *a don't need a write barrier jl_idtable_rehash(a, jl_array_len(*a)); jl_gc_wb(v, *a); break; diff --git a/src/julia_internal.h b/src/julia_internal.h index 84504574a9ca3..d788995373b07 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -136,6 +136,8 @@ extern JL_THREAD void *jl_stackbase; void jl_dump_bitcode(char *fname, const char *sysimg_data, size_t sysimg_len); void jl_dump_objfile(char *fname, int jit_model, const char *sysimg_data, size_t sysimg_len); int32_t jl_get_llvm_gv(jl_value_t *p); +// the first argument to jl_idtable_rehash is used to return a value +// make sure it is rooted if it is used after the function returns void jl_idtable_rehash(jl_array_t **pa, size_t newsz); #ifdef _OS_LINUX_ diff --git a/src/table.c b/src/table.c index eb15018c34f36..8567db1129f77 100644 --- a/src/table.c +++ b/src/table.c @@ -12,26 +12,34 @@ static void **jl_table_lookup_bp(jl_array_t **pa, void *key); void jl_idtable_rehash(jl_array_t **pa, size_t newsz) { + // Assume *pa don't need a write barrier + // pa doesn't have to be a GC slot but *pa needs to be rooted size_t sz = jl_array_len(*pa); size_t i; void **ol = (void**)(*pa)->data; - *pa = jl_alloc_cell_1d(newsz); - // we do not check the write barrier here - // because pa always points to a C stack location - // (see eqtable_put) - // it should be changed if this assumption no longer holds + jl_array_t *newa = jl_alloc_cell_1d(newsz); + // keep the original array in the original slot since we need `ol` + // to be valid in the loop below. + JL_GC_PUSH1(&newa); for(i=0; i < sz; i+=2) { if (ol[i+1] != NULL) { - (*jl_table_lookup_bp(pa, ol[i])) = ol[i+1]; - jl_gc_wb(*pa, ol[i+1]); + (*jl_table_lookup_bp(&newa, ol[i])) = ol[i+1]; + jl_gc_wb(newa, ol[i+1]); // it is however necessary here because allocation // can (and will) occur in a recursive call inside table_lookup_bp } } + *pa = newa; + // we do not check the write barrier here + // because pa always points to a C stack location + // (see jl_eqtable_put and jl_finalize_deserializer) + // it should be changed if this assumption no longer holds + JL_GC_POP(); } static void **jl_table_lookup_bp(jl_array_t **pa, void *key) { + // pa points to a **rooted** gc frame slot uint_t hv; jl_array_t *a = *pa; size_t orig, index, iter; @@ -116,9 +124,12 @@ static void **jl_table_peek_bp(jl_array_t *a, void *key) DLLEXPORT jl_array_t *jl_eqtable_put(jl_array_t *h, void *key, void *val) { + JL_GC_PUSH1(&h); + // &h may be assigned to in jl_idtable_rehash so it need to be rooted void **bp = jl_table_lookup_bp(&h, key); *bp = val; jl_gc_wb(h, val); + JL_GC_POP(); return h; }