Reviewers: jarin,
Description:
[turbofan] Fix C++ evaluation order in AstGraphBuilder.
The evaluation order of receiver versus arguments is not properly
defined by C++. This caused issues with Clang where the environment
changed after the receiveing environment was already loaded.
[email protected]
BUG=chromium:467531
TEST=mjsunit/regress/regress-crbug-467531
Please review this at https://codereview.chromium.org/1015683002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+36, -6 lines):
M src/compiler/ast-graph-builder.cc
A test/mjsunit/regress/regress-crbug-467531.js
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc
b/src/compiler/ast-graph-builder.cc
index
63696e45cd2e7f2dbe932a2ac415e1c76e30053e..de01e24ab8fd226c59fe0538d72e2a3fd0313eab
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -1307,9 +1307,10 @@ void
AstGraphBuilder::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
// The result value, dispatch token and message is expected on the
operand
// stack (this is in sync with FullCodeGenerator::EnterFinallyBlock).
+ Node* message = BuildLoadExternal(message_object, kMachAnyTagged);
environment()->Push(token); // TODO(mstarzinger): Cook token!
environment()->Push(result);
- environment()->Push(BuildLoadExternal(message_object, kMachAnyTagged));
+ environment()->Push(message);
// Evaluate the finally-block.
Visit(stmt->finally_block());
@@ -1317,9 +1318,10 @@ void
AstGraphBuilder::VisitTryFinallyStatement(TryFinallyStatement* stmt) {
// The result value, dispatch token and message is restored from the
operand
// stack (this is in sync with FullCodeGenerator::ExitFinallyBlock).
- BuildStoreExternal(message_object, kMachAnyTagged, environment()->Pop());
+ message = environment()->Pop();
result = environment()->Pop();
token = environment()->Pop(); // TODO(mstarzinger): Uncook token!
+ BuildStoreExternal(message_object, kMachAnyTagged, message);
// Dynamic dispatch after the finally-block.
commands->ApplyDeferredCommands(token, result);
@@ -1650,8 +1652,9 @@ void
AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
environment()->Push(literal); // Duplicate receiver.
VisitForValue(property->key());
- environment()->Push(BuildToName(environment()->Pop(),
-
expr->GetIdForProperty(property_index)));
+ Node* name = BuildToName(environment()->Pop(),
+ expr->GetIdForProperty(property_index));
+ environment()->Push(name);
// TODO(mstarzinger): For ObjectLiteral::Property::PROTOTYPE the key
should
// not be on the operand stack while the value is being evaluated.
Come up
// with a repro for this and fix it. Also find a nice way to do so. :)
@@ -2475,7 +2478,8 @@ Node*
AstGraphBuilder::BuildPatchReceiverToGlobalProxy(Node* receiver) {
Node* check = NewNode(javascript()->StrictEqual(), receiver, undefined);
receiver_check.If(check);
receiver_check.Then();
- environment()->Push(BuildLoadGlobalProxy());
+ Node* proxy = BuildLoadGlobalProxy();
+ environment()->Push(proxy);
receiver_check.Else();
environment()->Push(receiver);
receiver_check.End();
@@ -2574,7 +2578,8 @@ Node* AstGraphBuilder::BuildHoleCheckThrow(Node*
value, Variable* variable,
Node* check = NewNode(javascript()->StrictEqual(), value, the_hole);
hole_check.If(check);
hole_check.Then();
- environment()->Push(BuildThrowReferenceError(variable, bailout_id));
+ Node* error = BuildThrowReferenceError(variable, bailout_id);
+ environment()->Push(error);
hole_check.Else();
environment()->Push(not_hole);
hole_check.End();
Index: test/mjsunit/regress/regress-crbug-467531.js
diff --git a/test/mjsunit/regress/regress-crbug-467531.js
b/test/mjsunit/regress/regress-crbug-467531.js
new file mode 100644
index
0000000000000000000000000000000000000000..73256c7acc747bebf076e0330ac30c204359239d
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-467531.js
@@ -0,0 +1,25 @@
+// Copyright 2015 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: --turbo-filter=* --always-opt
+
+assertThrows(function() {
+ "use strict";
+ try {
+ x = ref_error;
+ let x = 0;
+ } catch (e) {
+ throw e;
+ }
+}, ReferenceError);
+
+assertThrows(function() {
+ "use strict";
+ try {
+ x = ref_error;
+ let x = 0;
+ } finally {
+ // re-throw
+ }
+}, ReferenceError);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
