fix #34159, crash in deserialize caused by recent layout changes #34163
Conversation
src/dump.c
Outdated
| ios_write(s->s, (char*)&n, sizeof(n)); | ||
| last = ptr + sizeof(n); | ||
| } | ||
| else if (j < np && ptr == (char*)&((jl_value_t**)data)[jl_ptr_offset(t, j)]) { |
There was a problem hiding this comment.
I think this needs while ptr > jl_ptr_offset(t, j) { /* write out previous field */ }
|
Agreed. This is a better approach. I was alternatively also thinking about copying the value into temporary space ( |
JeffBezanson
force-pushed
the
jb/fix34159
branch
from
fe3d8cc
to
36345fb
Compare
src/dump.c
Outdated
| ios_write(s->s, last, prevptr - last); | ||
| jl_value_t *e = *(jl_value_t**)prevptr; | ||
| JL_GC_PROMISE_ROOTED(e); | ||
| if (t->mutabl && e && jl_is_cpointer(e) && jl_unbox_voidpointer(e) != (void*)-1 && jl_unbox_voidpointer(e) != NULL) |
There was a problem hiding this comment.
You probably want jl_field_type(t, i)->mutabl here (but that's always false)
| if (t->mutabl && e && jl_is_cpointer(e) && jl_unbox_voidpointer(e) != (void*)-1 && jl_unbox_voidpointer(e) != NULL) | |
| if (0) |
There was a problem hiding this comment.
Oh right, we need some kind of check for whether this is a direct field of the object. Maybe checking that jl_field_type(t, i-1) is concrete and inlinealloc?
There was a problem hiding this comment.
Ah, right. We probably want t->mutable && jl_field_isptr(t, i - 1) && jl_is_cpointer(e) && then?
JeffBezanson
force-pushed
the
jb/fix34159
branch
from
36345fb
to
ec28a0f
Compare
The field types are not necessarily available at this point in deserialization. I also like this implementation a bit better since it always handles null-ing C pointers on the serialize side.