From aabbbec67c3c7363ef08c657f2d688ad2e8f7de8 Mon Sep 17 00:00:00 2001 From: bradnelson Date: Mon, 12 Dec 2016 06:47:56 -0800 Subject: [PATCH] [wasm] [asmjs] Route asm.js warnings to the dev console. Generalize Messages to include an error level. Add a parameter to AddMessageHandler to select which error levels to receive, using a mask (default being just errors, i.e. the current behavior). BUG=v8:4203 R=dgozman@chromium.org,machenbach@chromium.org,danno@chromium.org,bmeurer@chromium.org,jochen@chromium.org Review-Url: https://codereview.chromium.org/2526703002 Cr-Commit-Position: refs/heads/master@{#41648} --- include/v8.h | 32 ++++++++- src/api.cc | 14 +++- src/asmjs/asm-js.cc | 5 +- src/asmjs/asm-typer.cc | 54 ++++++-------- src/asmjs/asm-typer.h | 18 ++--- src/d8.cc | 44 +++++++++++- src/factory.cc | 1 + src/messages.cc | 105 ++++++++++++++++------------ src/messages.h | 12 +++- src/objects-inl.h | 2 +- src/objects.h | 6 +- test/cctest/asmjs/test-asm-typer.cc | 9 ++- test/cctest/test-api.cc | 73 +++++++++++++++++++ 13 files changed, 275 insertions(+), 100 deletions(-) diff --git a/include/v8.h b/include/v8.h index e5ff0c0332..86aa8b7d2d 100644 --- a/include/v8.h +++ b/include/v8.h @@ -1477,6 +1477,11 @@ class V8_EXPORT Message { */ int GetEndPosition() const; + /** + * Returns the error level of the message. + */ + int ErrorLevel() const; + /** * Returns the index within the line of the first character where * the error occurred. @@ -6451,6 +6456,16 @@ class V8_EXPORT Isolate { kUseCounterFeatureCount // This enum value must be last. }; + enum MessageErrorLevel { + kMessageLog = (1 << 0), + kMessageDebug = (1 << 1), + kMessageInfo = (1 << 2), + kMessageError = (1 << 3), + kMessageWarning = (1 << 4), + kMessageAll = kMessageLog | kMessageDebug | kMessageInfo | kMessageError | + kMessageWarning, + }; + typedef void (*UseCounterCallback)(Isolate* isolate, UseCounterFeature feature); @@ -7055,7 +7070,7 @@ class V8_EXPORT Isolate { bool IsDead(); /** - * Adds a message listener. + * Adds a message listener (errors only). * * The same message listener can be added more than once and in that * case it will be called more than once for each message. @@ -7066,6 +7081,21 @@ class V8_EXPORT Isolate { bool AddMessageListener(MessageCallback that, Local data = Local()); + /** + * Adds a message listener. + * + * The same message listener can be added more than once and in that + * case it will be called more than once for each message. + * + * If data is specified, it will be passed to the callback when it is called. + * Otherwise, the exception object will be passed to the callback instead. + * + * A listener can listen for particular error levels by providing a mask. + */ + bool AddMessageListenerWithErrorLevel(MessageCallback that, + int message_levels, + Local data = Local()); + /** * Remove all message listeners from the specified callback function. */ diff --git a/src/api.cc b/src/api.cc index 37c289abf4..18de496ecf 100644 --- a/src/api.cc +++ b/src/api.cc @@ -2667,6 +2667,10 @@ int Message::GetEndPosition() const { return self->end_position(); } +int Message::ErrorLevel() const { + auto self = Utils::OpenHandle(this); + return self->error_level(); +} Maybe Message::GetStartColumn(Local context) const { auto self = Utils::OpenHandle(this); @@ -8452,18 +8456,24 @@ bool Isolate::IsDead() { return isolate->IsDead(); } - bool Isolate::AddMessageListener(MessageCallback that, Local data) { + return AddMessageListenerWithErrorLevel(that, kMessageError, data); +} + +bool Isolate::AddMessageListenerWithErrorLevel(MessageCallback that, + int message_levels, + Local data) { i::Isolate* isolate = reinterpret_cast(this); ENTER_V8(isolate); i::HandleScope scope(isolate); i::Handle list = isolate->factory()->message_listeners(); - i::Handle listener = isolate->factory()->NewFixedArray(2); + i::Handle listener = isolate->factory()->NewFixedArray(3); i::Handle foreign = isolate->factory()->NewForeign(FUNCTION_ADDR(that)); listener->set(0, *foreign); listener->set(1, data.IsEmpty() ? isolate->heap()->undefined_value() : *Utils::OpenHandle(*data)); + listener->set(2, i::Smi::FromInt(message_levels)); list = i::TemplateList::Add(isolate, list, listener); isolate->heap()->SetMessageListeners(*list); return true; diff --git a/src/asmjs/asm-js.cc b/src/asmjs/asm-js.cc index 7d00c5fd87..af27beef9a 100644 --- a/src/asmjs/asm-js.cc +++ b/src/asmjs/asm-js.cc @@ -160,8 +160,9 @@ MaybeHandle AsmJs::ConvertAsmToWasm(ParseInfo* info) { auto asm_wasm_result = builder.Run(&foreign_globals); if (!asm_wasm_result.success) { DCHECK(!info->isolate()->has_pending_exception()); - PrintF("Validation of asm.js module failed: %s\n", - builder.typer()->error_message()); + MessageHandler::ReportMessage(info->isolate(), + builder.typer()->message_location(), + builder.typer()->error_message()); return MaybeHandle(); } wasm::ZoneBuffer* module = asm_wasm_result.module_bytes; diff --git a/src/asmjs/asm-typer.cc b/src/asmjs/asm-typer.cc index 6dbfa87267..ddc8898266 100644 --- a/src/asmjs/asm-typer.cc +++ b/src/asmjs/asm-typer.cc @@ -9,6 +9,7 @@ #include #include +#include "include/v8.h" #include "src/v8.h" #include "src/asmjs/asm-types.h" @@ -17,21 +18,25 @@ #include "src/base/bits.h" #include "src/codegen.h" #include "src/globals.h" +#include "src/messages.h" #include "src/utils.h" -#define FAIL_LINE(line, msg) \ - do { \ - base::OS::SNPrintF(error_message_, sizeof(error_message_), \ - "asm: line %d: %s", (line) + 1, msg); \ - return AsmType::None(); \ +#define FAIL_LOCATION(location, msg) \ + do { \ + Handle message(isolate_->factory()->InternalizeOneByteString( \ + STATIC_CHAR_VECTOR(msg))); \ + error_message_ = MessageHandler::MakeMessageObject( \ + isolate_, MessageTemplate::kAsmJsInvalid, (location), message, \ + Handle::null()); \ + error_message_->set_error_level(v8::Isolate::kMessageWarning); \ + message_location_ = *(location); \ + return AsmType::None(); \ } while (false) -#define FAIL(node, msg) \ - do { \ - int line = node->position() == kNoSourcePosition \ - ? -1 \ - : script_->GetLineNumber(node->position()); \ - FAIL_LINE(line, msg); \ +#define FAIL(node, msg) \ + do { \ + MessageLocation location(script_, node->position(), node->position()); \ + FAIL_LOCATION(&location, msg); \ } while (false) #define RECURSE(call) \ @@ -164,8 +169,8 @@ AsmTyper::VariableInfo* AsmTyper::VariableInfo::Clone(Zone* zone) const { return new_var_info; } -void AsmTyper::VariableInfo::SetFirstForwardUse(int source_location) { - DCHECK(source_location_ == -1); +void AsmTyper::VariableInfo::SetFirstForwardUse( + const MessageLocation& source_location) { missing_definition_ = true; source_location_ = source_location; } @@ -400,7 +405,8 @@ AsmTyper::VariableInfo* AsmTyper::Lookup(Variable* variable) const { } void AsmTyper::AddForwardReference(VariableProxy* proxy, VariableInfo* info) { - info->SetFirstForwardUse(proxy->position()); + MessageLocation location(script_, proxy->position(), proxy->position()); + info->SetFirstForwardUse(location); forward_definitions_.push_back(info); } @@ -738,11 +744,8 @@ AsmType* AsmTyper::ValidateModuleAfterFunctionsPhase(FunctionLiteral* fun) { for (auto* forward_def : forward_definitions_) { if (forward_def->missing_definition()) { - int position = forward_def->source_location(); - int line = - position == kNoSourcePosition ? -1 : script_->GetLineNumber(position); - - FAIL_LINE(line, "Missing definition for forward declared identifier."); + FAIL_LOCATION(forward_def->source_location(), + "Missing definition for forward declared identifier."); } } @@ -2911,19 +2914,6 @@ AsmType* AsmTyper::NewHeapView(CallNew* new_heap_view) { return heap_view_info->type(); } -bool IsValidAsm(Isolate* isolate, Zone* zone, Handle