Skip to content

Commit

Permalink
Fix MVM_nativecall_refresh of CStruct always replacing child objects
Browse files Browse the repository at this point in the history
MVM_nativecall_refresh has never properly handled CStruct, CPPStruct and CUnion
members of CStructs, since it always compared the pointer to the 6model
object's body with the pointer contained in the referenced C struct. Since
those were never the same, we always destroyed any existing child objects.

In addition commit e99af05 made it worse by
trying to fix handling of inlined attributes while by changing the logic so
it could only ever work for inlined attributes. However for inlined attributes
we actually don't have to refresh child objects, since they cannot be replaced
at all anyway. After all they are not their own entities but just a part of the
containing struct. So we will always find them at a fixed offset into this
struct.

Fix by reverting the faulty commit, taking the correct pointers for comparison
and just not compare pointers at all for inlined attributes.
  • Loading branch information
niner committed Jan 17, 2021
1 parent 2d9899c commit 7a94f48
Showing 1 changed file with 39 additions and 34 deletions.
73 changes: 39 additions & 34 deletions src/core/nativecall.c
Expand Up @@ -1051,47 +1051,52 @@ void MVM_nativecall_refresh(MVMThreadContext *tc, MVMObject *cthingy) {
for (i = 0; i < repr_data->num_attributes; i++) {
MVMint32 kind = repr_data->attribute_locations[i] & MVM_CSTRUCT_ATTR_MASK;
MVMint32 slot = repr_data->attribute_locations[i] >> MVM_CSTRUCT_ATTR_SHIFT;
void *cptr = NULL; /* Address of the struct member holding the pointer in the C storage. */
void **cptr = NULL; /* Address of the struct member holding the pointer in the C storage. */
void *objptr = NULL; /* The pointer in the object representing the C object. */

if (kind == MVM_CSTRUCT_ATTR_IN_STRUCT || !body->child_objs[slot])
continue;

cptr = (void *)(storage + (uintptr_t)repr_data->struct_offsets[i]);
if (IS_CONCRETE(body->child_objs[slot])) {
switch (kind) {
case MVM_CSTRUCT_ATTR_CARRAY:
objptr = ((MVMCArrayBody *)OBJECT_BODY(body->child_objs[slot]))->storage;
break;
case MVM_CSTRUCT_ATTR_CPTR:
objptr = ((MVMCPointerBody *)OBJECT_BODY(body->child_objs[slot]))->ptr;
break;
case MVM_CSTRUCT_ATTR_CSTRUCT:
objptr = (MVMCStructBody *)OBJECT_BODY(body->child_objs[slot]);
break;
case MVM_CSTRUCT_ATTR_CPPSTRUCT:
objptr = (MVMCPPStructBody *)OBJECT_BODY(body->child_objs[slot]);
break;
case MVM_CSTRUCT_ATTR_CUNION:
objptr = (MVMCUnionBody *)OBJECT_BODY(body->child_objs[slot]);
break;
case MVM_CSTRUCT_ATTR_STRING:
objptr = NULL;
break;
default:
MVM_exception_throw_adhoc(tc,
"Fatal error: bad kind (%d) in CStruct write barrier",
kind);
}
if (repr_data->attribute_locations[i] & MVM_CSTRUCT_ATTR_INLINED) {
MVM_nativecall_refresh(tc, body->child_objs[slot]);
}
else {
objptr = NULL;
}
cptr = (void **)(storage + (uintptr_t)repr_data->struct_offsets[i]);
if (IS_CONCRETE(body->child_objs[slot])) {
switch (kind) {
case MVM_CSTRUCT_ATTR_CARRAY:
objptr = ((MVMCArrayBody *)OBJECT_BODY(body->child_objs[slot]))->storage;
break;
case MVM_CSTRUCT_ATTR_CPTR:
objptr = ((MVMCPointerBody *)OBJECT_BODY(body->child_objs[slot]))->ptr;
break;
case MVM_CSTRUCT_ATTR_CSTRUCT:
objptr = ((MVMCStructBody *)OBJECT_BODY(body->child_objs[slot]))->cstruct;
break;
case MVM_CSTRUCT_ATTR_CPPSTRUCT:
objptr = ((MVMCPPStructBody *)OBJECT_BODY(body->child_objs[slot]))->cppstruct;
break;
case MVM_CSTRUCT_ATTR_CUNION:
objptr = ((MVMCUnionBody *)OBJECT_BODY(body->child_objs[slot]))->cunion;
break;
case MVM_CSTRUCT_ATTR_STRING:
objptr = NULL;
break;
default:
MVM_exception_throw_adhoc(tc,
"Fatal error: bad kind (%d) in CStruct write barrier",
kind);
}
}
else {
objptr = NULL;
}

if (objptr != cptr)
body->child_objs[slot] = NULL;
else
MVM_nativecall_refresh(tc, body->child_objs[slot]);
if (objptr != *cptr) {
body->child_objs[slot] = NULL;
}
else
MVM_nativecall_refresh(tc, body->child_objs[slot]);
}
}
}
else if (REPR(cthingy)->ID == MVM_REPR_ID_MVMCPPStruct) {
Expand Down

0 comments on commit 7a94f48

Please sign in to comment.