[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}
This commit is contained in:
bradnelson 2016-12-12 06:47:56 -08:00 committed by Commit bot
parent be9ee2237d
commit aabbbec67c
13 changed files with 275 additions and 100 deletions

View File

@ -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<Value> data = Local<Value>());
/**
* 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<Value> data = Local<Value>());
/**
* Remove all message listeners from the specified callback function.
*/

View File

@ -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<int> Message::GetStartColumn(Local<Context> context) const {
auto self = Utils::OpenHandle(this);
@ -8452,18 +8456,24 @@ bool Isolate::IsDead() {
return isolate->IsDead();
}
bool Isolate::AddMessageListener(MessageCallback that, Local<Value> data) {
return AddMessageListenerWithErrorLevel(that, kMessageError, data);
}
bool Isolate::AddMessageListenerWithErrorLevel(MessageCallback that,
int message_levels,
Local<Value> data) {
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this);
ENTER_V8(isolate);
i::HandleScope scope(isolate);
i::Handle<i::TemplateList> list = isolate->factory()->message_listeners();
i::Handle<i::FixedArray> listener = isolate->factory()->NewFixedArray(2);
i::Handle<i::FixedArray> listener = isolate->factory()->NewFixedArray(3);
i::Handle<i::Foreign> 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;

View File

@ -160,8 +160,9 @@ MaybeHandle<FixedArray> 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<FixedArray>();
}
wasm::ZoneBuffer* module = asm_wasm_result.module_bytes;

View File

@ -9,6 +9,7 @@
#include <memory>
#include <string>
#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<String> message(isolate_->factory()->InternalizeOneByteString( \
STATIC_CHAR_VECTOR(msg))); \
error_message_ = MessageHandler::MakeMessageObject( \
isolate_, MessageTemplate::kAsmJsInvalid, (location), message, \
Handle<JSArray>::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<Script> script,
FunctionLiteral* root, std::string* error_message) {
error_message->clear();
AsmTyper typer(isolate, zone, script, root);
if (typer.Validate()) {
return true;
}
*error_message = typer.error_message();
return false;
}
} // namespace wasm
} // namespace internal
} // namespace v8

View File

