Revert "[codegen] Assert that deserialized SFIs have correct origins"

This reverts commit 2660997331.

Reason for revert: Breaks code_serializer tests - https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux%20-%20debug/36427/overview

Original change's description:
> [codegen] Assert that deserialized SFIs have correct origins
>
> 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}

Bug: v8:10284
Change-Id: I234fcf031001819b05dbcdd421f235f71e9805b2
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3114143
Auto-Submit: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#76456}
This commit is contained in:
Maya Lekova 2021-08-24 11:45:56 +00:00 committed by V8 LUCI CQ
parent 5a6c7dee4e
commit 438989d6c1
11 changed files with 81 additions and 173 deletions

View File

@ -105,6 +105,41 @@ 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
@ -127,7 +162,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 (function_info->HasMatchingOrigin(isolate(), script_details)) {
if (HasOrigin(isolate(), function_info, script_details)) {
result = scope.CloseAndEscape(function_info);
}
}
@ -138,7 +173,9 @@ MaybeHandle<SharedFunctionInfo> CompilationCacheScript::Lookup(
// handle created in the caller's handle scope.
Handle<SharedFunctionInfo> function_info;
if (result.ToHandle(&function_info)) {
DCHECK(function_info->HasMatchingOrigin(isolate(), script_details));
// Since HasOrigin can allocate, we need to protect the SharedFunctionInfo
// with handles during the call.
DCHECK(HasOrigin(isolate(), function_info, script_details));
isolate()->counters()->compilation_cache_hits()->Increment();
LOG(isolate(), CompilationCacheEvent("hit", "script", *function_info));
} else {

View File

@ -1706,10 +1706,10 @@ void BackgroundDeserializeTask::Run() {
MaybeHandle<SharedFunctionInfo> BackgroundDeserializeTask::Finish(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details) {
ScriptOriginOptions origin_options) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), &cached_data_, source,
script_details);
origin_options);
}
// ----------------------------------------------------------------------------
@ -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);
maybe_result = deserialize_task->Finish(isolate, source,
script_details.origin_options);
} else {
maybe_result = CodeSerializer::Deserialize(isolate, cached_data, source,
script_details);
maybe_result = CodeSerializer::Deserialize(
isolate, cached_data, source, script_details.origin_options);
}
bool consuming_code_cache_succeeded = false;
@ -2906,11 +2906,6 @@ 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()) {
@ -3030,7 +3025,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);
script_details.origin_options);
if (maybe_result.is_null()) {
// Deserializer failed. Fall through to compile.
compile_timer.set_consuming_code_cache_failed();

View File

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

View File

@ -15,8 +15,6 @@ 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())

View File

@ -8,7 +8,6 @@
#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"
@ -714,62 +713,5 @@ 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

View File

@ -38,7 +38,6 @@ class DebugInfo;
class IsCompiledScope;
template <typename>
class Signature;
struct ScriptDetails;
class WasmCapiFunctionData;
class WasmExportedFunctionData;
class WasmJSFunctionData;
@ -208,9 +207,6 @@ 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;
@ -244,9 +240,8 @@ 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:
@ -260,8 +255,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;
@ -273,9 +268,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;

View File

@ -10,7 +10,6 @@
#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"
@ -75,10 +74,8 @@ ScriptCompiler::CachedData* CodeSerializer::Serialize(
// Serialize code object.
Handle<String> source(String::cast(script->source()), isolate);
HandleScope scope(isolate);
ScriptDetails script_details(handle(script->name(), isolate),
script->origin_options());
CodeSerializer cs(isolate,
SerializedCodeData::SourceHash(source, script_details));
CodeSerializer cs(isolate, SerializedCodeData::SourceHash(
source, script->origin_options()));
DisallowGarbageCollection no_gc;
cs.reference_map()->AddAttachedReference(*source);
AlignedCachedData* cached_data = cs.SerializeSharedFunctionInfo(info);
@ -294,12 +291,12 @@ class StressOffThreadDeserializeThread final : public base::Thread {
CodeSerializer::StartDeserializeOffThread(&local_isolate, cached_data_);
}
MaybeHandle<SharedFunctionInfo> Finalize(
Isolate* isolate, Handle<String> source,
const ScriptDetails& script_details) {
MaybeHandle<SharedFunctionInfo> Finalize(Isolate* isolate,
Handle<String> source,
ScriptOriginOptions origin_options) {
return CodeSerializer::FinishOffThreadDeserialize(
isolate, std::move(off_thread_data_), cached_data_, source,
script_details);
origin_options);
}
private:
@ -372,72 +369,19 @@ 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,
const ScriptDetails& script_details) {
ScriptOriginOptions origin_options) {
if (FLAG_stress_background_compile) {
StressOffThreadDeserializeThread thread(isolate, cached_data);
CHECK(thread.Start());
thread.Join();
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 thread.Finalize(isolate, source, origin_options);
// TODO(leszeks): Compare off-thread deserialized data to on-thread.
}
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();
@ -446,7 +390,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::DeserializeMain(
SerializedCodeData::SanityCheckResult sanity_check_result =
SerializedCodeData::CHECK_SUCCESS;
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
cached_data, SerializedCodeData::SourceHash(source, script_details),
cached_data, SerializedCodeData::SourceHash(source, origin_options),
&sanity_check_result);
if (sanity_check_result != SerializedCodeData::CHECK_SUCCESS) {
if (FLAG_profile_deserialization) PrintF("[Cached code failed check]\n");
@ -466,6 +410,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::DeserializeMain(
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();
@ -499,6 +444,7 @@ 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();
@ -509,7 +455,7 @@ CodeSerializer::StartDeserializeOffThread(LocalIsolate* local_isolate,
MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
Isolate* isolate, OffThreadDeserializeData&& data,
AlignedCachedData* cached_data, Handle<String> source,
const ScriptDetails& script_details) {
ScriptOriginOptions origin_options) {
base::ElapsedTimer timer;
if (FLAG_profile_deserialization || FLAG_log_function_events) timer.Start();
@ -519,7 +465,7 @@ MaybeHandle<SharedFunctionInfo> CodeSerializer::FinishOffThreadDeserialize(
SerializedCodeData::SanityCheckResult sanity_check_result =
SerializedCodeData::CHECK_SUCCESS;
const SerializedCodeData scd = SerializedCodeData::FromCachedData(
cached_data, SerializedCodeData::SourceHash(source, script_details),
cached_data, SerializedCodeData::SourceHash(source, origin_options),
&sanity_check_result);
if (sanity_check_result != SerializedCodeData::CHECK_SUCCESS) {
// The only case where the deserialization result could exist despite a
@ -635,12 +581,11 @@ SerializedCodeData::SanityCheckWithoutSource() const {
}
uint32_t SerializedCodeData::SourceHash(Handle<String> source,
const ScriptDetails& script_details) {
ScriptOriginOptions origin_options) {
const uint32_t source_length = source->length();
static constexpr uint32_t kModuleFlagMask = (1 << 31);
const uint32_t is_module =
script_details.origin_options.IsModule() ? kModuleFlagMask : 0;
const uint32_t is_module = origin_options.IsModule() ? kModuleFlagMask : 0;
DCHECK_EQ(0, source_length & kModuleFlagMask);
return source_length | is_module;

View File

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

View File

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

View File

@ -707,7 +707,6 @@ 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 =

View File

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