Skip to content

Commit 08dee38

Browse files
committed
fix #8200
segfault due to incorrect serialization of singleton objects with fields. now we have a more robust approach to this that works as long as datatype->instance always points to a minimum-size object.
1 parent d69c193 commit 08dee38

File tree

2 files changed

+33
-50
lines changed

2 files changed

+33
-50
lines changed

src/dump.c

+32-49
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ static const ptrint_t SmallInt64_tag = 27;
2929
static const ptrint_t IdTable_tag = 28;
3030
static const ptrint_t Int32_tag = 29;
3131
static const ptrint_t Array1d_tag = 30;
32+
static const ptrint_t Singleton_tag = 31;
3233
static const ptrint_t Null_tag = 253;
3334
static const ptrint_t ShortBackRef_tag = 254;
3435
static const ptrint_t BackRef_tag = 255;
@@ -95,7 +96,6 @@ static void write_as_tag(ios_t *s, uint8_t tag)
9596
#define jl_serialize_value(s, v) jl_serialize_value_(s,(jl_value_t*)(v))
9697
static void jl_serialize_value_(ios_t *s, jl_value_t *v);
9798
static jl_value_t *jl_deserialize_value(ios_t *s);
98-
static jl_value_t *jl_deserialize_value_internal(ios_t *s);
9999
jl_value_t ***sysimg_gvars = NULL;
100100

101101
extern int globalUnique;
@@ -286,24 +286,24 @@ static void jl_serialize_datatype(ios_t *s, jl_datatype_t *dt)
286286
size_t nf = jl_tuple_len(dt->names);
287287
write_uint16(s, nf);
288288
write_int32(s, dt->size);
289+
int has_instance = !!(dt->instance != NULL);
290+
write_uint8(s, dt->abstract | (dt->mutabl<<1) | (dt->pointerfree<<2) | (has_instance<<3));
291+
if (!dt->abstract)
292+
write_int32(s, dt->uid);
293+
if (has_instance)
294+
jl_serialize_value(s, dt->instance);
289295
if (nf > 0) {
290296
write_int32(s, dt->alignment);
291297
ios_write(s, (char*)&dt->fields[0], nf*sizeof(jl_fielddesc_t));
292298
jl_serialize_value(s, dt->names);
293299
jl_serialize_value(s, dt->types);
294300
}
295-
int has_instance = !!(dt->instance != NULL);
296-
write_uint8(s, dt->abstract | (dt->mutabl<<1) | (dt->pointerfree<<2) | (has_instance<<3));
297-
if (!dt->abstract)
298-
write_int32(s, dt->uid);
299301

300302
jl_serialize_value(s, dt->parameters);
301303
jl_serialize_value(s, dt->name);
302304
jl_serialize_value(s, dt->super);
303305
jl_serialize_value(s, dt->env);
304306
jl_serialize_value(s, dt->linfo);
305-
if (has_instance)
306-
jl_serialize_value(s, dt->instance);
307307
jl_serialize_fptr(s, (void*)dt->fptr);
308308
}
309309

@@ -546,6 +546,10 @@ static void jl_serialize_value_(ios_t *s, jl_value_t *v)
546546
write_int32(s, (int32_t)*(int32_t*)data);
547547
}
548548
else {
549+
if (v == t->instance) {
550+
writetag(s, (jl_value_t*)Singleton_tag);
551+
return;
552+
}
549553
if ((jl_value_t*)t == jl_idtable_type)
550554
writetag(s, (jl_value_t*)IdTable_tag);
551555
else
@@ -598,13 +602,12 @@ static jl_fptr_t jl_deserialize_fptr(ios_t *s)
598602
return *(jl_fptr_t*)pbp;
599603
}
600604

601-
#define DTINSTANCE_PLACEHOLDER ((void*)2)
602-
603605
static jl_value_t *jl_deserialize_datatype(ios_t *s, int pos)
604606
{
605607
int tag = read_uint8(s);
606608
uint16_t nf = read_uint16(s);
607609
size_t size = read_int32(s);
610+
uint8_t flags = read_uint8(s);
608611
jl_datatype_t *dt;
609612
if (tag == 2)
610613
dt = jl_int32_type;
@@ -617,7 +620,18 @@ static jl_value_t *jl_deserialize_datatype(ios_t *s, int pos)
617620
dt->size = size;
618621
dt->struct_decl = NULL;
619622
dt->instance = NULL;
620-
623+
dt->abstract = flags&1;
624+
dt->mutabl = (flags>>1)&1;
625+
dt->pointerfree = (flags>>2)&1;
626+
if (!dt->abstract)
627+
dt->uid = read_int32(s);
628+
else
629+
dt->uid = 0;
630+
int has_instance = (flags>>3)&1;
631+
if (has_instance) {
632+
dt->instance = jl_deserialize_value(s);
633+
dt->instance->type = (jl_value_t*)dt;
634+
}
621635
assert(tree_literal_values==NULL);
622636
ptrhash_put(&backref_table, (void*)(ptrint_t)pos, dt);
623637

@@ -633,26 +647,11 @@ static jl_value_t *jl_deserialize_datatype(ios_t *s, int pos)
633647
dt->alignment = MAX_ALIGN;
634648
dt->names = dt->types = jl_null;
635649
}
636-
uint8_t flags = read_uint8(s);
637-
dt->abstract = flags&1;
638-
dt->mutabl = (flags>>1)&1;
639-
dt->pointerfree = (flags>>2)&1;
640-
int has_instance = (flags>>3)&1;
641-
if (!dt->abstract)
642-
dt->uid = read_int32(s);
643-
else
644-
dt->uid = 0;
645650
dt->parameters = (jl_tuple_t*)jl_deserialize_value(s);
646651
dt->name = (jl_typename_t*)jl_deserialize_value(s);
647652
dt->super = (jl_datatype_t*)jl_deserialize_value(s);
648653
dt->env = jl_deserialize_value(s);
649654
dt->linfo = (jl_lambda_info_t*)jl_deserialize_value(s);
650-
if (has_instance) {
651-
jl_value_t *instance = (jl_value_t*)jl_deserialize_value_internal(s);
652-
if (instance == DTINSTANCE_PLACEHOLDER)
653-
instance = jl_new_struct_uninit(dt);
654-
dt->instance = instance;
655-
}
656655
dt->fptr = jl_deserialize_fptr(s);
657656
if (dt->name == jl_array_type->name || dt->name == jl_pointer_type->name ||
658657
dt->name == jl_type_type->name || dt->name == jl_vararg_type->name ||
@@ -663,14 +662,12 @@ static jl_value_t *jl_deserialize_datatype(ios_t *s, int pos)
663662
// parametric types here.
664663
jl_cell_1d_push(datatype_list, (jl_value_t*)dt);
665664
}
666-
667665
return (jl_value_t*)dt;
668666
}
669667

