Skip to content

Commit

Permalink
allow proper backref handling of the element that happened to land at…
Browse files Browse the repository at this point in the history 
… index HT_NOTFOUND
  • Loading branch information
@vtjnash
vtjnash committed Jul 17, 2015
1 parent 39bf819 commit d81f6d2
Showing 1 changed file with 12 additions and 8 deletions.
20 changes: 12 additions & 8 deletions src/dump.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,10 @@ static void jl_serialize_globalvals(ios_t *s)
for(i=0; i < len; i+=2) {
void *offs = p[i+1];
if (offs != HT_NOTFOUND) {
uintptr_t pos = offs - HT_NOTFOUND - 1;

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Jul 19, 2015

Contributor

MSVC complains that void * is of unknown size here and the other lines that you are doing arithmetic on HT_NOTFOUND. Should it be cast to uintptr_t everywhere?

This comment has been minimized.

Sign in to view

Copy link
@yuyichao

yuyichao Jul 19, 2015

Contributor

I believe gcc also compain about doing arithmetic on void*. Either char* or uintptr_t should be fine.

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Jul 19, 2015

Contributor

hm thanks, trying to do (uintptr_t)HT_NOTFOUND doesn't help, instead we get

dump.c(293) : error C2036: 'void *' : unknown size
dump.c(293) : error C2440: 'initializing' : cannot convert from 'void *' to 'uin
tptr_t'
        There is no context in which this conversion is possible

This comment has been minimized.

Sign in to view

Copy link
@tkelman

tkelman Jul 19, 2015

Contributor

https://gist.github.com/7eaf463be8371efc84f7 seems to work but there are some other inconsistent linkage issues in gc that I'm bisecting on
int32_t gv = jl_get_llvm_gv((jl_value_t*)p[i]);
if (gv != 0) {
write_int32(s, (int)(intptr_t)offs);
write_int32(s, pos);
write_int32(s, gv);
}
}
Expand Down Expand Up @@ -445,7 +446,7 @@ static void jl_serialize_datatype(ios_t *s, jl_datatype_t *dt)
// also flag this in the backref table as special
uptrint_t *bp = (uptrint_t*)ptrhash_bp(&backref_table, dt);
assert(*bp != (uptrint_t)HT_NOTFOUND);
*bp |= 1;
*bp |= 1; assert(((uptrint_t)HT_NOTFOUND)|1);
}
}
else if (dt == jl_int32_type)
Expand Down Expand Up @@ -507,8 +508,10 @@ static void jl_serialize_module(ios_t *s, jl_module_t *m)
write_int8(s, ref_only);
}
jl_serialize_value(s, m->parent);
if (ref_only)
if (ref_only) {
assert(m->parent != m);
return;
}
size_t i;
void **table = m->bindings.table;
for(i=1; i < m->bindings.size; i+=2) {
Expand Down Expand Up @@ -601,13 +604,14 @@ static void jl_serialize_value_(ios_t *s, jl_value_t *v)
else {
bp = ptrhash_bp(&backref_table, v);
if (*bp != HT_NOTFOUND) {
uintptr_t pos = *bp - HT_NOTFOUND - 1;
if ((uptrint_t)*bp < 65536) {

This comment has been minimized.

Sign in to view

Copy link
@ScottPJones

ScottPJones Jul 19, 2015

Contributor

Shouldn't that test pos instead of *bp?

This comment has been minimized.

Sign in to view

Copy link
@vtjnash

vtjnash Jul 19, 2015

Author Member

yes
write_uint8(s, ShortBackRef_tag);
write_uint16(s, (uptrint_t)*bp);
write_uint16(s, pos);
}
else {
write_uint8(s, BackRef_tag);
write_int32(s, (uptrint_t)*bp);
write_int32(s, pos);
}
return;
}
Expand All @@ -627,7 +631,7 @@ static void jl_serialize_value_(ios_t *s, jl_value_t *v)
}
if (mode == MODE_MODULE || mode == MODE_MODULE_POSTWORK)
pos <<= 1;
ptrhash_put(&backref_table, v, (void*)pos);
ptrhash_put(&backref_table, v, HT_NOTFOUND + pos + 1);
}

size_t i;
Expand Down Expand Up @@ -809,7 +813,7 @@ static void jl_serialize_value_(ios_t *s, jl_value_t *v)
// also flag this in the backref table as special
uptrint_t *bp = (uptrint_t*)ptrhash_bp(&backref_table, v);
assert(*bp != (uptrint_t)HT_NOTFOUND);
*bp |= 1;
*bp |= 1; assert(((uptrint_t)HT_NOTFOUND)|1);
}
writetag(s, (jl_value_t*)Singleton_tag);
jl_serialize_value(s, t);
Expand Down Expand Up @@ -1881,7 +1885,7 @@ DLLEXPORT int jl_save_incremental(const char *fname, jl_array_t *worklist)
JL_SIGATOMIC_BEGIN();
arraylist_new(&reinit_list, 0);
htable_new(&backref_table, 5000);
ptrhash_put(&backref_table, jl_main_module, (void*)(uintptr_t)0);
ptrhash_put(&backref_table, jl_main_module, HT_NOTFOUND + 1);
backref_table_numel = 1;
jl_idtable_type = jl_base_module ? jl_get_global(jl_base_module, jl_symbol("ObjectIdDict")) : NULL;

Expand Down

0 comments on commit d81f6d2

Please sign in to comment.