From 186068ad3fd335d2451791dc4cac74ecf603b3a3 Mon Sep 17 00:00:00 2001 From: Matthias Liedtke Date: Fri, 20 Jan 2023 16:38:04 +0100 Subject: [PATCH] String.p.toLocaleLowerCase: Perform locale validation also on empty string The fast path implementation for toLocaleLowercase (added in 333db24b55ec1a75efa5851f684333c34ca63e65, https://crrev.com/c/3952317) skipped the locale validation if the string to be converted is the empty string. This CL addresses it by delaying the early return for empty string to be performed after the locale validation. Bug: chromium:1409058 Change-Id: I2f2839dc836d8de662d308c86099707bf9ddfd9e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4184199 Reviewed-by: Jakob Kummerow Auto-Submit: Matthias Liedtke Commit-Queue: Jakob Kummerow Cr-Commit-Position: refs/heads/main@{#85434} --- src/builtins/builtins-intl-gen.cc | 8 ++++---- test/mjsunit/mjsunit.status | 1 + test/mjsunit/regress/regress-1409058.js | 9 +++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/regress/regress-1409058.js diff --git a/src/builtins/builtins-intl-gen.cc b/src/builtins/builtins-intl-gen.cc index 6d72dcc929..8902662407 100644 --- a/src/builtins/builtins-intl-gen.cc +++ b/src/builtins/builtins-intl-gen.cc @@ -113,10 +113,6 @@ void IntlBuiltinsAssembler::ToLowerCaseImpl( ToLowerCaseKind kind, std::function)> ReturnFct) { Label call_c(this), return_string(this), runtime(this, Label::kDeferred); - // Early exit on empty strings. - const TNode length = LoadStringLengthAsWord32(string); - GotoIf(Word32Equal(length, Uint32Constant(0)), &return_string); - // Unpack strings if possible, and bail to runtime unless we get a one-byte // flat string. ToDirectStringAssembler to_direct( @@ -153,6 +149,10 @@ void IntlBuiltinsAssembler::ToLowerCaseImpl( Bind(&fast); } + // Early exit on empty string. + const TNode length = LoadStringLengthAsWord32(string); + GotoIf(Word32Equal(length, Uint32Constant(0)), &return_string); + const TNode instance_type = to_direct.instance_type(); CSA_DCHECK(this, Word32BinaryNot(IsIndirectStringInstanceType(instance_type))); diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 6b0e5e7c30..aebf497e90 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -470,6 +470,7 @@ # noi18n is required for Intl 'regress/regress-crbug-1052647': [PASS,FAIL], + 'regress/regress-1409058': [SKIP], # Temporal intl tests won't work in no_i18n 'temporal/function-exist': [FAIL], diff --git a/test/mjsunit/regress/regress-1409058.js b/test/mjsunit/regress/regress-1409058.js new file mode 100644 index 0000000000..e9351b1723 --- /dev/null +++ b/test/mjsunit/regress/regress-1409058.js @@ -0,0 +1,9 @@ +// Copyright 2023 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. + +for (const invalidLocale of ["", "1", "12", "123", "1234"]) { + print(invalidLocale); + assertThrows(() => "".toLocaleUpperCase(invalidLocale), RangeError); + assertThrows(() => "".toLocaleLowerCase(invalidLocale), RangeError); +}