From b1cfa6448cd6e048b5dd55ba9726805c9221ffe1 Mon Sep 17 00:00:00 2001
From: bradnelson <bradnelson@chromium.org>
Date: Tue, 10 Jan 2017 09:47:21 -0800
Subject: [PATCH] [wasm][asm.js] Ensure final validation phase runs.

Asm.js modules missing exports fail to run the last phase of
validation. Adding an explicit check for this.

BUG=676573
R=titzer@chromium.org,aseemgarg@chromium.org

Review-Url: https://codereview.chromium.org/2620893002
Cr-Commit-Position: refs/heads/master@{#42191}
---
 src/asmjs/asm-typer.cc             | 34 +++++++++++++++++++-----------
 src/asmjs/asm-typer.h              |  4 ++++
 src/asmjs/asm-wasm-builder.cc      | 12 +++++++++++
 test/mjsunit/asm/regress-676573.js | 17 +++++++++++++++
 4 files changed, 55 insertions(+), 12 deletions(-)
 create mode 100644 test/mjsunit/asm/regress-676573.js

diff --git a/src/asmjs/asm-typer.cc b/src/asmjs/asm-typer.cc
index 36ab1408b0..40683849dc 100644
--- a/src/asmjs/asm-typer.cc
+++ b/src/asmjs/asm-typer.cc
@@ -20,25 +20,31 @@
 #include "src/globals.h"
 #include "src/messages.h"
 #include "src/utils.h"
+#include "src/vector.h"
 
-#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();                                               \
+#define FAIL_LOCATION_RAW(location, msg)                               \
+  do {                                                                 \
+    Handle<String> message(                                            \
+        isolate_->factory()->InternalizeOneByteString(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)                                                    \
+#define FAIL_RAW(node, msg)                                                \
   do {                                                                     \
     MessageLocation location(script_, node->position(), node->position()); \
-    FAIL_LOCATION(&location, msg);                                         \
+    FAIL_LOCATION_RAW(&location, msg);                                     \
   } while (false)
 
+#define FAIL_LOCATION(location, msg) \
+  FAIL_LOCATION_RAW(location, STATIC_CHAR_VECTOR(msg))
+
+#define FAIL(node, msg) FAIL_RAW(node, STATIC_CHAR_VECTOR(msg))
+
 #define RECURSE(call)                                             \
   do {                                                            \
     if (GetCurrentStackPosition() < stack_limit_) {               \
@@ -530,6 +536,10 @@ AsmTyper::StandardMember AsmTyper::VariableAsStandardMember(Variable* var) {
   return member;
 }
 
+AsmType* AsmTyper::FailWithMessage(const char* text) {
+  FAIL_RAW(root_, OneByteVector(text));
+}
+
 bool AsmTyper::Validate() {
   return ValidateBeforeFunctionsPhase() &&
          !AsmType::None()->IsExactly(ValidateModuleFunctions(root_)) &&
diff --git a/src/asmjs/asm-typer.h b/src/asmjs/asm-typer.h
index 723731bcdd..8ddcb34b0f 100644
--- a/src/asmjs/asm-typer.h
+++ b/src/asmjs/asm-typer.h
@@ -86,6 +86,10 @@ class AsmTyper final {
   AsmType* TypeOf(Variable* v) const;
   StandardMember VariableAsStandardMember(Variable* var);
 
+  // Allow the asm-wasm-builder to trigger failures (for interleaved
+  // validating).
+  AsmType* FailWithMessage(const char* text);
+
   typedef std::unordered_set<StandardMember, std::hash<int> > StdlibSet;
 
   StdlibSet StdlibUses() const { return stdlib_uses_; }
diff --git a/src/asmjs/asm-wasm-builder.cc b/src/asmjs/asm-wasm-builder.cc
index 1d6757950a..787b8981e8 100644
--- a/src/asmjs/asm-wasm-builder.cc
+++ b/src/asmjs/asm-wasm-builder.cc
@@ -67,6 +67,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
         script_(script),
         typer_(typer),
         typer_failed_(false),
+        typer_finished_(false),
         breakable_blocks_(zone),
         foreign_variables_(zone),
         init_function_(nullptr),
@@ -122,6 +123,10 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
     if (HasStackOverflow()) {
       return false;
     }
+    if (!typer_finished_) {
+      typer_->FailWithMessage("Module missing export section.");
+      typer_failed_ = true;
+    }
     if (typer_failed_) {
       return false;
     }
@@ -322,10 +327,16 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
 
   void VisitReturnStatement(ReturnStatement* stmt) {
     if (scope_ == kModuleScope) {
+      if (typer_finished_) {
+        typer_->FailWithMessage("Module has multiple returns.");
+        typer_failed_ = true;
+        return;
+      }
       if (!typer_->ValidateAfterFunctionsPhase()) {
         typer_failed_ = true;
         return;
       }
+      typer_finished_ = true;
       scope_ = kExportScope;
       RECURSE(Visit(stmt->expression()));
       scope_ = kModuleScope;
@@ -1947,6 +1958,7 @@ class AsmWasmBuilderImpl final : public AstVisitor<AsmWasmBuilderImpl> {
   Handle<Script> script_;
   AsmTyper* typer_;
   bool typer_failed_;
+  bool typer_finished_;
   ZoneVector<std::pair<BreakableStatement*, bool>> breakable_blocks_;
   ZoneVector<ForeignVariable> foreign_variables_;
   WasmFunctionBuilder* init_function_;
diff --git a/test/mjsunit/asm/regress-676573.js b/test/mjsunit/asm/regress-676573.js
new file mode 100644
index 0000000000..a0a4cf6412
--- /dev/null
+++ b/test/mjsunit/asm/regress-676573.js
@@ -0,0 +1,17 @@
+// Copyright 2017 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.
+
+function baz() {
+  "use asm";
+}
+function B(stdlib, env) {
+  "use asm";
+  var x = env.foo | 0;
+}
+var bar = {
+  get foo() {
+  }
+};
+bar.__defineGetter__('foo', function() { return baz(); });
+B(this, bar);