[wasm] Generate correct locations for error messages

The current logic in Isolate::GetLocationFromStackTrace just ignores
wasm frames, making the computed location point to the first javascript
frame, like this:

test.js:17: RuntimeError: divide by zero
module.exports.main();
               ^
RuntimeError: divide by zero
    at main (<WASM>[1]+5)
    at test.js:17:16

This CL not only fixes the location to point to the top-most wasm
frame, but also exposes to the embedder that the script of that location
is a wasm script, allowing for custom printing of wasm locations.
The Shell::ReportException method now checks for this flag, and prints
wasm locations like this:

<WASM>[0]+5: RuntimeError: divide by zero
RuntimeError: divide by zero
    at main (<WASM>[0]+5)
    at test/message/wasm-trap.js:15:16

R=titzer@chromium.org, yangguo@chromium.org
BUG=chromium:613110

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

include/v8.h

@@ -962,19 +962,20 @@ class V8_EXPORT Data {
 class ScriptOriginOptions {
  public:
   V8_INLINE ScriptOriginOptions(bool is_shared_cross_origin = false,
-                                bool is_opaque = false)
+                                bool is_opaque = false, bool is_wasm = false)
       : flags_((is_shared_cross_origin ? kIsSharedCrossOrigin : 0) |
-               (is_opaque ? kIsOpaque : 0)) {}
+               (is_wasm ? kIsWasm : 0) | (is_opaque ? kIsOpaque : 0)) {}
   V8_INLINE ScriptOriginOptions(int flags)
-      : flags_(flags & (kIsSharedCrossOrigin | kIsOpaque)) {}
+      : flags_(flags & (kIsSharedCrossOrigin | kIsOpaque | kIsWasm)) {}
   bool IsSharedCrossOrigin() const {
     return (flags_ & kIsSharedCrossOrigin) != 0;
   }
   bool IsOpaque() const { return (flags_ & kIsOpaque) != 0; }
+  bool IsWasm() const { return (flags_ & kIsWasm) != 0; }
   int Flags() const { return flags_; }
 
  private:
-  enum { kIsSharedCrossOrigin = 1, kIsOpaque = 1 << 1 };
+  enum { kIsSharedCrossOrigin = 1, kIsOpaque = 1 << 1, kIsWasm = 1 << 2 };
   const int flags_;
 };
 
@@ -990,7 +991,8 @@ class ScriptOrigin {
       Local<Boolean> resource_is_shared_cross_origin = Local<Boolean>(),
       Local<Integer> script_id = Local<Integer>(),
       Local<Value> source_map_url = Local<Value>(),
-      Local<Boolean> resource_is_opaque = Local<Boolean>());
+      Local<Boolean> resource_is_opaque = Local<Boolean>(),
+      Local<Boolean> is_wasm = Local<Boolean>());
 
   V8_INLINE Local<Value> ResourceName() const;
   V8_INLINE Local<Integer> ResourceLineOffset() const;
@@ -8839,13 +8841,15 @@ ScriptOrigin::ScriptOrigin(Local<Value> resource_name,
                            Local<Boolean> resource_is_shared_cross_origin,
                            Local<Integer> script_id,
                            Local<Value> source_map_url,
-                           Local<Boolean> resource_is_opaque)
+                           Local<Boolean> resource_is_opaque,
+                           Local<Boolean> is_wasm)
     : resource_name_(resource_name),
       resource_line_offset_(resource_line_offset),
       resource_column_offset_(resource_column_offset),
       options_(!resource_is_shared_cross_origin.IsEmpty() &&
                    resource_is_shared_cross_origin->IsTrue(),
-               !resource_is_opaque.IsEmpty() && resource_is_opaque->IsTrue()),
+               !resource_is_opaque.IsEmpty() && resource_is_opaque->IsTrue(),
+               !is_wasm.IsEmpty() && is_wasm->IsTrue()),
       script_id_(script_id),
       source_map_url_(source_map_url) {}
 

src/api.cc

