From 660d828790936b7a6e5a970f879e1ab9838e80b9 Mon Sep 17 00:00:00 2001 From: peterwmwong Date: Wed, 20 Mar 2019 22:11:58 -0500 Subject: [PATCH] [debug] Mark toLocaleString and TA#join builtins as side-effect free. Bug: chromium:940373 Change-Id: If5f90ff5f873f0687c6a6a4063e0d09d6bbbd556 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1533157 Reviewed-by: Yang Guo Commit-Queue: Peter Wong Cr-Commit-Position: refs/heads/master@{#60440} --- src/debug/debug-evaluate.cc | 9 +++++++++ .../debug-evaluate-no-side-effect-builtins-2.js | 7 +++---- .../debug-evaluate-no-side-effect-builtins.js | 10 +++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/debug/debug-evaluate.cc b/src/debug/debug-evaluate.cc index 83757fc06a..12510b4f70 100644 --- a/src/debug/debug-evaluate.cc +++ b/src/debug/debug-evaluate.cc @@ -504,6 +504,7 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) { case Builtins::kObjectPrototypeIsPrototypeOf: case Builtins::kObjectPrototypePropertyIsEnumerable: case Builtins::kObjectPrototypeToString: + case Builtins::kObjectPrototypeToLocaleString: // Array builtins. case Builtins::kArrayIsArray: case Builtins::kArrayConstructor: @@ -546,12 +547,14 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) { case Builtins::kTypedArrayPrototypeFind: case Builtins::kTypedArrayPrototypeFindIndex: case Builtins::kTypedArrayPrototypeIncludes: + case Builtins::kTypedArrayPrototypeJoin: case Builtins::kTypedArrayPrototypeIndexOf: case Builtins::kTypedArrayPrototypeLastIndexOf: case Builtins::kTypedArrayPrototypeSlice: case Builtins::kTypedArrayPrototypeSubArray: case Builtins::kTypedArrayPrototypeEvery: case Builtins::kTypedArrayPrototypeSome: + case Builtins::kTypedArrayPrototypeToLocaleString: case Builtins::kTypedArrayPrototypeFilter: case Builtins::kTypedArrayPrototypeMap: case Builtins::kTypedArrayPrototypeReduce: @@ -609,6 +612,9 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) { case Builtins::kDatePrototypeToISOString: case Builtins::kDatePrototypeToUTCString: case Builtins::kDatePrototypeToString: + case Builtins::kDatePrototypeToLocaleString: + case Builtins::kDatePrototypeToLocaleDateString: + case Builtins::kDatePrototypeToLocaleTimeString: case Builtins::kDatePrototypeToTimeString: case Builtins::kDatePrototypeToJson: case Builtins::kDatePrototypeToPrimitive: @@ -674,6 +680,7 @@ DebugInfo::SideEffectState BuiltinGetSideEffectState(Builtins::Name id) { case Builtins::kNumberPrototypeToFixed: case Builtins::kNumberPrototypeToPrecision: case Builtins::kNumberPrototypeToString: + case Builtins::kNumberPrototypeToLocaleString: case Builtins::kNumberPrototypeValueOf: // BigInt builtins. case Builtins::kBigIntConstructor: @@ -977,6 +984,8 @@ static bool TransitivelyCalledBuiltinHasNoSideEffect(Builtins::Name caller, switch (caller) { case Builtins::kArrayPrototypeJoin: case Builtins::kArrayPrototypeToLocaleString: + case Builtins::kTypedArrayPrototypeJoin: + case Builtins::kTypedArrayPrototypeToLocaleString: return true; default: return false; diff --git a/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins-2.js b/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins-2.js index 8f68822052..98cfdb1764 100644 --- a/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins-2.js +++ b/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins-2.js @@ -45,11 +45,10 @@ function listener(event, exec_state, event_data, data) { if (typeof Date.prototype[f] === "function") { if (f.startsWith("set")) { fail(`date.${f}(5);`, true); - } else if (f.startsWith("toLocale")) { - if (typeof Intl === "undefined") continue; - fail(`date.${f}();`, true); + } else if (f.startsWith("toLocale") && typeof Intl === "undefined") { + continue; } else { - success(undefined, `date.${f}();`, true); + success(undefined, `date.${f}();`); } } } diff --git a/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins.js b/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins.js index 92d534ce58..29d8145b3c 100644 --- a/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins.js +++ b/test/debugger/debug/side-effect/debug-evaluate-no-side-effect-builtins.js @@ -59,6 +59,7 @@ function listener(event, exec_state, event_data, data) { success(true, `Object.prototype.isPrototypeOf({})`); success(true, `({a:1}).propertyIsEnumerable("a")`); success("[object Object]", `({a:1}).toString()`); + success("[object Object]", `({a:1}).toLocaleString()`); success("string", `(object_with_callbacks).toString()`); success(3, `(object_with_callbacks).valueOf()`); @@ -73,8 +74,8 @@ function listener(event, exec_state, event_data, data) { "flatMap", "forEach", "every", "some", "reduce", "reduceRight", "find", "filter", "map", "findIndex" ]; - var fails = ["toLocaleString", "pop", "push", "reverse", "shift", "unshift", - "splice", "sort", "copyWithin", "fill"]; + var fails = ["pop", "push", "reverse", "shift", "unshift", "splice", + "sort", "copyWithin", "fill"]; for (f of Object.getOwnPropertyNames(Array.prototype)) { if (typeof Array.prototype[f] === "function") { if (fails.includes(f)) { @@ -123,8 +124,7 @@ function listener(event, exec_state, event_data, data) { "forEach", "every", "some", "reduce", "reduceRight", "find", "filter", "map", "findIndex" ]; - fails = ["toString", "join", "toLocaleString", "reverse", "sort", - "copyWithin", "fill", "set"]; + fails = ["reverse", "sort", "copyWithin", "fill", "set"]; var typed_proto_proto = Object.getPrototypeOf(Object.getPrototypeOf(new Uint8Array())); for (f of Object.getOwnPropertyNames(typed_proto_proto)) { if (typeof typed_array[f] === "function" && f !== "constructor") { @@ -160,7 +160,7 @@ function listener(event, exec_state, event_data, data) { } for (f of Object.getOwnPropertyNames(Number.prototype)) { if (typeof Number.prototype[f] === "function") { - if (f == "toLocaleString") continue; + if (f == "toLocaleString" && typeof Intl === "undefined") continue; success(Number(0.5)[f](5), `Number(0.5).${f}(5);`); } }