[interpreter] Fix order of bytecode generated for adding getters/setters

Makes the order of the generated calls to the Runtime function
DefineAccessorPropertyUnchecked fixed regardless of hashseed so that
recompilation for lazy source positions always generates the same
result.

Moves AccessorTable from src/ast/ast.h to bytecode-generator.cc since
that's the only place that uses it.

Bug: v8:9383, v8:8510
Change-Id: I89e0aad1683a793714bfb48eca1b00abe20cad0a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1669689
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#62303}

src/ast/ast.h

@@ -1424,32 +1424,6 @@ class ObjectLiteral final : public AggregateLiteral {
       : public BitField<bool, FastElementsField::kNext, 1> {};
 };
 
-
-// A map from property names to getter/setter pairs allocated in the zone.
-class AccessorTable
-    : public base::TemplateHashMap<Literal, ObjectLiteral::Accessors,
-                                   bool (*)(void*, void*),
-                                   ZoneAllocationPolicy> {
- public:
-  explicit AccessorTable(Zone* zone)
-      : base::TemplateHashMap<Literal, ObjectLiteral::Accessors,
-                              bool (*)(void*, void*), ZoneAllocationPolicy>(
-            Literal::Match, ZoneAllocationPolicy(zone)),
-        zone_(zone) {}
-
-  Iterator lookup(Literal* literal) {
-    Iterator it = find(literal, true, ZoneAllocationPolicy(zone_));
-    if (it->second == nullptr) {
-      it->second = new (zone_) ObjectLiteral::Accessors();
-    }
-    return it;
-  }
-
- private:
-  Zone* zone_;
-};
-
-
 // An array literal has a literals object that is used
 // for minimizing the work when constructing it at runtime.
 class ArrayLiteral final : public AggregateLiteral {

src/interpreter/bytecode-generator.cc

@@ -915,6 +915,45 @@ class BytecodeGenerator::IteratorRecord final {
   Register next_;
 };
 
+namespace {
+
+// A map from property names to getter/setter pairs allocated in the zone that
+// also provides a way of accessing the pairs in the order they were first
+// added so that the generated bytecode is always the same.
+class AccessorTable
+    : public base::TemplateHashMap<Literal, ObjectLiteral::Accessors,
+                                   bool (*)(void*, void*),
+                                   ZoneAllocationPolicy> {
+ public:
+  explicit AccessorTable(Zone* zone)
+      : base::TemplateHashMap<Literal, ObjectLiteral::Accessors,
+                              bool (*)(void*, void*), ZoneAllocationPolicy>(
+            Literal::Match, ZoneAllocationPolicy(zone)),
+        zone_(zone) {}
+
+  Iterator lookup(Literal* literal) {
+    Iterator it = find(literal, true, ZoneAllocationPolicy(zone_));
+    if (it->second == nullptr) {
+      it->second = new (zone_) ObjectLiteral::Accessors();
+      ordered_accessors_.push_back({literal, it->second});
+    }
+    return it;
+  }
+
+  const std::vector<std::pair<Literal*, ObjectLiteral::Accessors*>>&
+  ordered_accessors() {
+    return ordered_accessors_;
+  }
+
+ private:
+  std::vector<std::pair<Literal*, ObjectLiteral::Accessors*>>
+      ordered_accessors_;
+
+  Zone* zone_;
+};
+
+}  // namespace
+
 #ifdef DEBUG
 
 static bool IsInEagerLiterals(
@@ -2480,14 +2519,13 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
 
   // Define accessors, using only a single call to the runtime for each pair of
   // corresponding getters and setters.
-  for (AccessorTable::Iterator it = accessor_table.begin();
-       it != accessor_table.end(); ++it) {
+  for (auto accessors : accessor_table.ordered_accessors()) {
     RegisterAllocationScope inner_register_scope(this);
     RegisterList args = register_allocator()->NewRegisterList(5);
     builder()->MoveRegister(literal, args[0]);
-    VisitForRegisterValue(it->first, args[1]);
-    VisitObjectLiteralAccessor(literal, it->second->getter, args[2]);
-    VisitObjectLiteralAccessor(literal, it->second->setter, args[3]);
+    VisitForRegisterValue(accessors.first, args[1]);
+    VisitObjectLiteralAccessor(literal, accessors.second->getter, args[2]);
+    VisitObjectLiteralAccessor(literal, accessors.second->setter, args[3]);
     builder()
         ->LoadLiteral(Smi::FromInt(NONE))
         .StoreAccumulatorInRegister(args[4])

test/mjsunit/regress/regress-9383.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 --cache=code --enable-lazy-source-positions
+
+// Test ensures that the getters and setters are added in the same order in the
+// generated bytecode regardless of the isolate's hash seed. This gets tested
+// because of the use of the code cache.
+var c = {
+  get b() {
+  },
+  get getter() {
+  },
+  set a(n) {
+  },
+  set a(n) {
+  },
+  set setter1(n) {
+  },
+  set setter2(n) {
+  },
+  set setter3(n) {
+  },
+  set setter4(n) {
+  },
+  set setter5(n) {
+  },
+  set setter6(n) {
+  },
+  set setter7(n) {
+  },
+  set setter8(n) {
+  },
+  set setter9(n) {
+  },
+  set setter10(n) {
+  },
+  set setter11(n) {
+  },
+  set setter12(n) {
+  },
+  set setter12(n) {
+  },
+};
+
+for (x in c) {
+  print(x);
+}
+
+throw new Error();