@@ -264,7 +264,8 @@ static ScriptOrigin GetScriptOriginForScript(i::Isolate* isolate,
       v8::Boolean::New(v8_isolate, options.IsSharedCrossOrigin()),
       v8::Integer::New(v8_isolate, script->id()),
       Utils::ToLocal(source_map_url),
-      v8::Boolean::New(v8_isolate, options.IsOpaque()));
+      v8::Boolean::New(v8_isolate, options.IsOpaque()),
+      v8::Boolean::New(v8_isolate, script->type() == i::Script::TYPE_WASM));
   return origin;
 }
 

src/d8.cc

@@ -422,12 +422,6 @@ bool CounterMap::Match(void* key1, void* key2) {
 }
 
 
-// Converts a V8 value to a C string.
-const char* Shell::ToCString(const v8::String::Utf8Value& value) {
-  return *value ? *value : "<string conversion failed>";
-}
-
-
 ScriptCompiler::CachedData* CompileForCachedData(
     Local<String> source, Local<Value> name,
     ScriptCompiler::CompileOptions compile_options) {
@@ -1256,12 +1250,17 @@ void Shell::Version(const v8::FunctionCallbackInfo<v8::Value>& args) {
 
 void Shell::ReportException(Isolate* isolate, v8::TryCatch* try_catch) {
   HandleScope handle_scope(isolate);
-  Local<Context> context;
-  bool enter_context = !isolate->InContext();
+  Local<Context> context = isolate->GetCurrentContext();
+  bool enter_context = context.IsEmpty();
   if (enter_context) {
     context = Local<Context>::New(isolate, evaluation_context_);
     context->Enter();
   }
+  // Converts a V8 value to a C string.
+  auto ToCString = [](const v8::String::Utf8Value& value) {
+    return *value ? *value : "<string conversion failed>";
+  };
+
   v8::String::Utf8Value exception(try_catch->Exception());
   const char* exception_string = ToCString(exception);
   Local<Message> message = try_catch->Message();
@@ -1269,40 +1268,40 @@ void Shell::ReportException(Isolate* isolate, v8::TryCatch* try_catch) {
     // V8 didn't provide any extra information about this error; just
     // print the exception.
     printf("%s\n", exception_string);
+  } else if (message->GetScriptOrigin().Options().IsWasm()) {
+    // Print <WASM>[(function index)]((function name))+(offset): (message).
+    int function_index = message->GetLineNumber(context).FromJust() - 1;
+    int offset = message->GetStartColumn(context).FromJust();
+    printf("<WASM>[%d]+%d: %s\n", function_index, offset, exception_string);
   } else {
     // Print (filename):(line number): (message).
     v8::String::Utf8Value filename(message->GetScriptOrigin().ResourceName());
     const char* filename_string = ToCString(filename);
-    Maybe<int> maybeline = message->GetLineNumber(isolate->GetCurrentContext());
-    int linenum = maybeline.IsJust() ? maybeline.FromJust() : -1;
+    int linenum = message->GetLineNumber(context).FromMaybe(-1);
     printf("%s:%i: %s\n", filename_string, linenum, exception_string);
     Local<String> sourceline;
-    if (message->GetSourceLine(isolate->GetCurrentContext())
-            .ToLocal(&sourceline)) {
+    if (message->GetSourceLine(context).ToLocal(&sourceline)) {
       // Print line of source code.
       v8::String::Utf8Value sourcelinevalue(sourceline);
       const char* sourceline_string = ToCString(sourcelinevalue);
       printf("%s\n", sourceline_string);
       // Print wavy underline (GetUnderline is deprecated).
-      int start =
-          message->GetStartColumn(isolate->GetCurrentContext()).FromJust();
+      int start = message->GetStartColumn(context).FromJust();
       for (int i = 0; i < start; i++) {
         printf(" ");
       }
-      int end = message->GetEndColumn(isolate->GetCurrentContext()).FromJust();
+      int end = message->GetEndColumn(context).FromJust();
       for (int i = start; i < end; i++) {
         printf("^");
       }
       printf("\n");
     }
-    Local<Value> stack_trace_string;
-    if (try_catch->StackTrace(isolate->GetCurrentContext())
-            .ToLocal(&stack_trace_string) &&
-        stack_trace_string->IsString()) {
-      v8::String::Utf8Value stack_trace(
-          Local<String>::Cast(stack_trace_string));
-      printf("%s\n", ToCString(stack_trace));
-    }
+  }
+  Local<Value> stack_trace_string;
+  if (try_catch->StackTrace(context).ToLocal(&stack_trace_string) &&
+      stack_trace_string->IsString()) {
+    v8::String::Utf8Value stack_trace(Local<String>::Cast(stack_trace_string));
+    printf("%s\n", ToCString(stack_trace));
   }
   printf("\n");
   if (enter_context) context->Exit();

src/d8.h

@@ -324,7 +324,6 @@ class Shell : public i::AllStatic {
                             Local<Value> name, bool print_result,
                             bool report_exceptions);
   static bool ExecuteModule(Isolate* isolate, const char* file_name);
-  static const char* ToCString(const v8::String::Utf8Value& value);
   static void ReportException(Isolate* isolate, TryCatch* try_catch);
   static Local<String> ReadFile(Isolate* isolate, const char* name);
   static Local<Context> CreateEvaluationContext(Isolate* isolate);

src/isolate.cc

@@ -1551,25 +1551,31 @@ bool Isolate::ComputeLocationFromStackTrace(MessageLocation* target,
 
   const int frame_count = elements->FrameCount();
   for (int i = 0; i < frame_count; i++) {
-    if (elements->IsWasmFrame(i)) {
-      // TODO(clemensh): Handle wasm frames if they ever need handling here.
-      continue;
-    }
-
-    if (elements->IsAsmJsWasmFrame(i)) {
+    if (elements->IsWasmFrame(i) || elements->IsAsmJsWasmFrame(i)) {
       Handle<WasmCompiledModule> compiled_module(
           WasmInstanceObject::cast(elements->WasmInstance(i))
               ->get_compiled_module());
       int func_index = elements->WasmFunctionIndex(i)->value();
       int code_offset = elements->Offset(i)->value();
-      int byte_pos = elements->Code(i)->SourcePosition(code_offset);
-      bool at_to_number_conversion =
-          elements->Flags(i)->value() & FrameArray::kAsmJsAtNumberConversion;
-      int source_pos = WasmCompiledModule::GetAsmJsSourcePosition(
-          compiled_module, func_index, byte_pos, at_to_number_conversion);
+      // TODO(wasm): Clean this up (bug 5007).
+      int pos = code_offset < 0
+                    ? (-1 - code_offset)
+                    : elements->Code(i)->SourcePosition(code_offset);
+      if (elements->IsAsmJsWasmFrame(i)) {
+        // For asm.js frames, make an additional translation step to get the
+        // asm.js source position.
+        bool at_to_number_conversion =
+            elements->Flags(i)->value() & FrameArray::kAsmJsAtNumberConversion;
+        pos = WasmCompiledModule::GetAsmJsSourcePosition(
+            compiled_module, func_index, pos, at_to_number_conversion);
+      } else {
+        // For pure wasm, make the function-local position module-relative by
+        // adding the function offset.
+        pos += compiled_module->GetFunctionOffset(func_index);
+      }
       Handle<Script> script = compiled_module->script();
 
-      *target = MessageLocation(script, source_pos, source_pos + 1);
+      *target = MessageLocation(script, pos, pos + 1);
       return true;
     }
 

src/messages.cc

@@ -673,6 +673,7 @@ MaybeHandle<String> WasmStackFrame::ToString() {
 }
 
 int WasmStackFrame::GetPosition() const {
+  // TODO(wasm): Clean this up (bug 5007).
   return (offset_ < 0) ? (-1 - offset_) : code_->SourcePosition(offset_);
 }
 

test/message/wasm-trap.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");
+
+var builder = new WasmModuleBuilder();
+builder.addFunction('main', kSig_i_v)
+    .addBody([kExprI32Const, 2, kExprI32Const, 0, kExprI32DivU])
+    .exportFunc();
+var module = builder.instantiate();
+module.exports.main();

test/message/wasm-trap.out

+<WASM>[0]+5: RuntimeError: divide by zero
+RuntimeError: divide by zero
+    at main (<WASM>[0]+5)
+    at *%(basename)s:15:16
+