Merged: [interpreter] Store accumulator to callee after optional chain checks

mythrialle · mythrialle · commit f309db52c2cc · 2021-02-04T12:18:18.000Z
Revision: df98901 BUG=chromium:1171954 NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=rmcilroy@chromium.org Change-Id: If09e1503ca07b47a112362495ec0bb9d502118c9 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2674008 Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/branch-heads/8.9@{#33} Cr-Branched-From: 16b9bbb-refs/heads/8.9.255@{#1} Cr-Branched-From: d16a2a6-refs/heads/master@{#72039}
diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc
@@ -4930,8 +4930,9 @@ void BytecodeGenerator::VisitCall(Call* expr) {
       Property* property = chain->expression()->AsProperty();
       BuildOptionalChain([&]() {
         VisitAndPushIntoRegisterList(property->obj(), &args);
-        VisitPropertyLoadForRegister(args.last_register(), property, callee);
+        VisitPropertyLoad(args.last_register(), property);
       });
+      builder()->StoreAccumulatorInRegister(callee);
       break;
     }
     case Call::SUPER_CALL:
diff --git a/test/mjsunit/regress/regress-crbug-1038178.js b/test/mjsunit/regress/regress-crbug-1038178.js
@@ -15,7 +15,7 @@ function opt(){
     (((function(){})())?.v)()
 }
 %PrepareFunctionForOptimization(opt)
-assertThrows(opt());
-assertThrows(opt());
+assertThrows(() => opt());
+assertThrows(() => opt());
 %OptimizeFunctionOnNextCall(opt)
-assertThrows(opt());
+assertThrows(() => opt());
diff --git a/test/mjsunit/regress/regress-crbug-1171954.js b/test/mjsunit/regress/regress-crbug-1171954.js
@@ -0,0 +1,19 @@
+// Copyright 2021 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: --always-opt
+
+// This causes the register used by the call in the later try-catch block to be
+// used by the ToName conversion for null which causes a DCHECK fail when
+// compiling. If register allocation changes, this test may no longer reproduce
+// the crash but it is not easy write a proper test because it is linked to
+// register allocation. This test should always work, so shouldn't cause any
+// flakes.
+try {
+  var { [null]: __v_12, } = {};
+} catch (e) {}
+
+try {
+  assertEquals((__v_40?.o?.m)().p);
+} catch (e) {}
