Commit 1c31ba7f authored by Santiago Aboy Solanes's avatar Santiago Aboy Solanes Committed by Commit Bot

[compiler] Don't acquire the lock in TryStringToDouble

In https://crrev.com/c/v8/v8/+/2536465 we added acquiring the lock in
WriteToFlat. Then, acquiring in TryStringToDouble not only is not
necessary but also has undefined behaviour.

This was causing timeouts and meant the tests were disabled in
https://crrev.com/c/v8/v8/+/2543398.

Bug: v8:7790, v8:11171
Change-Id: Iaab4e5079bac96786e536a2e4b766e93ea17e2c4
Fixes: v8:11171
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2544544Reviewed-by: 's avatarRoss McIlroy <rmcilroy@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71242}
parent bf52ff62
...@@ -1379,17 +1379,7 @@ base::Optional<double> TryStringToDouble(Handle<String> object, ...@@ -1379,17 +1379,7 @@ base::Optional<double> TryStringToDouble(Handle<String> object,
const int flags = ALLOW_HEX | ALLOW_OCTAL | ALLOW_BINARY; const int flags = ALLOW_HEX | ALLOW_OCTAL | ALLOW_BINARY;
auto buffer = std::make_unique<uc16[]>(max_length_for_conversion); auto buffer = std::make_unique<uc16[]>(max_length_for_conversion);
Isolate* isolate = nullptr;
// Read only strings have no isolate associated to them and we don't need a
// lock for those.
const bool need_lock = GetIsolateFromHeapObject(*object, &isolate);
if (need_lock) {
isolate->string_access()->LockShared();
}
String::WriteToFlat(*object, buffer.get(), 0, length); String::WriteToFlat(*object, buffer.get(), 0, length);
if (need_lock) {
isolate->string_access()->UnlockShared();
}
Vector<const uc16> v(buffer.get(), length); Vector<const uc16> v(buffer.get(), length);
return StringToDouble(v, flags); return StringToDouble(v, flags);
} }
......
...@@ -161,10 +161,10 @@ inline uint64_t PositiveNumberToUint64(Object number); ...@@ -161,10 +161,10 @@ inline uint64_t PositiveNumberToUint64(Object number);
double StringToDouble(Isolate* isolate, Handle<String> string, int flags, double StringToDouble(Isolate* isolate, Handle<String> string, int flags,
double empty_string_val = 0.0); double empty_string_val = 0.0);
// String to double helper without heap allocation. It will acquire the string // String to double helper without heap allocation.
// lock in order to be thread-safe. Returns base::nullopt if the string is // Returns base::nullopt if the string is longer than
// longer than {max_length_for_conversion}. 23 was chosen because any // {max_length_for_conversion}. 23 was chosen because any representable double
// representable double can be represented using a string of length 23. // can be represented using a string of length 23.
V8_EXPORT_PRIVATE base::Optional<double> TryStringToDouble( V8_EXPORT_PRIVATE base::Optional<double> TryStringToDouble(
Handle<String> object, int max_length_for_conversion = 23); Handle<String> object, int max_length_for_conversion = 23);
......
...@@ -117,9 +117,6 @@ ...@@ -117,9 +117,6 @@
'test-code-stub-assembler/PopAndReturnFromJSBuiltinWithStackParameters' : [FAIL], 'test-code-stub-assembler/PopAndReturnFromJSBuiltinWithStackParameters' : [FAIL],
'test-code-stub-assembler/PopAndReturnFromTFCBuiltinWithStackParameters' : [FAIL], 'test-code-stub-assembler/PopAndReturnFromTFCBuiltinWithStackParameters' : [FAIL],
# BUG(v8:11171).
'test-concurrent-string/InspectOneIntoTwoByteExternalizing' : [SKIP],
############################################################################ ############################################################################
# Slow tests. # Slow tests.
'test-debug/CallFunctionInDebugger': [PASS, ['mode == debug', SLOW]], 'test-debug/CallFunctionInDebugger': [PASS, ['mode == debug', SLOW]],
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment