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

[codegen] Assert that deserialized SFIs have correct origins

Browse Source 
Re-use the same check we already have in place for the
compilation cache for when we use CodeSerializer::Deserialize.

- Move HasOrigin to SharedFunctionInfo::HasMatchingOrigin
- HasMatchingOrigin no longer allocates
- Pass ScriptDetails in more places

Bug: v8:10284
Change-Id: I6e074bd1e7db9a35fdf7123d04a65841d9813e02
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3090968
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76451}
This commit is contained in:
Camillo Bruni 2021-08-24 11:51:51 +02:00 committed by V8 LUCI CQ
parent 1c4ae62dbc
commit 2660997331
11 changed files with 173 additions and 81 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
src/codegen/compilation-cache.cc
View File

@ -105,41 +105,6 @@ void CompilationSubCache::Remove(Handle<SharedFunctionInfo> function_info) {
CompilationCacheScript::CompilationCacheScript(Isolate* isolate)
: CompilationSubCache(isolate, 1) {}
namespace {
// We only re-use a cached function for some script source code if the
// script originates from the same place. This is to avoid issues
// when reporting errors, etc.
bool HasOrigin(Isolate* isolate, Handle<SharedFunctionInfo> function_info,
const ScriptDetails& script_details) {
Handle<Script> script =
Handle<Script>(Script::cast(function_info->script()), isolate);
// If the script name isn't set, the boilerplate script should have
// an undefined name to have the same origin.
Handle<Object> name;
if (!script_details.name_obj.ToHandle(&name)) {
return script->name().IsUndefined(isolate);
}
// Do the fast bailout checks first.
if (script_details.line_offset != script->line_offset()) return false;
if (script_details.column_offset != script->column_offset()) return false;
// Check that both names are strings. If not, no match.
if (!name->IsString() || !script->name().IsString()) return false;
// Are the origin_options same?
if (script_details.origin_options.Flags() !=
script->origin_options().Flags()) {
return false;
}
// Compare the two name strings for equality.
if (!String::Equals(isolate, Handle<String>::cast(name),
Handle<String>(String::cast(script->name()), isolate))) {
return false;
}
// TODO(10284): Enable host-defined options check again
return true;
}
} // namespace
// TODO(245): Need to allow identical code from different contexts to
// be cached in the same script generation. Currently the first use
// will be cached, but subsequent code from different source / line
@ -162,7 +127,7 @@ MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
if (probe.ToHandle(&function_info)) {
// Break when we've found a suitable shared function info that
// matches the origin.
if (HasOrigin(isolate(), function_info, script_details)) {
if (function_info->HasMatchingOrigin(isolate(), script_details)) {
result = scope.CloseAndEscape(function_info);
}
}
@ -173,9 +138,7 @@ MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
// handle created in the caller's handle scope.
Handle<SharedFunctionInfo> function_info;
if (result.ToHandle(&function_info)) {
// Since HasOrigin can allocate, we need to protect the SharedFunctionInfo
// with handles during the call.
DCHECK(HasOrigin(isolate(), function_info, script_details));
DCHECK(function_info->HasMatchingOrigin(isolate(), script_details));
isolate()->counters()->compilation_cache_hits()->Increment();
LOG(isolate(), CompilationCacheEvent("hit", "script", *function_info));
} else {

19
src/codegen/compiler.cc
View File

@ -1706,10 +1706,10 @@ void BackgroundDeserializeTask::Run() {
MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
Isolate* isolate, Handle<String> source,
ScriptOriginOptions origin_options) {
const ScriptDetails& script_details) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), &cached_data_, source,
origin_options);
script_details);
}
// ----------------------------------------------------------------------------
@ -2884,11 +2884,11 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
"V8.CompileDeserialize");
if (deserialize_task) {
// If there's a cache consume task, finish it.
maybe_result = deserialize_task->Finish(isolate, source,
script_details.origin_options);
maybe_result =
deserialize_task->Finish(isolate, source, script_details);
} else {
maybe_result = CodeSerializer::Deserialize(
isolate, cached_data, source, script_details.origin_options);
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
script_details);
}
bool consuming_code_cache_succeeded = false;
@ -2906,6 +2906,11 @@ MaybeHandle<SharedFunctionInfo> GetSharedFunctionInfoForScriptImpl(
compile_timer.set_consuming_code_cache_failed();
}
}
if (!maybe_result.is_null()) {
// Assert we end up with compatible SFIs.
DCHECK(maybe_result.ToHandleChecked()->HasMatchingOrigin(isolate,
script_details));
}
}
if (maybe_result.is_null()) {
@ -3025,7 +3030,7 @@ MaybeHandle<JSFunction> Compiler::GetWrappedFunction(
TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("v8.compile"),
"V8.CompileDeserialize");
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
script_details.origin_options);
script_details);
if (maybe_result.is_null()) {
// Deserializer failed. Fall through to compile.
compile_timer.set_consuming_code_cache_failed();

2
src/codegen/compiler.h
View File

@ -600,7 +600,7 @@ class V8_EXPORT_PRIVATE BackgroundDeserializeTask {
MaybeHandle<SharedFunctionInfo> Finish(Isolate* isolate,
Handle<String> source,
ScriptOriginOptions origin_options);
const ScriptDetails& script_details);
bool rejected() const { return cached_data_.rejected(); }

2
src/codegen/script-details.h
View File

@ -15,6 +15,8 @@ namespace internal {
struct ScriptDetails {
ScriptDetails()
: line_offset(0), column_offset(0), repl_mode(REPLMode::kNo) {}
explicit ScriptDetails(ScriptOriginOptions origin_options)
: ScriptDetails(Handle<Object>(), origin_options) {}
explicit ScriptDetails(
Handle<Object> script_name,
ScriptOriginOptions origin_options = v8::ScriptOriginOptions())

58
src/objects/shared-function-info.cc
View File

@ -8,6 +8,7 @@
#include "src/ast/scopes.h"
#include "src/codegen/compilation-cache.h"
#include "src/codegen/compiler.h"
#include "src/codegen/script-details.h"
#include "src/common/globals.h"
#include "src/diagnostics/code-tracer.h"
#include "src/objects/shared-function-info-inl.h"
@ -713,5 +714,62 @@ void SharedFunctionInfo::UninstallDebugBytecode(SharedFunctionInfo shared,
kReleaseStore);
}
// static
// We only re-use a cached function for some script source code if the
// script originates from the same place. This is to avoid issues
// when reporting errors, etc.
bool SharedFunctionInfo::HasMatchingOrigin(
Isolate* isolate, const ScriptDetails& script_details) const {
DisallowGarbageCollection no_gc;
Script script = Script::cast(this->script());
// If the script name isn't set, the boilerplate script should have
// an undefined name to have the same origin.
Object name_obj;
{
Handle<Object> tmp_handle;
if (script_details.name_obj.ToHandle(&tmp_handle)) {
name_obj = *tmp_handle;
} else {
return script.name().IsUndefined(isolate);
}
}
// Do the fast bailout checks first.
if (script_details.line_offset != script.line_offset()) return false;
if (script_details.column_offset != script.column_offset()) return false;
// Check that both names are strings. If not, no match.
if (!name_obj.IsString() || !script.name().IsString()) return false;
// Are the origin_options same?
if (script_details.origin_options.Flags() !=
script.origin_options().Flags()) {
return false;
}
if (!String::cast(name_obj).Equals(String::cast(script.name()))) return false;
// TODO(10284): Enable strict checks again.
// FixedArray host_defined_options;
// {
// Handle<FixedArray> tmp_handle;
// if (script_details.host_defined_options.ToHandle(&tmp_handle)) {
// host_defined_options = *tmp_handle;
// } else {
// host_defined_options = ReadOnlyRoots(isolate).empty_fixed_array();
// }
// }
// FixedArray script_options = script.host_defined_options();
// if (host_defined_options == script_options) return true;
// int length = host_defined_options.length();
// if (length != script_options.length()) return false;
// for (int i = 0; i < length; i++) {
// // host-defined options is a v8::PrimitiveArray.
// DCHECK(host_defined_options.get(i).IsPrimitive());
// DCHECK(script_options.get(i).IsPrimitive());
// if (!host_defined_options.get(i).StrictEquals(script_options.get(i))) {
// return false;
// }
// }
return true;
}
} // namespace internal
} // namespace v8

