AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

parsing: Lock ExternalStrings in the ExternalStringStream.

Browse Source 
Utf*Characterstream caches the data pointer of ExternalStrings through
ExternalStringStream, so lock the strings in ExternalStringStream.

Bug: chromium:877044
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: I241caaf64e109b33e2f9982573e11c514410509c
Reviewed-on: https://chromium-review.googlesource.com/1194003
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55613}
This commit is contained in:
Benoît Lizé 2018-09-04 15:13:02 +02:00 committed by Commit Bot
parent 13afaecc93
commit a1da383fb3
6 changed files with 179 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
include/v8.h
View File

@ -153,6 +153,7 @@ template<typename T> class CustomArguments;
class PropertyCallbackArguments;
class FunctionCallbackArguments;
class GlobalHandles;
class ScopedExternalStringLock;
namespace wasm {
class NativeModule;
@ -2746,7 +2747,26 @@ class V8_EXPORT String : public Name {
public:
virtual ~ExternalStringResourceBase() {}
virtual bool IsCompressible() const { return false; }
V8_DEPRECATE_SOON("Use IsCacheable().",
virtual bool IsCompressible() const) {
return false;
}
/**
* If a string is cacheable, the value returned by
* ExternalStringResource::data() may be cached, otherwise it is not
* expected to be stable beyond the current top-level task.
*/
virtual bool IsCacheable() const {
#if __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
#endif
return !IsCompressible();
#if __clang__
#pragma clang diagnostic pop
#endif
}
protected:
ExternalStringResourceBase() {}
@ -2759,6 +2779,24 @@ class V8_EXPORT String : public Name {
*/
virtual void Dispose() { delete this; }
/**
* For a non-cacheable string, the value returned by
* |ExternalStringResource::data()| has to be stable between |Lock()| and
* |Unlock()|, that is the string must behave as is |IsCacheable()| returned
* true.
*
* These two functions must be thread-safe, and can be called from anywhere.
* They also must handle lock depth, in the sense that each can be called
* several times, from different threads, and unlocking should only happen
* when the balance of Lock() and Unlock() calls is 0.
*/
virtual void Lock() const {}
/**
* Unlocks the string.
*/
virtual void Unlock() const {}
// Disallow copying and assigning.
ExternalStringResourceBase(const ExternalStringResourceBase&) = delete;
void operator=(const ExternalStringResourceBase&) = delete;
@ -2766,6 +2804,7 @@ class V8_EXPORT String : public Name {
private:
friend class internal::Heap;
friend class v8::String;
friend class internal::ScopedExternalStringLock;
};
/**

4
src/heap/factory.cc
View File

@ -1243,7 +1243,7 @@ MaybeHandle<String> Factory::NewExternalStringFromOneByte(
if (length == 0) return empty_string();
Handle<Map> map;
if (resource->IsCompressible()) {
if (!resource->IsCacheable()) {
map = uncached_external_one_byte_string_map();
} else {
map = external_one_byte_string_map();
@ -1273,7 +1273,7 @@ MaybeHandle<String> Factory::NewExternalStringFromTwoByte(
length <= kOneByteCheckLengthLimit &&
String::IsOneByte(resource->data(), static_cast<int>(length));
Handle<Map> map;
if (resource->IsCompressible()) {
if (!resource->IsCacheable()) {
map = is_one_byte ? uncached_external_string_with_one_byte_data_map()
: uncached_external_string_map();
} else {

4
src/objects.cc
View File

@ -2613,7 +2613,7 @@ bool String::MakeExternal(v8::String::ExternalStringResource* resource) {
// Externalizing twice leaks the external resource, so it's
// prohibited by the API.
DCHECK(this->SupportsExternalization());
DCHECK(!resource->IsCompressible());
DCHECK(resource->IsCacheable());
#ifdef ENABLE_SLOW_DCHECKS
if (FLAG_enable_slow_asserts) {
// Assert that the resource and the string are equivalent.
@ -2695,7 +2695,7 @@ bool String::MakeExternal(v8::String::ExternalOneByteStringResource* resource) {
// Externalizing twice leaks the external resource, so it's
// prohibited by the API.
DCHECK(this->SupportsExternalization());
DCHECK(!resource->IsCompressible());
DCHECK(resource->IsCacheable());
#ifdef ENABLE_SLOW_DCHECKS
if (FLAG_enable_slow_asserts) {
// Assert that the resource and the string are equivalent.

82
src/parsing/scanner-character-streams.cc
View File

@ -4,6 +4,9 @@
#include "src/parsing/scanner-character-streams.h"
#include <memory>
#include <vector>
#include "include/v8.h"
#include "src/counters.h"
#include "src/globals.h"
@ -15,21 +18,50 @@
namespace v8 {
namespace internal {
class ScopedExternalStringLock {
public:
explicit ScopedExternalStringLock(ExternalString* string) {
DCHECK(string);
if (string->IsExternalOneByteString()) {
resource_ = ExternalOneByteString::cast(string)->resource();
} else {
DCHECK(string->IsExternalTwoByteString());
resource_ = ExternalTwoByteString::cast(string)->resource();
}
DCHECK(resource_);
resource_->Lock();
}
// Copying a lock increases the locking depth.
ScopedExternalStringLock(const ScopedExternalStringLock& other)
: resource_(other.resource_) {
resource_->Lock();
}
~ScopedExternalStringLock() { resource_->Unlock(); }
private:
// Not nullptr.
const v8::String::ExternalStringResourceBase* resource_;
};
namespace {
const unibrow::uchar kUtf8Bom = 0xFEFF;
} // namespace
template <typename Char>
struct HeapStringType;
struct CharTraits;
template <>
struct HeapStringType<uint8_t> {
struct CharTraits<uint8_t> {
typedef SeqOneByteString String;
typedef ExternalOneByteString ExternalString;
};
template <>
struct HeapStringType<uint16_t> {
struct CharTraits<uint16_t> {
typedef SeqTwoByteString String;
typedef ExternalTwoByteString ExternalString;
};
template <typename Char>
@ -47,7 +79,7 @@ struct Range {
template <typename Char>
class OnHeapStream {
public:
typedef typename HeapStringType<Char>::String String;
typedef typename CharTraits<Char>::String String;
OnHeapStream(Handle<String> string, size_t start_offset, size_t end)
: string_(string), start_offset_(start_offset), length_(end) {}
@ -74,12 +106,17 @@ class OnHeapStream {
// ExternalTwoByteString.
template <typename Char>
class ExternalStringStream {
typedef typename CharTraits<Char>::ExternalString ExternalString;
public:
ExternalStringStream(const Char* data, size_t end)
: data_(data), length_(end) {}
ExternalStringStream(ExternalString* string, size_t start_offset,
size_t length)
: lock_(string),
data_(string->GetChars() + start_offset),
length_(length) {}
ExternalStringStream(const ExternalStringStream& other)
: data_(other.data_), length_(other.length_) {}
: lock_(other.lock_), data_(other.data_), length_(other.length_) {}
Range<Char> GetDataAt(size_t pos) {
return {&data_[Min(length_, pos)], &data_[length_]};
@ -88,6 +125,25 @@ class ExternalStringStream {
static const bool kCanBeCloned = true;
static const bool kCanAccessHeap = false;
private:
ScopedExternalStringLock lock_;
const Char* const data_;
const size_t length_;
};
// A Char stream backed by a C array. Testing only.
template <typename Char>
class TestingStream {
public:
TestingStream(const Char* data, size_t length)
: data_(data), length_(length) {}
Range<Char> GetDataAt(size_t pos) {
return {&data_[Min(length_, pos)], &data_[length_]};
}
static const bool kCanBeCloned = true;
static const bool kCanAccessHeap = false;
private:
const Char* const data_;
const size_t length_;
@ -754,14 +810,12 @@ Utf16CharacterStream* ScannerStream::For(Isolate* isolate, Handle<String> data,
}
if (data->IsExternalOneByteString()) {
return new BufferedCharacterStream<ExternalStringStream>(
static_cast<size_t>(start_pos),
ExternalOneByteString::cast(*data)->GetChars() + start_offset,
static_cast<size_t>(end_pos));
static_cast<size_t>(start_pos), ExternalOneByteString::cast(*data),
start_offset, static_cast<size_t>(end_pos));
} else if (data->IsExternalTwoByteString()) {
return new UnbufferedCharacterStream<ExternalStringStream>(
static_cast<size_t>(start_pos),
ExternalTwoByteString::cast(*data)->GetChars() + start_offset,
static_cast<size_t>(end_pos));
static_cast<size_t>(start_pos), ExternalTwoByteString::cast(*data),
start_offset, static_cast<size_t>(end_pos));
} else if (data->IsSeqOneByteString()) {
return new BufferedCharacterStream<OnHeapStream>(
static_cast<size_t>(start_pos), Handle<SeqOneByteString>::cast(data),
@ -784,7 +838,7 @@ std::unique_ptr<Utf16CharacterStream> ScannerStream::ForTesting(
std::unique_ptr<Utf16CharacterStream> ScannerStream::ForTesting(
const char* data, size_t length) {
return std::unique_ptr<Utf16CharacterStream>(
new BufferedCharacterStream<ExternalStringStream>(
new BufferedCharacterStream<TestingStream>(
static_cast<size_t>(0), reinterpret_cast<const uint8_t*>(data),
static_cast<size_t>(length)));
}

77
test/cctest/parsing/test-scanner-streams.cc
View File

@ -61,15 +61,43 @@ class ChunkSource : public v8::ScriptCompiler::ExternalSourceStream {
size_t current_;
};
class TestExternalResource : public v8::String::ExternalStringResource {
// Checks that Lock() / Unlock() pairs are balanced. Not thread-safe.
class LockChecker {
public:
LockChecker() : lock_depth_(0) {}
~LockChecker() { CHECK_EQ(0, lock_depth_); }
void Lock() const { lock_depth_++; }
void Unlock() const {
CHECK_GT(lock_depth_, 0);
lock_depth_--;
}
bool IsLocked() const { return lock_depth_ > 0; }
int LockDepth() const { return lock_depth_; }
protected:
mutable int lock_depth_;
};
class TestExternalResource : public v8::String::ExternalStringResource,
public LockChecker {
public:
explicit TestExternalResource(uint16_t* data, int length)
: data_(data), length_(static_cast<size_t>(length)) {}
: LockChecker(), data_(data), length_(static_cast<size_t>(length)) {}
~TestExternalResource() {}
const uint16_t* data() const override {
CHECK(IsLocked());
return data_;
}
const uint16_t* data() const { return data_; }
size_t length() const { return length_; }
size_t length() const override { return length_; }
bool IsCacheable() const override { return false; }
void Lock() const override { LockChecker::Lock(); }
void Unlock() const override { LockChecker::Unlock(); }
private:
uint16_t* data_;
@ -77,13 +105,21 @@ class TestExternalResource : public v8::String::ExternalStringResource {
};
class TestExternalOneByteResource
: public v8::String::ExternalOneByteStringResource {
: public v8::String::ExternalOneByteStringResource,
public LockChecker {
public:
TestExternalOneByteResource(const char* data, size_t length)
: data_(data), length_(length) {}
const char* data() const { return data_; }
size_t length() const { return length_; }
const char* data() const override {
CHECK(IsLocked());
return data_;
}
size_t length() const override { return length_; }
bool IsCacheable() const override { return false; }
void Lock() const override { LockChecker::Lock(); }
void Unlock() const override { LockChecker::Unlock(); }
private:
const char* data_;
@ -101,6 +137,17 @@ const char unicode_utf8[] =
const uint16_t unicode_ucs2[] = {97, 98, 99, 228, 10784, 55357,
56489, 100, 101, 102, 0};
i::Handle<i::String> NewExternalTwoByteStringFromResource(
i::Isolate* isolate, TestExternalResource* resource) {
i::Factory* factory = isolate->factory();
// String creation accesses the resource.
resource->Lock();
i::Handle<i::String> uc16_string(
factory->NewExternalStringFromTwoByte(resource).ToHandleChecked());
resource->Unlock();
return uc16_string;
}
} // anonymous namespace
TEST(Utf8StreamAsciiOnly) {
@ -413,7 +460,7 @@ void TestCloneCharacterStream(const char* reference,
CHECK_EQU(reference[i], stream->Advance());
}
// Test advancing oriignal stream didn't affect the clone.
// Test advancing original stream didn't affect the clone.
TestCharacterStream(reference, clone.get(), length, 0, length);
// Test advancing clone didn't affect original stream.
@ -439,7 +486,7 @@ void TestCharacterStreams(const char* one_byte_source, unsigned length,
}
TestExternalResource resource(uc16_buffer.get(), length);
i::Handle<i::String> uc16_string(
factory->NewExternalStringFromTwoByte(&resource).ToHandleChecked());
NewExternalTwoByteStringFromResource(isolate, &resource));
std::unique_ptr<i::Utf16CharacterStream> uc16_stream(
i::ScannerStream::For(isolate, uc16_string, start, end));
TestCharacterStream(one_byte_source, uc16_stream.get(), length, start, end);
@ -739,9 +786,17 @@ TEST(CloneCharacterStreams) {
}
TestExternalResource resource(uc16_buffer.get(), length);
i::Handle<i::String> uc16_string(
factory->NewExternalStringFromTwoByte(&resource).ToHandleChecked());
NewExternalTwoByteStringFromResource(isolate, &resource));
std::unique_ptr<i::Utf16CharacterStream> uc16_stream(
i::ScannerStream::For(isolate, uc16_string, 0, length));
CHECK(resource.IsLocked());
CHECK_EQ(1, resource.LockDepth());
std::unique_ptr<i::Utf16CharacterStream> cloned = uc16_stream->Clone();
CHECK_EQ(2, resource.LockDepth());
uc16_stream = std::move(cloned);
CHECK_EQ(1, resource.LockDepth());
TestCloneCharacterStream(one_byte_source, uc16_stream.get(), length);
// This avoids the GC from trying to free a stack allocated resource.

2
test/cctest/test-regexp.cc
View File

@ -1968,7 +1968,7 @@ class UncachedExternalString
public:
const char* data() const override { return "abcdefghijklmnopqrstuvwxyz"; }
size_t length() const override { return 26; }
bool IsCompressible() const override { return true; }
bool IsCacheable() const override { return false; }
};
TEST(UncachedExternalString) {