Skip to content

Commit

Permalink
Fix nested runloops leaving call stack in confused state
Browse files Browse the repository at this point in the history
Certain actions (like return) unwind the call stack to the next record
representing a call frame. When returning from a nested runloop (e.g. a
NativeCall callback) however, we must only unwind the records added by
the code running in the nested runloop. Otherwise we can end up unwinding
records that are still needed by a currently running dispatch in the outer
runloop.

To fix this We add a new kind of callstack record representing the boundary of
a nested runloop and holding a reference to the outer frame (because return
needs to know the frame to return to). Also make sure that the outer frame's
return type and reference to the register into which to write the return
value are restored fully.
  • Loading branch information
niner committed Oct 9, 2021
1 parent d490109 commit d7379be
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 15 deletions.
19 changes: 19 additions & 0 deletions src/core/callstack.c
Expand Up @@ -69,6 +69,7 @@ char * record_name(MVMuint8 kind) {
case MVM_CALLSTACK_RECORD_BIND_CONTROL: return "bind control";
case MVM_CALLSTACK_RECORD_ARGS_FROM_C: return "args from C";
case MVM_CALLSTACK_RECORD_DEOPTED_RESUME_INIT: return "deoptimized resume init";
case MVM_CALLSTACK_RECORD_NESTED_RUNLOOP: return "nested runloop";
default: return "unknown";
}
}
Expand Down Expand Up @@ -121,6 +122,8 @@ size_t record_size(MVMCallStackRecord *record) {
return to_8_bytes(sizeof(MVMCallStackDeoptedResumeInit)) +
to_8_bytes(cs->flag_count * sizeof(MVMRegister));
}
case MVM_CALLSTACK_RECORD_NESTED_RUNLOOP:
return sizeof(MVMCallStackNestedRunloop);
default:
MVM_panic(1, "Unknown callstack record type in record_size");
}
Expand All @@ -136,6 +139,14 @@ void MVM_callstack_init(MVMThreadContext *tc) {
sizeof(MVMCallStackStart));
}

/* Allocates a bytecode frame record on the callstack. */
MVMCallStackRecord * MVM_callstack_allocate_nested_runloop(MVMThreadContext *tc) {
tc->stack_top = allocate_record(tc, MVM_CALLSTACK_RECORD_NESTED_RUNLOOP,
sizeof(MVMCallStackNestedRunloop));
((MVMCallStackNestedRunloop*)tc->stack_top)->cur_frame = tc->cur_frame;
return tc->stack_top;
}

/* Allocates a bytecode frame record on the callstack. */
MVMCallStackFrame * MVM_callstack_allocate_frame(MVMThreadContext *tc) {
tc->stack_top = allocate_record(tc, MVM_CALLSTACK_RECORD_FRAME,
Expand Down Expand Up @@ -559,6 +570,9 @@ MVMFrame * MVM_callstack_unwind_frame(MVMThreadContext *tc, MVMuint8 exceptional
}
break;
}
case MVM_CALLSTACK_RECORD_NESTED_RUNLOOP: {
return ((MVMCallStackNestedRunloop*)tc->stack_top)->cur_frame;
}
default:
MVM_panic(1, "Unknown call stack record type in unwind");
}
Expand Down Expand Up @@ -719,6 +733,11 @@ static void mark(MVMThreadContext *tc, MVMCallStackRecord *from_record, MVMGCWor
}
break;
}
case MVM_CALLSTACK_RECORD_NESTED_RUNLOOP:
add_collectable(tc, worklist, snapshot,
((MVMCallStackNestedRunloop *)record)->cur_frame,
"Callstack reference to frame starting a nested runloop");
break;
default:
MVM_panic(1, "Unknown call stack record type in GC marking");
}
Expand Down
10 changes: 10 additions & 0 deletions src/core/callstack.h
Expand Up @@ -322,8 +322,18 @@ struct MVMCallStackDeoptedResumeInit {
MVMRegister *args;
};

#define MVM_CALLSTACK_RECORD_NESTED_RUNLOOP 14
struct MVMCallStackNestedRunloop {
/* Commonalities of all records. */
MVMCallStackRecord common;

/* The tag itself. */
MVMFrame *cur_frame;
};

