[parser] Force stable order for variables

When parsing an arrowhead, it's possible for temporary variables to be
created with a different index depending on whether the parsing is lazy
or eager. This then results in bytecode mismatches as the index is used
to determine which register to use.

To make the ordering stable, this changes variable allocation in arrow
functions to always allocate the non-temporaries first and then the
temporaries.

Bug: chromium:1020162
Change-Id: Ia47c4c2916d63f12d20d663e4e3842bfd68f6d8e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1977865
Commit-Queue: Dan Elphick <delphick@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65657}

src/ast/scopes.cc

@@ -2327,8 +2327,23 @@ void Scope::AllocateNonParameterLocal(Variable* var) {
 }
 
 void Scope::AllocateNonParameterLocalsAndDeclaredGlobals() {
-  for (Variable* local : locals_) {
-    AllocateNonParameterLocal(local);
+  if (is_declaration_scope() && AsDeclarationScope()->is_arrow_scope()) {
+    // In arrow functions, allocate non-temporaries first and then all the
+    // temporaries to make the local variable ordering stable when reparsing to
+    // collect source positions.
+    for (Variable* local : locals_) {
+      if (local->mode() != VariableMode::kTemporary)
+        AllocateNonParameterLocal(local);
+    }
+
+    for (Variable* local : locals_) {
+      if (local->mode() == VariableMode::kTemporary)
+        AllocateNonParameterLocal(local);
+    }
+  } else {
+    for (Variable* local : locals_) {
+      AllocateNonParameterLocal(local);
+    }
   }
 
   if (is_declaration_scope()) {

test/mjsunit/regress/regress-crbug-1020162.js

+// Copyright 2019 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.
+
+// Flags: --throws
+
+((get = foo.get(...a, a), b) => 0)();