19
src/objects/shared-function-info.h
View File

@ -38,6 +38,7 @@ class DebugInfo;
class IsCompiledScope;
template <typename>
class Signature;
struct ScriptDetails;
class WasmCapiFunctionData;
class WasmExportedFunctionData;
class WasmJSFunctionData;
@ -207,6 +208,9 @@ class SharedFunctionInfo
int function_literal_id,
bool reset_preparsed_scope_data = true);
bool HasMatchingOrigin(Isolate* isolate,
const ScriptDetails& script_details) const;
// Layout description of the optimized code map.
static const int kEntriesStart = 0;
static const int kContextOffset = 0;
@ -240,8 +244,9 @@ class SharedFunctionInfo
// Updates the scope info if available.
V8_EXPORT_PRIVATE void SetPosition(int start_position, int end_position);
// [outer scope info | feedback metadata] Shared storage for outer scope info
// (on uncompiled functions) and feedback metadata (on compiled functions).
// [outer scope info | feedback metadata] Shared storage for outer scope
// info (on uncompiled functions) and feedback metadata (on compiled
// functions).
DECL_ACCESSORS(raw_outer_scope_info_or_feedback_metadata, HeapObject)
DECL_ACQUIRE_GETTER(raw_outer_scope_info_or_feedback_metadata, HeapObject)
private:
@ -255,8 +260,8 @@ class SharedFunctionInfo
inline bool HasOuterScopeInfo() const;
inline ScopeInfo GetOuterScopeInfo() const;
// [feedback metadata] Metadata template for feedback vectors of instances of
// this function.
// [feedback metadata] Metadata template for feedback vectors of instances
// of this function.
inline bool HasFeedbackMetadata() const;
inline bool HasFeedbackMetadata(AcquireLoadTag tag) const;
inline FeedbackMetadata feedback_metadata() const;
@ -268,9 +273,9 @@ class SharedFunctionInfo
// for some period of time, use IsCompiledScope instead.
inline bool is_compiled() const;
// Returns an IsCompiledScope which reports whether the function is compiled,
// and if compiled, will avoid the function becoming uncompiled while it is
// held.
// Returns an IsCompiledScope which reports whether the function is
// compiled, and if compiled, will avoid the function becoming uncompiled
// while it is held.
template <typename IsolateT>
inline IsCompiledScope is_compiled_scope(IsolateT* isolate) const;

