[wasm] Patch trapping position into stack trace

And add more tests for traps at different locations.

R=titzer@chromium.org, yangguo@chromium.org

Review-Url: https://codereview.chromium.org/1924253002
Cr-Commit-Position: refs/heads/master@{#36202}

src/js/messages.js

@@ -826,7 +826,10 @@ function GetStackFrames(raw_stack) {
     var fun = raw_stack[i + 1];
     var code = raw_stack[i + 2];
     var pc = raw_stack[i + 3];
-    var pos = %FunctionGetPositionForOffset(code, pc);
+    // For traps in wasm, the bytecode offset is passed as (-1 - offset).
+    // Otherwise, lookup the position from the pc.
+    var pos = IS_NUMBER(fun) && pc < 0 ? (-1 - pc) :
+      %FunctionGetPositionForOffset(code, pc);
     sloppy_frames--;
     frames.push(new CallSite(recv, fun, pos, (sloppy_frames < 0)));
   }

src/runtime/runtime-internal.cc

@@ -101,10 +101,53 @@ RUNTIME_FUNCTION(Runtime_ThrowWasmError) {
   DCHECK_EQ(2, args.length());
   CONVERT_SMI_ARG_CHECKED(message_id, 0);
   CONVERT_SMI_ARG_CHECKED(byte_offset, 1);
-  USE(byte_offset);  // TODO(clemensh): patch the stack trace with this offset
-  Handle<Object> error = isolate->factory()->NewError(
+  Handle<Object> error_obj = isolate->factory()->NewError(
       static_cast<MessageTemplate::Template>(message_id));
-  return isolate->Throw(*error);
+
+  // 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.
+  // We hence pass the byte offset explicitely, and patch it into the top-most
+  // frame (a wasm frame) on the collected stack trace.
+  // TODO(wasm): This implementation is temporary, see bug #5007:
+  // https://bugs.chromium.org/p/v8/issues/detail?id=5007
+  Handle<JSObject> error = Handle<JSObject>::cast(error_obj);
+  Handle<Object> stack_trace_obj = JSReceiver::GetDataProperty(
+      error, isolate->factory()->stack_trace_symbol());
+  // Patch the stack trace (array of <receiver, function, code, position>).
+  if (stack_trace_obj->IsJSArray()) {
+    Handle<FixedArray> stack_elements(
+        FixedArray::cast(JSArray::cast(*stack_trace_obj)->elements()));
+    DCHECK_EQ(1, stack_elements->length() % 4);
+    DCHECK(Code::cast(stack_elements->get(3))->kind() == Code::WASM_FUNCTION);
+    DCHECK(stack_elements->get(4)->IsSmi() &&
+           Smi::cast(stack_elements->get(4))->value() >= 0);
+    stack_elements->set(4, Smi::FromInt(-1 - byte_offset));
+  }
+  Handle<Object> detailed_stack_trace_obj = JSReceiver::GetDataProperty(
+      error, isolate->factory()->detailed_stack_trace_symbol());
+  // Patch the detailed stack trace (array of JSObjects with various
+  // properties).
+  if (detailed_stack_trace_obj->IsJSArray()) {
+    Handle<FixedArray> stack_elements(
+        FixedArray::cast(JSArray::cast(*detailed_stack_trace_obj)->elements()));
+    DCHECK_GE(stack_elements->length(), 1);
+    Handle<JSObject> top_frame(JSObject::cast(stack_elements->get(0)));
+    Handle<String> wasm_offset_key =
+        isolate->factory()->InternalizeOneByteString(
+            STATIC_CHAR_VECTOR("column"));
+    LookupIterator it(top_frame, wasm_offset_key, top_frame,
+                      LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
+    if (it.IsFound()) {
+      DCHECK(JSReceiver::GetDataProperty(&it)->IsSmi());
+      Maybe<bool> data_set = JSReceiver::SetDataProperty(
+          &it, handle(Smi::FromInt(byte_offset), isolate));
+      DCHECK(data_set.IsJust() && data_set.FromJust() == true);
+      USE(data_set);
+    }
+  }
+
+  return isolate->Throw(*error_obj);
 }
 
 RUNTIME_FUNCTION(Runtime_UnwindAndFindExceptionHandler) {

test/cctest/cctest.gyp

@@ -193,6 +193,7 @@
         'wasm/test-signatures.h',
         'wasm/test-wasm-function-name-table.cc',
         'wasm/test-wasm-stack.cc',
+        'wasm/test-wasm-trap-position.cc',
         'wasm/wasm-run-utils.h',
       ],
       'conditions': [

test/cctest/wasm/test-wasm-stack.cc

@@ -157,8 +157,7 @@ TEST(CollectDetailedWasmStack_WasmError) {
 
   // Line number is 1-based, with 0 == kNoLineNumberInfo.
   ExceptionInfo expected_exceptions[] = {
-      // TODO(clemens): position should be 1
-      {"<WASM>", static_cast<int>(wasm_index), -1},   // -
+      {"<WASM>", static_cast<int>(wasm_index), 1},    // -
       {"<WASM>", static_cast<int>(wasm_index_2), 1},  // -
       {"callFn", 1, 24}                               //-
   };

test/cctest/wasm/test-wasm-trap-position.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/wasm/wasm-macro-gen.h"
+
+#include "test/cctest/cctest.h"
+#include "test/cctest/compiler/value-helper.h"
+#include "test/cctest/wasm/test-signatures.h"
+#include "test/cctest/wasm/wasm-run-utils.h"
+
+using namespace v8::base;
+using namespace v8::internal;
+using namespace v8::internal::compiler;
+using namespace v8::internal::wasm;
+
+using v8::Local;
+using v8::Utils;
+
+namespace {
+
+#define CHECK_CSTREQ(exp, found)                                           \
+  do {                                                                     \
+    const char* exp_ = (exp);                                              \
+    const char* found_ = (found);                                          \
+    DCHECK_NOT_NULL(exp);                                                  \
+    if (V8_UNLIKELY(found_ == nullptr || strcmp(exp_, found_) != 0)) {     \
+      V8_Fatal(__FILE__, __LINE__,                                         \
+               "Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, \
+               exp_, found_ ? found_ : "<null>");                          \
+    }                                                                      \
+  } while (0)
+
+struct ExceptionInfo {
+  const char* func_name;
+  int line_nr;
+  int column;
+};
+
+template <int N>
+void CheckExceptionInfos(Isolate* isolate, Handle<Object> exc,
+                         const ExceptionInfo (&excInfos)[N]) {
+  // Check that it's indeed an Error object.
+  CHECK(Object::IsErrorObject(isolate, exc));
+
+  // Extract stack frame from the exception.
+  Local<v8::Value> localExc = Utils::ToLocal(exc);
+  v8::Local<v8::StackTrace> stack = v8::Exception::GetStackTrace(localExc);
+  CHECK(!stack.IsEmpty());
+  CHECK_EQ(N, stack->GetFrameCount());
+
+  for (int frameNr = 0; frameNr < N; ++frameNr) {
+    v8::Local<v8::StackFrame> frame = stack->GetFrame(frameNr);
+    v8::String::Utf8Value funName(frame->GetFunctionName());
+    CHECK_CSTREQ(excInfos[frameNr].func_name, *funName);
+    CHECK_EQ(excInfos[frameNr].line_nr, frame->GetLineNumber());
+    CHECK_EQ(excInfos[frameNr].column, frame->GetColumn());
+  }
+}
+
+}  // namespace
+
+// Trigger a trap for executing unreachable.
+TEST(Unreachable) {
+  TestSignatures sigs;
+  TestingModule module;
+
+  WasmFunctionCompiler comp1(sigs.v_v(), &module,
+                             ArrayVector("exec_unreachable"));
+  // Set the execution context, such that a runtime error can be thrown.
+  comp1.SetModuleContext();
+  BUILD(comp1, WASM_UNREACHABLE);
+  uint32_t wasm_index = comp1.CompileAndAdd();
+
+  Handle<JSFunction> js_wasm_wrapper = module.WrapCode(wasm_index);
+
+  Handle<JSFunction> js_trampoline = Handle<JSFunction>::cast(
+      v8::Utils::OpenHandle(*v8::Local<v8::Function>::Cast(
+          CompileRun("(function callFn(fn) { fn(); })"))));
+
+  Isolate* isolate = js_wasm_wrapper->GetIsolate();
+  isolate->SetCaptureStackTraceForUncaughtExceptions(true, 10,
+                                                     v8::StackTrace::kOverview);
+  Handle<Object> global(isolate->context()->global_object(), isolate);
+  MaybeHandle<Object> maybe_exc;
+  Handle<Object> args[] = {js_wasm_wrapper};
+  MaybeHandle<Object> returnObjMaybe =
+      Execution::TryCall(isolate, js_trampoline, global, 1, args, &maybe_exc);
+  CHECK(returnObjMaybe.is_null());
+
+  ExceptionInfo expected_exceptions[] = {
+      {"<WASM>", static_cast<int>(wasm_index), 1},  // --
+      {"callFn", 1, 24}                             // --
+  };
+  CheckExceptionInfos(isolate, maybe_exc.ToHandleChecked(),
+                      expected_exceptions);
+}
+
+// Trigger a trap for loading from out-of-bounds.
+TEST(IllegalLoad) {
+  TestSignatures sigs;
+  TestingModule module;
+
+  WasmFunctionCompiler comp1(sigs.v_v(), &module, ArrayVector("mem_oob"));
+  // Set the execution context, such that a runtime error can be thrown.
+  comp1.SetModuleContext();
+  BUILD(comp1, WASM_IF(WASM_ONE,
+                       WASM_LOAD_MEM(MachineType::Int32(), WASM_I32V_1(-3))));
+  uint32_t wasm_index = comp1.CompileAndAdd();
+
+  WasmFunctionCompiler comp2(sigs.v_v(), &module, ArrayVector("call_mem_oob"));
+  // Insert a NOP such that the position of the call is not one.
+  BUILD(comp2, WASM_NOP, WASM_CALL_FUNCTION0(wasm_index));
+  uint32_t wasm_index_2 = comp2.CompileAndAdd();
+
+  Handle<JSFunction> js_wasm_wrapper = module.WrapCode(wasm_index_2);
+
+  Handle<JSFunction> js_trampoline = Handle<JSFunction>::cast(
+      v8::Utils::OpenHandle(*v8::Local<v8::Function>::Cast(
+          CompileRun("(function callFn(fn) { fn(); })"))));
+
+  Isolate* isolate = js_wasm_wrapper->GetIsolate();
+  isolate->SetCaptureStackTraceForUncaughtExceptions(true, 10,
+                                                     v8::StackTrace::kOverview);
+  Handle<Object> global(isolate->context()->global_object(), isolate);
+  MaybeHandle<Object> maybe_exc;
+  Handle<Object> args[] = {js_wasm_wrapper};
+  MaybeHandle<Object> returnObjMaybe =
+      Execution::TryCall(isolate, js_trampoline, global, 1, args, &maybe_exc);
+  CHECK(returnObjMaybe.is_null());
+
+  // Line number is 1-based, with 0 == kNoLineNumberInfo.
+  ExceptionInfo expected_exceptions[] = {
+      {"<WASM>", static_cast<int>(wasm_index), 6},    // --
+      {"<WASM>", static_cast<int>(wasm_index_2), 2},  // --
+      {"callFn", 1, 24}                               // --
+  };
+  CheckExceptionInfos(isolate, maybe_exc.ToHandleChecked(),
+                      expected_exceptions);
+}

test/mjsunit/wasm/stack.js

@@ -106,10 +106,9 @@ Error.prepareStackTrace = function(error, frames) {
     assertContains("unreachable", e.message);
     verifyStack(e.stack, [
         // isWasm               function   line  pos        file
-        // TODO(clemensh): pos should be 1
-        [    true,    "exec_unreachable",    1,   -1,       null],
+        [    true,    "exec_unreachable",    1,    1,       null],
         [   false, "testWasmUnreachable",  103,    0, "stack.js"],
-        [   false,                  null,  115,    0, "stack.js"]
+        [   false,                  null,  114,    0, "stack.js"]
     ]);
   }
 })();
@@ -122,11 +121,10 @@ Error.prepareStackTrace = function(error, frames) {
     assertContains("out of bounds", e.message);
     verifyStack(e.stack, [
         // isWasm                  function   line  pos        file
-        // TODO(clemensh): pos should be 3
-        [    true,                      "?",     2,  -1,       null],
+        [    true,                      "?",     2,   3,       null],
         [    true, "call_mem_out_of_bounds",     3,   1,       null],
-        [   false, "testWasmMemOutOfBounds",   119,   0, "stack.js"],
-        [   false,                     null,   132,   0, "stack.js"]
+        [   false, "testWasmMemOutOfBounds",   118,   0, "stack.js"],
+        [   false,                     null,   130,   0, "stack.js"]
     ]);
   }
 })();

test/mjsunit/wasm/trap-location.js

+// 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.
+
+// Flags: --expose-wasm
+
+load("test/mjsunit/wasm/wasm-constants.js");
+load("test/mjsunit/wasm/wasm-module-builder.js");
+
+// Collect the Callsite objects instead of just a string:
+Error.prepareStackTrace = function(error, frames) {
+  return frames;
+};
+
+var builder = new WasmModuleBuilder();
+
+var sig_index = builder.addSignature(kSig_i_v)
+
+// Build a function to resemble this code:
+//   if (idx < 2) {
+//     return load(-2 / idx);
+//   } else if (idx == 2) {
+//     unreachable;
+//   } else {
+//     return call_indirect(idx);
+//   }
+// There are four different traps which are triggered by different input values:
+// (0) division by zero; (1) mem oob; (2) unreachable; (3) invalid call target
+// Each of them also has a different location where it traps.
+builder.addFunction("main", kSig_i_i)
+  .addBody([
+      // offset 1
+      kExprBlock,
+            kExprGetLocal, 0,
+            kExprI32Const, 2,
+          kExprI32LtU,
+        kExprIf,
+        // offset 8
+              kExprI32Const, 0x7e /* -2 */,
+              kExprGetLocal, 0,
+            kExprI32DivU,
+          // offset 13
+          kExprI32LoadMem, 0, 0,
+          kExprBr, 1, 1,
+        kExprEnd,
+        // offset 20
+            kExprGetLocal, 0,
+            kExprI32Const, 2,
+          kExprI32Eq,
+        kExprIf,
+          kExprUnreachable,
+        kExprEnd,
+        // offset 28
+          kExprGetLocal, 0,
+        kExprCallIndirect, kArity0, sig_index,
+      kExprEnd,
+  ])
+  .exportAs("main");
+
+var module = builder.instantiate();
+
+function testWasmTrap(value, reason, position) {
+  try {
+    module.exports.main(value);
+    fail("expected wasm exception");
+  } catch (e) {
+    assertEquals(kTrapMsgs[reason], e.message, "trap reason");
+    assertEquals(3, e.stack.length, "number of frames");
+    assertEquals(0, e.stack[0].getLineNumber(), "wasmFunctionIndex");
+    assertEquals(position, e.stack[0].getPosition(), "position");
+  }
+}
+
+// The actual tests:
+testWasmTrap(0, kTrapDivByZero,      12);
+testWasmTrap(1, kTrapMemOutOfBounds, 13);
+testWasmTrap(2, kTrapUnreachable,    26);
+testWasmTrap(3, kTrapFuncInvalid,    30);

test/mjsunit/wasm/wasm-constants.js

@@ -99,6 +99,7 @@ var kSig_d_dd = [2, kAstF64, kAstF64, 1, kAstF64];
 var kSig_l_ll = [2, kAstI64, kAstI64, 1, kAstI64];
 var kSig_i_dd = [2, kAstF64, kAstF64, 1, kAstI32];
 var kSig_v_v = [0, 0];
+var kSig_i_v = [0, 1, kAstI32];
 
 function makeSig_v_xx(x) {
   return [2, x, x, 0];