[asm.js] Delay reporting warnings until finalization.

To avoid accessing the heap during asm.js compilation, use the pending
error handler to store the pending warnings such that they can be reported
later during finalization.

As part of this change, refactor PendingCompilationErrorHandler to have a
MessageDetails class holding details of either error or warning messages.

BUG=v8:5203

Change-Id: I5b09254f8899b8dc57d94f1986c7183da847eae3
Reviewed-on: https://chromium-review.googlesource.com/735607
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49007}

src/asmjs/asm-js.cc

@@ -145,12 +145,11 @@ void ReportCompilationSuccess(Handle<Script> script, int position,
 }
 
 // Hook to report failed execution of {AsmJs::CompileAsmViaWasm} phase.
-void ReportCompilationFailure(Handle<Script> script, int position,
+void ReportCompilationFailure(ParseInfo* parse_info, int position,
                               const char* reason) {
   if (FLAG_suppress_asm_messages) return;
-  Vector<const char> text = CStrVector(reason);
-  Report(script, position, text, MessageTemplate::kAsmJsInvalid,
-         v8::Isolate::kMessageWarning);
+  parse_info->pending_error_handler()->ReportWarningAt(
+      position, position, MessageTemplate::kAsmJsInvalid, reason);
 }
 
 // Hook to report successful execution of {AsmJs::InstantiateAsmWasm} phase.
@@ -240,17 +239,11 @@ CompilationJob::Status AsmJsCompilationJob::ExecuteJobImpl() {
   stream->Seek(compilation_info()->literal()->start_position());
   wasm::AsmJsParser parser(&translate_zone, stack_limit(), stream);
   if (!parser.Run()) {
-    // TODO(rmcilroy): Temporarily allow heap access here until we have a
-    // mechanism for delaying pending messages.
-    DCHECK(
-        ThreadId::Current().Equals(compilation_info()->isolate()->thread_id()));
-    AllowHeapAllocation allow_allocation;
-    AllowHandleAllocation allow_handles;
-    allow_deref.emplace();
-
     DCHECK(!compilation_info()->isolate()->has_pending_exception());
-    ReportCompilationFailure(parse_info()->script(), parser.failure_location(),
-                             parser.failure_message());
+    if (!FLAG_suppress_asm_messages) {
+      ReportCompilationFailure(parse_info(), parser.failure_location(),
+                               parser.failure_message());
+    }
     return FAILED;
   }
   module_ = new (compile_zone) wasm::ZoneBuffer(compile_zone);

src/compiler.cc

@@ -443,6 +443,13 @@ bool FinalizeUnoptimizedCode(
       return false;
     }
   }
+
+  // Report any warnings generated during compilation.
+  if (parse_info->pending_error_handler()->has_pending_warnings()) {
+    parse_info->pending_error_handler()->ReportWarnings(isolate,
+                                                        parse_info->script());
+  }
+
   return true;
 }
 

src/pending-compilation-error-handler.cc

