[wasm] Acquire the context for TrapIf from a wasm frame

src/runtime/runtime-wasm.cc

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/runtime/runtime-utils.h"
 
 #include "src/arguments.h"
 #include "src/assembler.h"
 #include "src/compiler/wasm-compiler.h"
 #include "src/conversions.h"
@@ skipping 34 matching lines @@
 
 RUNTIME_FUNCTION(Runtime_WasmGrowMemory) {
   HandleScope scope(isolate);
   DCHECK_EQ(1, args.length());
   CONVERT_UINT32_ARG_CHECKED(delta_pages, 0);
   Handle<WasmInstanceObject> instance = GetWasmInstanceOnStackTop(isolate);
   return *isolate->factory()->NewNumberFromInt(
       wasm::GrowMemory(isolate, instance, delta_pages));
 }
 
 Object* ThrowRuntimeError(Isolate* isolate, int message_id, int byte_offset,

[Michael Starzinger, 2017/02/01 11:36:52]

We could add a DCHECK_NULL(isolate->context()) here to ensure callers adhere to
the convention. We do something similar in Runtime_NotifyDeoptimized already,
where we also call {Isolate::set_context} in a similar fashion.

[ahaas, 2017/02/01 11:48:51]

Done.

                           bool patch_source_position) {
   HandleScope scope(isolate);
+  StackFrameIterator it(isolate);
+  it.Advance();
+  DCHECK(!it.done());
+  DCHECK(it.frame()->is_wasm());

[Clemens Hammacher, 2017/01/31 19:20:33]

If this is just DCHECKs, we can actually skip them, because
StackFrameIterator::frame already DCHECKs !done(), and WasmCompiledFrame::cast
checks the frame type.

[ahaas, 2017/02/01 11:48:51]

I removed the DCHECK(!it.done()); and added
CHECK(it.frame()->is_wasm_compiled());

+  isolate->set_context(*WasmCompiledFrame::cast(it.frame())

[titzer, 2017/01/31 19:16:08]

We should be careful here. I assume that Throw() below will reset the context,
but I'm not entirely sure.

[Clemens Hammacher, 2017/01/31 19:20:33]

I am still not sure if it's really needed, but to be on the save side, we should
use a SaveContext here.

[Michael Starzinger, 2017/02/01 11:36:52]

AFAICT the {Throw} method itself should not change the current context. What
will change the current context though is the CEntryStub during the stack
unwinding after this function returns. This unwinding itself should not inspect
the current context. So at first glance this looks safe to me.

+                            ->wasm_instance()
+                            ->compiled_module()
+                            ->native_context());
   Handle<Object> error_obj = isolate->factory()->NewWasmRuntimeError(
       static_cast<MessageTemplate::Template>(message_id));
 
   if (!patch_source_position) {
     return isolate->Throw(*error_obj);
   }
 
   // For wasm traps, the byte offset (a.k.a source position) can not be
   // determined from relocation info, since the explicit checks for traps
   // converge in one singe block which calls this runtime function.
@@ skipping 104 matching lines @@
   // Set the current isolate's context, saving the previous one.
   SaveContext save(isolate);
   isolate->set_context(*instance->compiled_module()->native_context());
 
   instance->debug_info()->RunInterpreter(func_index, arg_buffer);
   return isolate->heap()->undefined_value();
 }
 
 }  // namespace internal
 }  // namespace v8