From 70f691315929cf408fafee7811bef635045523a8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= <marja@chromium.org>
Date: Fri, 11 Aug 2017 09:55:01 +0200
Subject: [PATCH] [parser] Skipping inner funcs: remove untrue DCHECK.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- See bug for the reduced test case.

- Not adding a regression test here: I don't want to assert that PreParser
  doesn't detect the redeclaration error, OTOH I don't want to make it detect
  the error either (in order to not couple detecting the error with
  FLAG_experimental_preparser_analysis).

BUG=chromium:753896, v8:5516

Change-Id: I0f1beffe30e5cb48d6dbec35181980864e6df153
Reviewed-on: https://chromium-review.googlesource.com/608976
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47326}
---
 src/ast/scopes.cc                     |  7 ++++++-
 test/cctest/parsing/test-preparser.cc | 20 ++++++++++++++++++++
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 472d64bee6..c7d4899552 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -1216,8 +1216,13 @@ Variable* Scope::DeclareVariableName(const AstRawString* name,
     DCHECK_NE(var, kDummyPreParserVariable);
     if (var == nullptr) {
       var = DeclareLocal(name, mode);
+    } else if (IsLexicalVariableMode(mode) ||
+               IsLexicalVariableMode(var->mode())) {
+      // Duplicate functions are allowed in the sloppy mode, but if this is not
+      // a function declaration, it's an error. This is an error PreParser
+      // hasn't previously detected. TODO(marja): Investigate whether we can now
+      // start returning this error.
     } else if (mode == VAR) {
-      DCHECK_EQ(var->mode(), VAR);
       var->set_maybe_assigned();
     }
     var->set_is_used();
diff --git a/test/cctest/parsing/test-preparser.cc b/test/cctest/parsing/test-preparser.cc
index 7e08209079..076103a855 100644
--- a/test/cctest/parsing/test-preparser.cc
+++ b/test/cctest/parsing/test-preparser.cc
@@ -768,3 +768,23 @@ TEST(PreParserScopeAnalysis) {
     }
   }
 }
+
+// Regression test for
+// https://bugs.chromium.org/p/chromium/issues/detail?id=753896. Should not
+// crash.
+TEST(Regress753896) {
+  i::FLAG_experimental_preparser_scope_analysis = true;
+  i::Isolate* isolate = CcTest::i_isolate();
+  i::Factory* factory = isolate->factory();
+  i::HandleScope scope(isolate);
+  LocalContext env;
+
+  i::Handle<i::String> source = factory->InternalizeUtf8String(
+      "function lazy() { let v = 0; if (true) { var v = 0; } }");
+  i::Handle<i::Script> script = factory->NewScript(source);
+  i::ParseInfo info(script);
+
+  // We don't assert that parsing succeeded or that it failed; currently the
+  // error is not detected inside lazy functions, but it might be in the future.
+  i::parsing::ParseProgram(&info, isolate);
+}