From 08aec7d721dc461e4f46803298f8efed38534c17 Mon Sep 17 00:00:00 2001 From: Benedikt Meurer Date: Thu, 20 Sep 2018 22:00:30 +0200 Subject: [PATCH] [es2015] Fix ToPrimitive conversions in relational comparisons. The order in which ToNumber(left) and ToPrimitive(right,hint Number) is called when performing an abstract relational comparison is observable, and we need to make sure to trigger the conversions in the correct order. Bug: chromium:687063 Change-Id: Idc9edb99643c4cf1774b89dcdc319ed5dc7cdc8a Reviewed-on: https://chromium-review.googlesource.com/1236557 Reviewed-by: Ross McIlroy Reviewed-by: Benedikt Meurer Commit-Queue: Benedikt Meurer Cr-Commit-Position: refs/heads/master@{#56125} --- src/code-stub-assembler.cc | 5 ++-- test/mjsunit/regress/regress-crbug-687063.js | 31 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-687063.js diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index 61df9fb75c..039090623b 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -10713,15 +10713,16 @@ Node* CodeStubAssembler::RelationalComparison(Operation op, Node* left, } // If {left} is a receiver, call ToPrimitive(left, hint Number). - // Otherwise call ToNumeric(left) and then ToNumeric(right). + // Otherwise call ToNumeric(right) and then ToNumeric(left), the + // order here is important as it's observable by user code. STATIC_ASSERT(LAST_JS_RECEIVER_TYPE == LAST_TYPE); Label if_left_receiver(this, Label::kDeferred); GotoIf(IsJSReceiverInstanceType(left_instance_type), &if_left_receiver); + var_right.Bind(CallBuiltin(Builtins::kToNumeric, context, right)); var_left.Bind( CallBuiltin(Builtins::kNonNumberToNumeric, context, left)); - var_right.Bind(CallBuiltin(Builtins::kToNumeric, context, right)); Goto(&loop); BIND(&if_left_receiver); diff --git a/test/mjsunit/regress/regress-crbug-687063.js b/test/mjsunit/regress/regress-crbug-687063.js new file mode 100644 index 0000000000..8c579331fb --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-687063.js @@ -0,0 +1,31 @@ +// Copyright 2018 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: --allow-natives-syntax + +// Collect the actual properties looked up on the Proxy. +const actual = []; + +// Perform a relational comparison with a Proxy on the right hand +// side and a Symbol which cannot be turned into a Number on the +// left hand side. +function foo() { + actual.length = 0; + const lhs = Symbol(); + const rhs = new Proxy({}, { + get: function(target, property, receiver) { + actual.push(property); + return undefined; + } + }); + return lhs < rhs; +} + +assertThrows(foo, TypeError); +assertEquals([Symbol.toPrimitive, 'valueOf', 'toString'], actual); +assertThrows(foo, TypeError); +assertEquals([Symbol.toPrimitive, 'valueOf', 'toString'], actual); +%OptimizeFunctionOnNextCall(foo); +assertThrows(foo, TypeError); +assertEquals([Symbol.toPrimitive, 'valueOf', 'toString'], actual);