87
src/snapshot/code-serializer.cc
View File

@ -10,6 +10,7 @@
#include "src/base/platform/elapsed-timer.h"
#include "src/base/platform/platform.h"
#include "src/codegen/macro-assembler.h"
#include "src/codegen/script-details.h"
#include "src/common/globals.h"
#include "src/debug/debug.h"
#include "src/handles/maybe-handles.h"
@ -74,8 +75,10 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(
// Serialize code object.
Handle<String> source(String::cast(script->source()), isolate);
HandleScope scope(isolate);
CodeSerializer cs(isolate, SerializedCodeData::SourceHash(
source, script->origin_options()));
ScriptDetails script_details(handle(script->name(), isolate),
script->origin_options());
CodeSerializer cs(isolate,
SerializedCodeData::SourceHash(source, script_details));
DisallowGarbageCollection no_gc;
cs.reference_map()->AddAttachedReference(*source);
AlignedCachedData* cached_data = cs.SerializeSharedFunctionInfo(info);
@ -291,12 +294,12 @@ class StressOffThreadDeserializeThread final : public base::Thread {
CodeSerializer::StartDeserializeOffThread(&local_isolate, cached_data_);
}
MaybeHandle<SharedFunctionInfo> Finalize(Isolate* isolate,
Handle<String> source,
ScriptOriginOptions origin_options) {
MaybeHandle<SharedFunctionInfo> Finalize(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), cached_data_, source,
origin_options);
script_details);
}
private:
@ -369,19 +372,72 @@ void FinalizeDeserialization(Isolate* isolate,
}
}
void CompareSFIs(Handle<SharedFunctionInfo> main_thread,
Handle<SharedFunctionInfo> off_thread) {
DisallowGarbageCollection no_gc;
DCHECK_EQ(main_thread->flags(kRelaxedLoad), off_thread->flags(kRelaxedLoad));
DCHECK_EQ(main_thread->flags2(), off_thread->flags2());
DCHECK_EQ(main_thread->raw_function_token_offset(),
off_thread->raw_function_token_offset());
DCHECK_EQ(main_thread->internal_formal_parameter_count(),
off_thread->internal_formal_parameter_count());
DCHECK(main_thread->Name().Equals(off_thread->Name()));
if (!main_thread->script().IsScript()) {
DCHECK(!off_thread->script().IsScript());
return;
}
Script main_thread_script = Script::cast(main_thread->script());
#ifdef DEBUG
Script off_thread_script = Script::cast(off_thread->script());
#endif
DCHECK_EQ(main_thread_script.flags(), off_thread_script.flags());
DCHECK_EQ(main_thread_script.script_type(), off_thread_script.script_type());
if (main_thread_script.source().IsString()) {
DCHECK(String::cast(main_thread_script.source())
.Equals(String::cast(off_thread_script.source())));
} else {
DCHECK(!off_thread_script.source().IsString());
}
if (main_thread_script.source_url().IsString()) {
DCHECK(String::cast(main_thread_script.source())
.Equals(String::cast(off_thread_script.source())));
} else {
DCHECK(!off_thread_script.source_url().IsString());
}
if (main_thread_script.name().IsString()) {
DCHECK(String::cast(main_thread_script.name())
.Equals(String::cast(off_thread_script.name())));
} else {
DCHECK(!off_thread_script.name().IsString());
}
}
} // namespace
MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options) {
const ScriptDetails& script_details) {
if (FLAG_stress_background_compile) {
StressOffThreadDeserializeThread thread(isolate, cached_data);
CHECK(thread.Start());
thread.Join();
return thread.Finalize(isolate, source, origin_options);
// TODO(leszeks): Compare off-thread deserialized data to on-thread.
MaybeHandle<SharedFunctionInfo> off_thread_result =
thread.Finalize(isolate, source, script_details);
MaybeHandle<SharedFunctionInfo> main_thread_result =
DeserializeMain(isolate, cached_data, source, script_details);
DCHECK_EQ(off_thread_result.is_null(), main_thread_result.is_null());
if (!main_thread_result.is_null()) {
CompareSFIs(main_thread_result.ToHandleChecked(),
off_thread_result.ToHandleChecked());
}
return off_thread_result;
}
return DeserializeMain(isolate, cached_data, source, script_details);
}
MaybeHandle<SharedFunctionInfo> CodeSerializer::DeserializeMain(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
const ScriptDetails& script_details) {
base::ElapsedTimer timer;
if (FLAG_profile_deserialization || FLAG_log_function_events) timer.Start();
@ -390,7 +446,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
SerializedCodeData::SanityCheckResult sanity_check_result =
SerializedCodeData::CHECK_SUCCESS;
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
cached_data, SerializedCodeData::SourceHash(source, origin_options),
cached_data, SerializedCodeData::SourceHash(source, script_details),
&sanity_check_result);
if (sanity_check_result != SerializedCodeData::CHECK_SUCCESS) {
if (FLAG_profile_deserialization) PrintF("[Cached code failed check]\n");
@ -410,7 +466,6 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::Deserialize(
if (FLAG_profile_deserialization) PrintF("[Deserializing failed]\n");
return MaybeHandle<SharedFunctionInfo>();
}
if (FLAG_profile_deserialization) {
double ms = timer.Elapsed().InMillisecondsF();
int length = cached_data->length();
@ -444,7 +499,6 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
MaybeHandle<SharedFunctionInfo> local_maybe_result =
OffThreadObjectDeserializer::DeserializeSharedFunctionInfo(
local_isolate, &scd, &result.scripts);
result.maybe_result =
local_isolate->heap()->NewPersistentMaybeHandle(local_maybe_result);
result.persistent_handles = local_isolate->heap()->DetachPersistentHandles();
@ -455,7 +509,7 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
Isolate* isolate, OffThreadDeserializeData&& data,
AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options) {
const ScriptDetails& script_details) {
base::ElapsedTimer timer;
if (FLAG_profile_deserialization || FLAG_log_function_events) timer.Start();
@ -465,7 +519,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
SerializedCodeData::SanityCheckResult sanity_check_result =
SerializedCodeData::CHECK_SUCCESS;
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
cached_data, SerializedCodeData::SourceHash(source, origin_options),
cached_data, SerializedCodeData::SourceHash(source, script_details),
&sanity_check_result);
if (sanity_check_result != SerializedCodeData::CHECK_SUCCESS) {
// The only case where the deserialization result could exist despite a
@ -581,11 +635,12 @@ SerializedCodeData::SanityCheckWithoutSource() const {
}
uint32_t SerializedCodeData::SourceHash(Handle<String> source,
ScriptOriginOptions origin_options) {
const ScriptDetails& script_details) {
const uint32_t source_length = source->length();
static constexpr uint32_t kModuleFlagMask = (1 << 31);
const uint32_t is_module = origin_options.IsModule() ? kModuleFlagMask : 0;
const uint32_t is_module =
script_details.origin_options.IsModule() ? kModuleFlagMask : 0;
DCHECK_EQ(0, source_length & kModuleFlagMask);
return source_length | is_module;

12
src/snapshot/code-serializer.h
View File

@ -13,6 +13,7 @@ namespace v8 {
namespace internal {
class PersistentHandles;
struct ScriptDetails;
class V8_EXPORT_PRIVATE AlignedCachedData {
public:
@ -68,7 +69,7 @@ class CodeSerializer : public Serializer {
V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo> Deserialize(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
ScriptOriginOptions origin_options);
const ScriptDetails& script_details);
V8_WARN_UNUSED_RESULT static OffThreadDeserializeData
StartDeserializeOffThread(LocalIsolate* isolate,
@ -78,7 +79,7 @@ class CodeSerializer : public Serializer {
FinishOffThreadDeserialize(Isolate* isolate, OffThreadDeserializeData&& data,
AlignedCachedData* cached_data,
Handle<String> source,
ScriptOriginOptions origin_options);
const ScriptDetails& script_details);
uint32_t source_hash() const { return source_hash_; }
@ -91,9 +92,12 @@ class CodeSerializer : public Serializer {
private:
void SerializeObjectImpl(Handle<HeapObject> o) override;
bool SerializeReadOnlyObject(Handle<HeapObject> obj);
V8_WARN_UNUSED_RESULT static MaybeHandle<SharedFunctionInfo> DeserializeMain(
Isolate* isolate, AlignedCachedData* cached_data, Handle<String> source,
const ScriptDetails& script_details);
DISALLOW_GARBAGE_COLLECTION(no_gc_)
uint32_t source_hash_;
};
@ -145,7 +149,7 @@ class SerializedCodeData : public SerializedData {
base::Vector<const byte> Payload() const;
static uint32_t SourceHash(Handle<String> source,
ScriptOriginOptions origin_options);
const ScriptDetails& script_details);
private:
explicit SerializedCodeData(AlignedCachedData* data);

9
test/cctest/heap/test-heap.cc
View File

@ -1490,8 +1490,7 @@ TEST(CompilationCacheCachingBehavior) {
// The script should be in the cache now.
{
v8::HandleScope scope(CcTest::isolate());
ScriptDetails script_details(Handle<Object>(),
v8::ScriptOriginOptions(true, false));
ScriptDetails script_details(v8::ScriptOriginOptions(true, false));
MaybeHandle<SharedFunctionInfo> cached_script =
compilation_cache->LookupScript(source, script_details, language_mode);
CHECK(!cached_script.is_null());
@ -1501,8 +1500,7 @@ TEST(CompilationCacheCachingBehavior) {
{
CcTest::CollectAllGarbage();
v8::HandleScope scope(CcTest::isolate());
ScriptDetails script_details(Handle<Object>(),
v8::ScriptOriginOptions(true, false));
ScriptDetails script_details(v8::ScriptOriginOptions(true, false));
MaybeHandle<SharedFunctionInfo> cached_script =
compilation_cache->LookupScript(source, script_details, language_mode);
CHECK(!cached_script.is_null());
@ -1521,8 +1519,7 @@ TEST(CompilationCacheCachingBehavior) {
{
v8::HandleScope scope(CcTest::isolate());
// Ensure code aging cleared the entry from the cache.
ScriptDetails script_details(Handle<Object>(),
v8::ScriptOriginOptions(true, false));
ScriptDetails script_details(v8::ScriptOriginOptions(true, false));
MaybeHandle<SharedFunctionInfo> cached_script =
compilation_cache->LookupScript(source, script_details, language_mode);
CHECK(cached_script.is_null());

1
test/cctest/test-log.cc
View File

@ -707,6 +707,7 @@ UNINITIALIZED_TEST(ExternalCodeEventListener) {
UNINITIALIZED_TEST(ExternalCodeEventListenerInnerFunctions) {
i::FLAG_log = false;
i::FLAG_prof = false;
i::FLAG_stress_background_compile = false;
v8::ScriptCompiler::CachedData* cache;
static const char* source_cstring =

4
test/cctest/test-serialize.cc
View File

@ -4129,7 +4129,9 @@ TEST(WeakArraySerializationInCodeCache) {
.ToHandleChecked();
AlignedCachedData* cache = nullptr;
ScriptDetails script_details(src);
// TODO(leszeks): Fix off-thread deserialization.
ScriptDetails script_details(
isolate->factory()->NewStringFromAsciiChecked(source));
CompileScriptAndProduceCache(isolate, src, script_details, &cache,
v8::ScriptCompiler::kNoCompileOptions);