@@ -14,6 +14,37 @@
 namespace v8 {
 namespace internal {
 
+Handle<String> PendingCompilationErrorHandler::MessageDetails::ArgumentString(
+    Isolate* isolate) const {
+  if (arg_ != nullptr) return arg_->string();
+  if (char_arg_ != nullptr) {
+    return isolate->factory()
+        ->NewStringFromUtf8(CStrVector(char_arg_))
+        .ToHandleChecked();
+  }
+  return isolate->factory()->undefined_string();
+}
+
+MessageLocation PendingCompilationErrorHandler::MessageDetails::GetLocation(
+    Handle<Script> script) const {
+  return MessageLocation(script, start_position_, end_position_);
+}
+
+void PendingCompilationErrorHandler::ReportWarnings(Isolate* isolate,
+                                                    Handle<Script> script) {
+  DCHECK(!has_pending_error());
+
+  for (const MessageDetails& warning : warning_messages_) {
+    MessageLocation location = warning.GetLocation(script);
+    Handle<String> argument = warning.ArgumentString(isolate);
+    Handle<JSMessageObject> message =
+        MessageHandler::MakeMessageObject(isolate, warning.message(), &location,
+                                          argument, Handle<FixedArray>::null());
+    message->set_error_level(v8::Isolate::kMessageWarning);
+    MessageHandler::ReportMessage(isolate, &location, message);
+  }
+}
+
 void PendingCompilationErrorHandler::ReportErrors(
     Isolate* isolate, Handle<Script> script,
     AstValueFactory* ast_value_factory) {
@@ -27,37 +58,22 @@ void PendingCompilationErrorHandler::ReportErrors(
   }
 }
 
-Handle<String> PendingCompilationErrorHandler::ArgumentString(
-    Isolate* isolate) {
-  if (arg_ != nullptr) return arg_->string();
-  if (char_arg_ != nullptr) {
-    return isolate->factory()
-        ->NewStringFromUtf8(CStrVector(char_arg_))
-        .ToHandleChecked();
-  }
-  return isolate->factory()->undefined_string();
-}
-
-Handle<String> PendingCompilationErrorHandler::FormatMessage(Isolate* isolate) {
-  return MessageTemplate::FormatMessage(isolate, message_,
-                                        ArgumentString(isolate));
-}
-
 void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
                                                        Handle<Script> script) {
   if (!has_pending_error_) return;
-  MessageLocation location(script, start_position_, end_position_);
-  Factory* factory = isolate->factory();
-  Handle<String> argument = ArgumentString(isolate);
+
+  MessageLocation location = error_details_.GetLocation(script);
+  Handle<String> argument = error_details_.ArgumentString(isolate);
   isolate->debug()->OnCompileError(script);
 
+  Factory* factory = isolate->factory();
   Handle<Object> error;
   switch (error_type_) {
     case kReferenceError:
-      error = factory->NewReferenceError(message_, argument);
+      error = factory->NewReferenceError(error_details_.message(), argument);
       break;
     case kSyntaxError:
-      error = factory->NewSyntaxError(message_, argument);
+      error = factory->NewSyntaxError(error_details_.message(), argument);
       break;
     default:
       UNREACHABLE();
@@ -89,5 +105,12 @@ void PendingCompilationErrorHandler::ThrowPendingError(Isolate* isolate,
 
   isolate->Throw(*error, &location);
 }
+
+Handle<String> PendingCompilationErrorHandler::FormatErrorMessageForTest(
+    Isolate* isolate) const {
+  return MessageTemplate::FormatMessage(isolate, error_details_.message(),
+                                        error_details_.ArgumentString(isolate));
+}
+
 }  // namespace internal
 }  // namespace v8

src/pending-compilation-error-handler.h

@@ -5,6 +5,8 @@
 #ifndef V8_PENDING_COMPILATION_ERROR_HANDLER_H_
 #define V8_PENDING_COMPILATION_ERROR_HANDLER_H_
 
+#include <forward_list>
+
 #include "src/base/macros.h"
 #include "src/globals.h"
 #include "src/handles.h"
@@ -25,11 +27,6 @@ class PendingCompilationErrorHandler {
   PendingCompilationErrorHandler()
       : has_pending_error_(false),
         stack_overflow_(false),
-        start_position_(-1),
-        end_position_(-1),
-        message_(MessageTemplate::kNone),
-        arg_(nullptr),
-        char_arg_(nullptr),
         error_type_(kSyntaxError) {}
 
   void ReportMessageAt(int start_position, int end_position,
@@ -38,11 +35,9 @@ class PendingCompilationErrorHandler {
                        ParseErrorType error_type = kSyntaxError) {
     if (has_pending_error_) return;
     has_pending_error_ = true;
-    start_position_ = start_position;
-    end_position_ = end_position;
-    message_ = message;
-    char_arg_ = arg;
-    arg_ = nullptr;
+
+    error_details_ =
+        MessageDetails(start_position, end_position, message, nullptr, arg);
     error_type_ = error_type;
   }
 
@@ -52,14 +47,19 @@ class PendingCompilationErrorHandler {
                        ParseErrorType error_type = kSyntaxError) {
     if (has_pending_error_) return;
     has_pending_error_ = true;
-    start_position_ = start_position;
-    end_position_ = end_position;
-    message_ = message;
-    char_arg_ = nullptr;
-    arg_ = arg;
+
+    error_details_ =
+        MessageDetails(start_position, end_position, message, arg, nullptr);
     error_type_ = error_type;
   }
 
+  void ReportWarningAt(int start_position, int end_position,
+                       MessageTemplate::Template message,
+                       const char* arg = nullptr) {
+    warning_messages_.emplace_front(
+        MessageDetails(start_position, end_position, message, nullptr, arg));
+  }
+
   bool stack_overflow() const { return stack_overflow_; }
 
   void set_stack_overflow() {
@@ -68,26 +68,58 @@ class PendingCompilationErrorHandler {
   }
 
   bool has_pending_error() const { return has_pending_error_; }
+  bool has_pending_warnings() const { return !warning_messages_.empty(); }
 
   // Handle errors detected during parsing.
   void ReportErrors(Isolate* isolate, Handle<Script> script,
                     AstValueFactory* ast_value_factory);
 
-  Handle<String> FormatMessage(Isolate* isolate);
+  // Handle warnings detected during compilation.
+  void ReportWarnings(Isolate* isolate, Handle<Script> script);
+
+  Handle<String> FormatErrorMessageForTest(Isolate* isolate) const;
 
  private:
+  class MessageDetails {
+   public:
+    MOVE_ONLY_NO_DEFAULT_CONSTRUCTOR(MessageDetails);
+    MessageDetails()
+        : start_position_(-1),
+          end_position_(-1),
+          message_(MessageTemplate::kNone),
+          arg_(nullptr),
+          char_arg_(nullptr) {}
+    MessageDetails(int start_position, int end_position,
+                   MessageTemplate::Template message, const AstRawString* arg,
+                   const char* char_arg)
+        : start_position_(start_position),
+          end_position_(end_position),
+          message_(message),
+          arg_(arg),
+          char_arg_(char_arg) {}
+
+    Handle<String> ArgumentString(Isolate* isolate) const;
+    MessageLocation GetLocation(Handle<Script> script) const;
+    MessageTemplate::Template message() const { return message_; }
+
+   private:
+    int start_position_;
+    int end_position_;
+    MessageTemplate::Template message_;
+    const AstRawString* arg_;
+    const char* char_arg_;
+  };
+
   void ThrowPendingError(Isolate* isolate, Handle<Script> script);
-  Handle<String> ArgumentString(Isolate* isolate);
 
   bool has_pending_error_;
   bool stack_overflow_;
-  int start_position_;
-  int end_position_;
-  MessageTemplate::Template message_;
-  const AstRawString* arg_;
-  const char* char_arg_;
+
+  MessageDetails error_details_;
   ParseErrorType error_type_;
 
+  std::forward_list<MessageDetails> warning_messages_;
+
   DISALLOW_COPY_AND_ASSIGN(PendingCompilationErrorHandler);
 };
 

test/cctest/test-parsing.cc

@@ -1428,7 +1428,7 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
     // Check that preparser and parser produce the same error.
     if (test_preparser && !ignore_error_msg) {
       i::Handle<i::String> preparser_message =
-          pending_error_handler.FormatMessage(CcTest::i_isolate());
+          pending_error_handler.FormatErrorMessageForTest(CcTest::i_isolate());
       if (!i::String::Equals(message_string, preparser_message)) {
         v8::base::OS::Print(
             "Expected parser and preparser to produce the same error on:\n"
@@ -1449,7 +1449,7 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
         "\t%s\n"
         "However, the parser succeeded",
         source->ToCString().get(),
-        pending_error_handler.FormatMessage(CcTest::i_isolate())
+        pending_error_handler.FormatErrorMessageForTest(CcTest::i_isolate())
             ->ToCString()
             .get());
     CHECK(false);