/* Functions for working with the call stack. */
void MVM_callstack_init(MVMThreadContext *tc);
MVMCallStackRecord * MVM_callstack_allocate_nested_runloop(MVMThreadContext *tc);
MVMCallStackFrame * MVM_callstack_allocate_frame(MVMThreadContext *tc);
MVMCallStackHeapFrame * MVM_callstack_allocate_heap_frame(MVMThreadContext *tc);
MVMCallStackDispatchRecord * MVM_callstack_allocate_dispatch_record(MVMThreadContext *tc);
Expand Down
42 changes: 27 additions & 15 deletions src/core/interp.c
Expand Up @@ -6701,32 +6701,44 @@ void MVM_interp_run(MVMThreadContext *tc, void (*initial_invoke)(MVMThreadContex
}

void MVM_interp_run_nested(MVMThreadContext *tc, void (*initial_invoke)(MVMThreadContext *, void *), void *invoke_data, MVMRegister *res) {
MVMFrame *backup_cur_frame = MVM_frame_force_to_heap(tc, tc->cur_frame);
MVMFrame *backup_thread_entry_frame = tc->thread_entry_frame;
void **backup_jit_return_address = tc->jit_return_address;
tc->jit_return_address = NULL;
MVMRunloopState outer_runloop = {tc->interp_cur_op, tc->interp_bytecode_start, tc->interp_reg_base, tc->interp_cu};
MVMFrame *backup_cur_frame = MVM_frame_force_to_heap(tc, tc->cur_frame);
MVMFrame *backup_thread_entry_frame = tc->thread_entry_frame;
MVMReturnType backup_return_type = tc->cur_frame->return_type;
MVMRegister *backup_return_value = tc->cur_frame->return_value;
void **backup_jit_return_address = tc->jit_return_address;
tc->jit_return_address = NULL;
MVMRunloopState outer_runloop = {
tc->interp_cur_op,
tc->interp_bytecode_start,
tc->interp_reg_base,
tc->interp_cu
};
MVMROOT2(tc, backup_cur_frame, backup_thread_entry_frame, {
MVMuint32 backup_mark = MVM_gc_root_temp_mark(tc);
jmp_buf backup_interp_jump;
memcpy(backup_interp_jump, tc->interp_jump, sizeof(jmp_buf));


tc->cur_frame->return_value = res;
tc->cur_frame->return_type = MVM_RETURN_OBJ;
MVMCallStackRecord *csrecord = MVM_callstack_allocate_nested_runloop(tc);
tc->cur_frame->return_value = res;
tc->cur_frame->return_type = MVM_RETURN_OBJ;
tc->cur_frame->return_address = *tc->interp_cur_op;

tc->nested_interpreter++;
MVM_interp_run(tc, initial_invoke, invoke_data, &outer_runloop);
tc->interp_cur_op = outer_runloop.interp_cur_op;
tc->interp_bytecode_start = outer_runloop.interp_bytecode_start;
tc->interp_reg_base = outer_runloop.interp_reg_base;
tc->interp_cu = outer_runloop.interp_cu;
tc->nested_interpreter--;

tc->cur_frame = backup_cur_frame;
tc->jit_return_address = backup_jit_return_address;
tc->thread_entry_frame = backup_thread_entry_frame;
assert(tc->stack_top == csrecord);
tc->stack_top = tc->stack_top->prev;
tc->interp_cur_op = outer_runloop.interp_cur_op;
tc->interp_bytecode_start = outer_runloop.interp_bytecode_start;
tc->interp_reg_base = outer_runloop.interp_reg_base;
tc->interp_cu = outer_runloop.interp_cu;

tc->cur_frame = backup_cur_frame;
tc->cur_frame->return_type = backup_return_type;
tc->cur_frame->return_value = backup_return_value;
tc->jit_return_address = backup_jit_return_address;
tc->thread_entry_frame = backup_thread_entry_frame;

memcpy(tc->interp_jump, backup_interp_jump, sizeof(jmp_buf));
MVM_gc_root_temp_mark_reset(tc, backup_mark);
Expand Down

0 comments on commit d7379be

Please sign in to comment.