@ -16,6 +16,7 @@
#include "src/ast/ast-types.h"
#include "src/ast/ast.h"
#include "src/effects.h"
#include "src/messages.h"
#include "src/type-info.h"
#include "src/zone/zone-containers.h"
#include "src/zone/zone.h"
@ -78,7 +79,8 @@ class AsmTyper final {
bool ValidateAfterFunctionsPhase();
void ClearFunctionNodeTypes();
const char* error_message() const { return error_message_; }
Handle<JSMessageObject> error_message() const { return error_message_; }
const MessageLocation* message_location() const { return &message_location_; }
AsmType* TypeOf(AstNode* node) const;
AsmType* TypeOf(Variable* v) const;
@ -138,7 +140,7 @@ class AsmTyper final {
bool IsHeap() const { return standard_member_ == kHeap; }
void MarkDefined() { missing_definition_ = false; }
void SetFirstForwardUse(int source_location);
void SetFirstForwardUse(const MessageLocation& source_location);
StandardMember standard_member() const { return standard_member_; }
void set_standard_member(StandardMember standard_member) {
@ -153,7 +155,7 @@ class AsmTyper final {
bool missing_definition() const { return missing_definition_; }
int source_location() const { return source_location_; }
const MessageLocation* source_location() { return &source_location_; }
static VariableInfo* ForSpecialSymbol(Zone* zone,
StandardMember standard_member);
@ -165,11 +167,8 @@ class AsmTyper final {
// missing_definition_ is set to true for forward definition - i.e., use
// before definition.
bool missing_definition_ = false;
// source_location_ holds the line number that first referenced this
// VariableInfo. Used for error messages.
// TODO(bradnelson): When merged with console change, this should
// become a source location.
int source_location_ = -1;
// Used for error messages.
MessageLocation source_location_;
};
// RAII-style manager for the in_function_ member variable.
@ -397,7 +396,8 @@ class AsmTyper final {
static const int kErrorMessageLimit = 128;
AsmType* fround_type_;
AsmType* ffi_type_;
char error_message_[kErrorMessageLimit];
Handle<JSMessageObject> error_message_;
MessageLocation message_location_;
StdlibSet stdlib_uses_;
SourceLayoutTracker source_layout_;

View File

@ -1579,9 +1579,43 @@ Local<ObjectTemplate> Shell::CreateGlobalTemplate(Isolate* isolate) {
return global_template;
}
static void EmptyMessageCallback(Local<Message> message, Local<Value> error) {
// Nothing to be done here, exceptions thrown up to the shell will be reported
static void PrintNonErrorsMessageCallback(Local<Message> message,
Local<Value> error) {
// Nothing to do here for errors, exceptions thrown up to the shell will be
// reported
// separately by {Shell::ReportException} after they are caught.
// Do print other kinds of messages.
switch (message->ErrorLevel()) {
case v8::Isolate::kMessageWarning:
case v8::Isolate::kMessageLog:
case v8::Isolate::kMessageInfo:
case v8::Isolate::kMessageDebug: {
break;
}
case v8::Isolate::kMessageError: {
// Ignore errors, printed elsewhere.
return;
}
default: {
UNREACHABLE();
break;
}
}
// Converts a V8 value to a C string.
auto ToCString = [](const v8::String::Utf8Value& value) {
return *value ? *value : "<string conversion failed>";
};
Isolate* isolate = Isolate::GetCurrent();
v8::String::Utf8Value msg(message->Get());
const char* msg_string = ToCString(msg);
// 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;
printf("%s:%i: %s\n", filename_string, linenum, msg_string);
}
void Shell::Initialize(Isolate* isolate) {
@ -1589,7 +1623,11 @@ void Shell::Initialize(Isolate* isolate) {
if (i::StrLength(i::FLAG_map_counters) != 0)
MapCounters(isolate, i::FLAG_map_counters);
// Disable default message reporting.
isolate->AddMessageListener(EmptyMessageCallback);
isolate->AddMessageListenerWithErrorLevel(
PrintNonErrorsMessageCallback,
v8::Isolate::kMessageError | v8::Isolate::kMessageWarning |
v8::Isolate::kMessageInfo | v8::Isolate::kMessageDebug |
v8::Isolate::kMessageLog);
}

View File

@ -2295,6 +2295,7 @@ Handle<JSMessageObject> Factory::NewJSMessageObject(
message_obj->set_end_position(end_position);
message_obj->set_script(*script);
message_obj->set_stack_frames(*stack_frames);
message_obj->set_error_level(v8::Isolate::kMessageError);
return message_obj;
}

View File

@ -47,10 +47,9 @@ void MessageHandler::DefaultMessageReport(Isolate* isolate,
}
}
Handle<JSMessageObject> MessageHandler::MakeMessageObject(
Isolate* isolate, MessageTemplate::Template message,
MessageLocation* location, Handle<Object> argument,
const MessageLocation* location, Handle<Object> argument,
Handle<JSArray> stack_frames) {
Factory* factory = isolate->factory();
@ -75,50 +74,63 @@ Handle<JSMessageObject> MessageHandler::MakeMessageObject(
return message_obj;
}
void MessageHandler::ReportMessage(Isolate* isolate, MessageLocation* loc,
void MessageHandler::ReportMessage(Isolate* isolate, const MessageLocation* loc,
Handle<JSMessageObject> message) {
// We are calling into embedder's code which can throw exceptions.
// Thus we need to save current exception state, reset it to the clean one
// and ignore scheduled exceptions callbacks can throw.
// We pass the exception object into the message handler callback though.
Object* exception_object = isolate->heap()->undefined_value();
if (isolate->has_pending_exception()) {
exception_object = isolate->pending_exception();
}
Handle<Object> exception(exception_object, isolate);
Isolate::ExceptionScope exception_scope(isolate);
isolate->clear_pending_exception();
isolate->set_external_caught_exception(false);
// Turn the exception on the message into a string if it is an object.
if (message->argument()->IsJSObject()) {
HandleScope scope(isolate);
Handle<Object> argument(message->argument(), isolate);
MaybeHandle<Object> maybe_stringified;
Handle<Object> stringified;
// Make sure we don't leak uncaught internally generated Error objects.
if (argument->IsJSError()) {
maybe_stringified = Object::NoSideEffectsToString(isolate, argument);
} else {
v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
catcher.SetVerbose(false);
catcher.SetCaptureMessage(false);
maybe_stringified = Object::ToString(isolate, argument);
}
if (!maybe_stringified.ToHandle(&stringified)) {
stringified = isolate->factory()->NewStringFromAsciiChecked("exception");
}
message->set_argument(*stringified);
}
v8::Local<v8::Message> api_message_obj = v8::Utils::MessageToLocal(message);
v8::Local<v8::Value> api_exception_obj = v8::Utils::ToLocal(exception);
if (api_message_obj->ErrorLevel() == v8::Isolate::kMessageError) {
// We are calling into embedder's code which can throw exceptions.
// Thus we need to save current exception state, reset it to the clean one
// and ignore scheduled exceptions callbacks can throw.
// We pass the exception object into the message handler callback though.
Object* exception_object = isolate->heap()->undefined_value();
if (isolate->has_pending_exception()) {
exception_object = isolate->pending_exception();
}
Handle<Object> exception(exception_object, isolate);
Isolate::ExceptionScope exception_scope(isolate);
isolate->clear_pending_exception();
isolate->set_external_caught_exception(false);
// Turn the exception on the message into a string if it is an object.
if (message->argument()->IsJSObject()) {
HandleScope scope(isolate);
Handle<Object> argument(message->argument(), isolate);
MaybeHandle<Object> maybe_stringified;
Handle<Object> stringified;
// Make sure we don't leak uncaught internally generated Error objects.
if (argument->IsJSError()) {
maybe_stringified = Object::NoSideEffectsToString(isolate, argument);
} else {
v8::TryCatch catcher(reinterpret_cast<v8::Isolate*>(isolate));
catcher.SetVerbose(false);
catcher.SetCaptureMessage(false);
maybe_stringified = Object::ToString(isolate, argument);
}
if (!maybe_stringified.ToHandle(&stringified)) {
stringified =
isolate->factory()->NewStringFromAsciiChecked("exception");
}
message->set_argument(*stringified);
}
v8::Local<v8::Value> api_exception_obj = v8::Utils::ToLocal(exception);
ReportMessageNoExceptions(isolate, loc, message, api_exception_obj);
} else {
ReportMessageNoExceptions(isolate, loc, message, v8::Local<v8::Value>());
}
}
void MessageHandler::ReportMessageNoExceptions(
Isolate* isolate, const MessageLocation* loc, Handle<Object> message,
v8::Local<v8::Value> api_exception_obj) {
v8::Local<v8::Message> api_message_obj = v8::Utils::MessageToLocal(message);
int error_level = api_message_obj->ErrorLevel();
Handle<TemplateList> global_listeners =
isolate->factory()->message_listeners();
@ -134,6 +146,11 @@ void MessageHandler::ReportMessage(Isolate* isolate, MessageLocation* loc,
if (global_listeners->get(i)->IsUndefined(isolate)) continue;
FixedArray* listener = FixedArray::cast(global_listeners->get(i));
Foreign* callback_obj = Foreign::cast(listener->get(0));
int32_t message_levels =
static_cast<int32_t>(Smi::cast(listener->get(2))->value());
if (!(message_levels & error_level)) {
continue;
}
v8::MessageCallback callback =
FUNCTION_CAST<v8::MessageCallback>(callback_obj->foreign_address());
Handle<Object> callback_data(listener->get(1), isolate);

View File

@ -669,6 +669,8 @@ class ErrorUtils : public AllStatic {
T(WasmTrapFuncSigMismatch, "function signature mismatch") \
T(WasmTrapInvalidIndex, "invalid index into function table") \
T(WasmTrapTypeError, "invalid type") \
/* Asm.js validation warnings */ \
T(AsmJsInvalid, "Invalid asm.js: %") \
/* DataCloneError messages */ \
T(DataCloneError, "% could not be cloned.") \
T(DataCloneErrorNeuteredArrayBuffer, \
@ -709,11 +711,11 @@ class MessageHandler {
// Returns a message object for the API to use.
static Handle<JSMessageObject> MakeMessageObject(
Isolate* isolate, MessageTemplate::Template type,
MessageLocation* location, Handle<Object> argument,
const MessageLocation* location, Handle<Object> argument,
Handle<JSArray> stack_frames);
// Report a formatted message (needs JS allocation).
static void ReportMessage(Isolate* isolate, MessageLocation* loc,
static void ReportMessage(Isolate* isolate, const MessageLocation* loc,
Handle<JSMessageObject> message);
static void DefaultMessageReport(Isolate* isolate, const MessageLocation* loc,
@ -721,6 +723,12 @@ class MessageHandler {
static Handle<String> GetMessage(Isolate* isolate, Handle<Object> data);
static std::unique_ptr<char[]> GetLocalizedMessage(Isolate* isolate,
Handle<Object> data);
private:
static void ReportMessageNoExceptions(Isolate* isolate,
const MessageLocation* loc,
Handle<Object> message_obj,
v8::Local<v8::Value> api_exception_obj);
};

View File

@ -6808,7 +6808,7 @@ ACCESSORS(JSMessageObject, script, Object, kScriptOffset)
ACCESSORS(JSMessageObject, stack_frames, Object, kStackFramesOffset)
SMI_ACCESSORS(JSMessageObject, start_position, kStartPositionOffset)
SMI_ACCESSORS(JSMessageObject, end_position, kEndPositionOffset)
SMI_ACCESSORS(JSMessageObject, error_level, kErrorLevelOffset)
INT_ACCESSORS(Code, instruction_size, kInstructionSizeOffset)
INT_ACCESSORS(Code, prologue_offset, kPrologueOffset)

View File

@ -8887,6 +8887,9 @@ class JSMessageObject: public JSObject {
// position, or the empty string if the position is invalid.
Handle<String> GetSourceLine() const;
inline int error_level() const;
inline void set_error_level(int level);
DECLARE_CAST(JSMessageObject)
// Dispatched behavior.
@ -8900,7 +8903,8 @@ class JSMessageObject: public JSObject {
static const int kStackFramesOffset = kScriptOffset + kPointerSize;
static const int kStartPositionOffset = kStackFramesOffset + kPointerSize;
static const int kEndPositionOffset = kStartPositionOffset + kPointerSize;
static const int kSize = kEndPositionOffset + kPointerSize;
static const int kErrorLevelOffset = kEndPositionOffset + kPointerSize;
static const int kSize = kErrorLevelOffset + kPointerSize;
typedef FixedBodyDescriptor<HeapObject::kMapOffset,
kStackFramesOffset + kPointerSize,

View File

@ -126,7 +126,8 @@ class AsmTyperHarnessBuilder {
WithGlobal(var_name, type);
auto* var_info = typer_->Lookup(DeclareVariable(var_name));
CHECK(var_info);
var_info->SetFirstForwardUse(-1);
MessageLocation location;
var_info->SetFirstForwardUse(location);
return this;
}
@ -260,12 +261,14 @@ class AsmTyperHarnessBuilder {
return false;
}
if (std::strstr(typer_->error_message(), error_message) == nullptr) {
std::unique_ptr<char[]> msg = i::MessageHandler::GetLocalizedMessage(
isolate_, typer_->error_message());
if (std::strstr(msg.get(), error_message) == nullptr) {
std::cerr << "Asm validation failed with the wrong error message:\n"
"Expected to contain '"
<< error_message << "'\n"
" Actually is '"
<< typer_->error_message() << "'\n";
<< msg.get() << "'\n";
return false;
}

View File

@ -17349,6 +17349,79 @@ TEST(CaptureStackTraceForUncaughtExceptionAndSetters) {
isolate->SetCaptureStackTraceForUncaughtExceptions(false);
}
static int asm_warning_triggered = 0;
static void AsmJsWarningListener(v8::Local<v8::Message> message,
v8::Local<Value>) {
DCHECK_EQ(v8::Isolate::kMessageWarning, message->ErrorLevel());
asm_warning_triggered = 1;
}
TEST(AsmJsWarning) {
i::FLAG_validate_asm = true;
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
v8::HandleScope scope(isolate);
asm_warning_triggered = 0;
isolate->AddMessageListenerWithErrorLevel(AsmJsWarningListener,
v8::Isolate::kMessageAll);
CompileRun(
"function module() {\n"
" 'use asm';\n"
" var x = 'hi';\n"
" return {};\n"
"}\n"
"module();");
DCHECK_EQ(1, asm_warning_triggered);
isolate->RemoveMessageListeners(AsmJsWarningListener);
}
static int error_level_message_count = 0;
static int expected_error_level = 0;
static void ErrorLevelListener(v8::Local<v8::Message> message,
v8::Local<Value>) {
DCHECK_EQ(expected_error_level, message->ErrorLevel());
++error_level_message_count;
}
TEST(ErrorLevelWarning) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
v8::HandleScope scope(isolate);
const char* source = "fake = 1;";
v8::Local<v8::Script> lscript = CompileWithOrigin(source, "test");
i::Handle<i::SharedFunctionInfo> obj = i::Handle<i::SharedFunctionInfo>::cast(
v8::Utils::OpenHandle(*lscript->GetUnboundScript()));
CHECK(obj->script()->IsScript());
i::Handle<i::Script> script(i::Script::cast(obj->script()));
int levels[] = {
v8::Isolate::kMessageLog, v8::Isolate::kMessageInfo,
v8::Isolate::kMessageDebug, v8::Isolate::kMessageWarning,
};
error_level_message_count = 0;
isolate->AddMessageListenerWithErrorLevel(ErrorLevelListener,
v8::Isolate::kMessageAll);
for (size_t i = 0; i < arraysize(levels); i++) {
i::MessageLocation location(script, 0, 0);
i::Handle<i::String> msg(i_isolate->factory()->InternalizeOneByteString(
STATIC_CHAR_VECTOR("test")));
i::Handle<i::JSMessageObject> message =
i::MessageHandler::MakeMessageObject(
i_isolate, i::MessageTemplate::kAsmJsInvalid, &location, msg,
i::Handle<i::JSArray>::null());
message->set_error_level(levels[i]);
expected_error_level = levels[i];
i::MessageHandler::ReportMessage(i_isolate, &location, message);
}
isolate->RemoveMessageListeners(ErrorLevelListener);
DCHECK_EQ(arraysize(levels), error_level_message_count);
}
static void StackTraceFunctionNameListener(v8::Local<v8::Message> message,
v8::Local<Value>) {