Commit 48ed752a authored by Camillo Bruni's avatar Camillo Bruni Committed by V8 LUCI CQ

Revert "[codegen] Disable host-defined options checks in cache"

This reverts commit 810d34df.

Reason for revert: The stricter host checks prevent
certain security issues. We will have to live with regressions
until we have a more flexible caching solution in place.

Original change's description:
> [codegen] Disable host-defined options checks in cache
>
> We see too many regressions for now in M94 (~10% more misses in
> some cases).
>
> This CL reverts the logic to the state before landing
> https://crrev.com/c/3069152 without having to revert the several
> refactoring CLs that landed on top of it.
>
> Bug: v8:10284, chromium:1238312, chromium:1237242
> Change-Id: I57e66b9e0d58c36d2f1563b07720e3729c88ec94
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3103006
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Reviewed-by: Jakob Gruber <jgruber@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#76362}

Bug: v8:10284, chromium:1238312, chromium:1237242
Change-Id: I4c662dd0ac16a4406f06fb2a62b9e4e65fa428ce
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3114057
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: 's avatarJakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76504}
parent f4fb979b
...@@ -135,7 +135,24 @@ bool HasOrigin(Isolate* isolate, Handle<SharedFunctionInfo> function_info, ...@@ -135,7 +135,24 @@ bool HasOrigin(Isolate* isolate, Handle<SharedFunctionInfo> function_info,
Handle<String>(String::cast(script->name()), isolate))) { Handle<String>(String::cast(script->name()), isolate))) {
return false; return false;
} }
// TODO(10284): Enable host-defined options check again
Handle<FixedArray> host_defined_options;
if (!script_details.host_defined_options.ToHandle(&host_defined_options)) {
host_defined_options = isolate->factory()->empty_fixed_array();
}
Handle<FixedArray> script_options(script->host_defined_options(), isolate);
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; return true;
} }
} // namespace } // namespace
......
...@@ -1838,13 +1838,12 @@ TEST(CodeSerializerPromotedToCompilationCache) { ...@@ -1838,13 +1838,12 @@ TEST(CodeSerializerPromotedToCompilationCache) {
{ {
// Lookup with different host_defined_options should fail: // Lookup with different host_defined_options should fail:
// TODO(10284): Enable host-defined options check again
ScriptDetails script_details(src); ScriptDetails script_details(src);
script_details.host_defined_options = isolate->factory()->NewFixedArray(5); script_details.host_defined_options = isolate->factory()->NewFixedArray(5);
MaybeHandle<SharedFunctionInfo> shared = MaybeHandle<SharedFunctionInfo> shared =
isolate->compilation_cache()->LookupScript(src, script_details, isolate->compilation_cache()->LookupScript(src, script_details,
LanguageMode::kSloppy); LanguageMode::kSloppy);
CHECK_EQ(*shared.ToHandleChecked(), *copy); CHECK(shared.is_null());
} }
delete cache; delete cache;
......
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