670668
jl_array_t *jl_eqtable_put(jl_array_t *h, void *key, void *val);
671669

672-
// Internal jl_deserialize_value. May return the placeholder value DTINSTANCE_PLACEHOLDER, unlike jl_deserialize_value
673-
static jl_value_t *jl_deserialize_value_internal(ios_t *s)
670+
static jl_value_t *jl_deserialize_value(ios_t *s)
674671
{
675672
int pos = ios_pos(s);
676673
int32_t tag = read_uint8(s);
@@ -873,14 +870,6 @@ static jl_value_t *jl_deserialize_value_internal(ios_t *s)
873870
return v;
874871
}
875872
else if (vtag == (jl_value_t*)jl_datatype_type || vtag == (jl_value_t*)IdTable_tag) {
876-
// If the value v we are about to deserialize is some dt->instance, we have a circular
877-
// reference, because v == v->type->instance. To avoid this, we put a null value in
878-
// the backref table to reserve the space. If jl_deserialize_value encounters this,
879-
// it knows to do the allocation itself. This work, because in all instances where
880-
// v->type->instance != null, v->type has no fields, so there is no further serialized
881-
// data stored that we would need to construct the type.
882-
if (usetable)
883-
ptrhash_put(&backref_table, (void*)(ptrint_t)pos, DTINSTANCE_PLACEHOLDER);
884873
jl_datatype_t *dt = (jl_datatype_t*)jl_deserialize_value(s);
885874
if (dt == jl_datatype_type)
886875
return jl_deserialize_datatype(s, pos);
@@ -913,11 +902,6 @@ static jl_value_t *jl_deserialize_value_internal(ios_t *s)
913902
ptrhash_put(&backref_table, (void*)(ptrint_t)pos, v);
914903
}
915904
else {
916-
if (dt->instance) {
917-
if (usetable)
918-
*ptrhash_bp(&backref_table, (void*)(ptrint_t)pos) = dt->instance;
919-
return dt->instance;
920-
}
921905
v = jl_new_struct_uninit(dt);
922906
if (usetable)
923907
ptrhash_put(&backref_table, (void*)(ptrint_t)pos, v);
@@ -941,17 +925,16 @@ static jl_value_t *jl_deserialize_value_internal(ios_t *s)
941925
// TODO: put WeakRefs on the weak_refs list
942926
return v;
943927
}
928+
else if (vtag == (jl_value_t*)Singleton_tag) {
929+
jl_value_t *v = allocobj(sizeof(void*));
930+
if (usetable)
931+
ptrhash_put(&backref_table, (void*)(ptrint_t)pos, v);
932+
return v;
933+
}
944934
assert(0);
945935
return NULL;
946936
}
947937

948-
static jl_value_t *jl_deserialize_value(ios_t *s)
949-
{
950-
jl_value_t *v = jl_deserialize_value_internal(s);
951-
assert(v != DTINSTANCE_PLACEHOLDER);
952-
return v;
953-
}
954-
955938
// --- entry points ---
956939

957940
extern jl_array_t *jl_module_init_order;
@@ -1215,7 +1198,7 @@ void jl_init_serializer(void)
12151198
jl_expr_type, (void*)LongSymbol_tag, (void*)LongTuple_tag,
12161199
(void*)LongExpr_tag, (void*)LiteralVal_tag,
12171200
(void*)SmallInt64_tag, (void*)IdTable_tag,
1218-
(void*)Int32_tag, (void*)Array1d_tag,
1201+
(void*)Int32_tag, (void*)Array1d_tag, (void*)Singleton_tag,
12191202
jl_module_type, jl_tvar_type, jl_lambda_info_type,
12201203

12211204
jl_null, jl_false, jl_true, jl_any_type, jl_symbol("Any"),

src/julia.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ STATIC_INLINE int jl_isbits(void *t) // corresponding to isbits() in julia
556556

557557
STATIC_INLINE int jl_is_datatype_singleton(jl_datatype_t *d)
558558
{
559-
return (d->size == 0 && (d->names==jl_null || !d->mutabl));
559+
return (!d->abstract && d->size == 0 && (d->names==jl_null || !d->mutabl));
560560
}
561561

562562
STATIC_INLINE int jl_is_abstracttype(void *v)

0 commit comments

Comments
 (0)