Skip to content

Commit

Permalink
A bit more CallFrame cleanup.
Browse files Browse the repository at this point in the history
We don't have a constructor for CallFrame so it's not always clear which
fields are initialized. In this case, VM::call_frame_ is NULL when the VM
instance is created, so calling VM::push_call_frame sets previous to NULL when
pushing the new frame. This is fine, but brittle.

Ultimately, CallFrame needs to be split into separate frame types for managed
code, FFI, JIT, and native methods (ie C-API), and constructors should be
defined for every type.
  • Loading branch information
brixen committed Apr 1, 2016
1 parent f05b27d commit 46933dd
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 26 deletions.
1 change: 1 addition & 0 deletions machine/builtin/block_environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,7 @@ namespace rubinius {

call_frame->constant_scope_ = invocation.constant_scope;

call_frame->previous = NULL;
call_frame->arguments = &args;
call_frame->dispatch_data = env;
call_frame->compiled_code = env->compiled_code();
Expand Down
1 change: 1 addition & 0 deletions machine/builtin/native_method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ namespace rubinius {
NativeMethodFrame nmf(env, env->current_native_frame(), nm);
CallFrame* previous_frame = 0;
CallFrame* call_frame = ALLOCA_CALL_FRAME(0);
call_frame->previous = NULL;
call_frame->constant_scope_ = 0;
call_frame->dispatch_data = (void*)&nmf;
call_frame->compiled_code = 0;
Expand Down
41 changes: 15 additions & 26 deletions machine/capi/thread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -249,23 +249,21 @@ extern "C" {
thread->locals_remove(state, state->symbol("argument"));

NativeMethodFrame nmf(env, 0, nm);
CallFrame* previous_frame = 0;
CallFrame cf;
cf.constant_scope_ = 0;
cf.dispatch_data = (void*)&nmf;
cf.compiled_code = 0;
cf.flags = CallFrame::cNativeMethod;
cf.optional_jit_data = 0;
cf.top_scope_ = 0;
cf.scope = 0;
cf.arguments = 0;

CallFrame* saved_frame = env->current_call_frame();
env->set_current_call_frame(&cf);
CallFrame call_frame;
call_frame.previous = NULL;
call_frame.constant_scope_ = 0;
call_frame.dispatch_data = (void*)&nmf;
call_frame.compiled_code = 0;
call_frame.flags = CallFrame::cNativeMethod;
call_frame.optional_jit_data = 0;
call_frame.top_scope_ = 0;
call_frame.scope = 0;
call_frame.arguments = 0;

env->set_current_call_frame(&call_frame);
env->set_current_native_frame(&nmf);

// Register the CallFrame, because we might GC below this.
state->vm()->push_call_frame(&cf, previous_frame);
state->vm()->set_call_frame(&call_frame);

nmf.setup(
env->get_handle(thread),
Expand Down Expand Up @@ -293,14 +291,10 @@ extern "C" {

LEAVE_CAPI(state);

state->vm()->pop_call_frame(previous_frame);

env->set_current_call_frame(saved_frame);
env->set_current_native_frame(nmf.previous());
env->set_current_call_frame(NULL);
env->set_current_native_frame(NULL);
ep.pop(env);

state->vm()->thread.get()->alive(state, cFalse);

return value;
}

Expand All @@ -324,11 +318,6 @@ extern "C" {

VALUE thr_handle = env->get_handle(thr);
thr->fork(state);
// We do a lock and unlock here so we wait until the started
// thread is actually ready. This is to prevent we GC stuff on
// the stack in the C-API caller that might be stuffed in the void* argument
thr->hard_lock(state);
thr->hard_unlock(state);

return thr_handle;
}
Expand Down
2 changes: 2 additions & 0 deletions machine/jit/llvm/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ extern "C" {

call_frame->prepare(mcode->stack_size);

call_frame->previous = NULL;
call_frame->constant_scope_ = code->scope();
call_frame->dispatch_data = NULL;
call_frame->compiled_code = code;
Expand Down Expand Up @@ -166,6 +167,7 @@ extern "C" {

call_frame->constant_scope_ = invocation->constant_scope;

call_frame->previous = NULL;
call_frame->arguments = args;
call_frame->dispatch_data = env;
call_frame->compiled_code = env->compiled_code();
Expand Down
2 changes: 2 additions & 0 deletions machine/machine_code.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,7 @@ namespace rubinius {

call_frame->prepare(mcode->stack_size);

call_frame->previous = NULL;
call_frame->constant_scope_ = code->scope();
call_frame->dispatch_data = NULL;
call_frame->compiled_code = code;
Expand Down Expand Up @@ -831,6 +832,7 @@ namespace rubinius {

Arguments args(state->symbol("__script__"), G(main), cNil, 0, 0);

call_frame->previous = NULL;
call_frame->constant_scope_ = code->scope();
call_frame->dispatch_data = 0;
call_frame->compiled_code = code;
Expand Down
1 change: 1 addition & 0 deletions machine/memory/finalizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ namespace memory {
CallFrame* previous_frame = 0;
CallFrame* call_frame = ALLOCA_CALL_FRAME(0);

call_frame->previous = NULL;
call_frame->constant_scope_ = 0;
call_frame->dispatch_data = (void*)&nmf;
call_frame->compiled_code = 0;
Expand Down

0 comments on commit 46933dd

Please sign in to comment.