Closed Bug 1666683 Opened 4 years ago Closed 4 years ago

Check input CompileOptions when decoding, or getting from cache

Categories

(Core :: JavaScript Engine, task, P1)

Product:
Core
Component:
JavaScript Engine
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
83 Branch
Tracking Flags:
Tracking Status
firefox83 --- fixed

People

(Reporter: arai, Assigned: arai)

References

(Blocks 1 open bug)

Details

Attachments

(12 files, 1 obsolete file)

Bug 1666683 - Part 1: Reorder CompileOptions parameter. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 2: Add CompileOptions parameter to JS::DecodeScript, as a placeholder. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 3: Add options parameter to ScriptPreloader::GetCachedScript. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 4: Add options parameter to ReadCachedScript in mozJSLoaderUtils. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 5: Add options parameter to nsXPConnect::ReadScript. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 6: Add ScriptPreloader::FillCompileOptionsForCachedScript. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 7: Fix shell offThreadDecodeScript to use the same option as execute. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 8: Move options_ field from XDROffThreadDecoder to XDRDecoder. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 9: Add XDRState::isMultiDecode as a preparation to check options conditionally. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 10: Check CompileOptions when decoding XDR. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 11: Add FillImmutableFlagsFromCompileOptionsForTopLevel to centralize CompileOptions-ImmutableScriptFlags interaction. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1666683 - Part 12: Add JS::CheckCompileOptionsMatch and check options in ScriptPreloader. r?tcampbell!
(deleted), text/x-phabricator-request
Details

Depends on D90585

Blocks: stencil-backlog
No longer blocks: stencil-mvp
Attachment #9177295 - Attachment is obsolete: true

In ScriptLoader, same CompileOption is passed to both JS::Compile and JS::DecodeScriptMaybeStencil.
but in other places, callers aren't passing same CompileOption as JS::Compile to decoding API.

  1. JS::DecodeScript doesn't receive CompileOptions.
  2. JS::DecodeMultiOffThreadScripts receives CompileOptions, but the option is shared across multiple scripts that can be compiled with different options

and in remaining consumers, the decoded script is stored into cache and used later, and compilation happens when the cache isn't found.
It means we should check CompileOptions vs ImmutableScriptFlags when getting from the cache, instead of when decoding.

I'll add an API that checks CompileOptions vs ImmutableScriptFlags, for existing consumers.

once we replace the cache to store stencil, we can compare CompileOptions directly between cache and compilation input.

Summary: Check input CompileOptions when decoding → Check input CompileOptions when decoding, or getting from cache

plan:

  1. for JS::DecodeScript, add CompileOptions parameter and modify the consumer to pass the same options as JS::Compile, and check the options vs flags internally
  2. for JS::DecodeMultiOffThreadScripts, modify the consumers to pass options that is consistent as much as possible (but still not all options can be same), and check some options vs flags internally
  3. for JS::DecodeMultiOffThreadScripts consumers, when getting a decoded script from the cache, make it receive CompileOptions and check options vs flags

This adds the parameter, without using it, also without passing the correct
value.
The later patch will change the consumer of those API to pass the correct value,
and then use the passed parameter.

Depends on D92403

Attachment #9179562 - Attachment description: Bug 1666683 - Part 12: Add JS::IsCompileOptionsMatching and check options in ScriptPreloader. r?tcampbell! → Bug 1666683 - Part 12: Add JS::CheckCompileOptionsMatch and check options in ScriptPreloader. r?tcampbell!
Pushed by arai_a@mac.com: https://hg.mozilla.org/integration/autoland/rev/c5f2032ed919 Part 1: Reorder CompileOptions parameter. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/a6da1179a589 Part 2: Add CompileOptions parameter to JS::DecodeScript, as a placeholder. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/777c94a3c977 Part 3: Add options parameter to ScriptPreloader::GetCachedScript. r=tcampbell,kmag https://hg.mozilla.org/integration/autoland/rev/3014569a17c7 Part 4: Add options parameter to ReadCachedScript in mozJSLoaderUtils. r=tcampbell,kmag https://hg.mozilla.org/integration/autoland/rev/8ecde54fab31 Part 5: Add options parameter to nsXPConnect::ReadScript. r=tcampbell,kmag https://hg.mozilla.org/integration/autoland/rev/483e81a2dbd7 Part 6: Add ScriptPreloader::FillCompileOptionsForCachedScript. r=tcampbell,kmag https://hg.mozilla.org/integration/autoland/rev/3e4b7122f576 Part 7: Fix shell offThreadDecodeScript to use the same option as execute. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/50cd482cdb64 Part 8: Move options_ field from XDROffThreadDecoder to XDRDecoder. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/5a6df8bb1ab5 Part 9: Add XDRState::isMultiDecode as a preparation to check options conditionally. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/240b2e7d6afc Part 10: Check CompileOptions when decoding XDR. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/2fbbdd37bb54 Part 11: Add FillImmutableFlagsFromCompileOptionsForTopLevel to centralize CompileOptions-ImmutableScriptFlags interaction. r=tcampbell https://hg.mozilla.org/integration/autoland/rev/8827caa78683 Part 12: Add JS::CheckCompileOptionsMatch and check options in ScriptPreloader. r=tcampbell,kmag
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: