Closed Bug 1358521 Opened 3 years ago Closed 3 years ago

XDROffThreadDecoder fails to decode inner functions (Gecko)

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: nbp, Assigned: nbp)

References

Details

Attachments

(1 file)

When decoding function from the script loader in Bug 900784, we fail because we attempt to decode inner functions and match the CompileOption of the inner function with the CompileOption of the top-level.

I guess this is not an issue on the XDR test cases because the evaluate function already expects the top-level to have a return value, as opposed as the Script Loader.
Blocks: 1316078
Comment on attachment 8860441 [details] [diff] [review]
XDRScript: Only fail on mismatch between the top-level JSScript and the decoding CompileOption.

Review of attachment 8860441 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsscript.cpp
@@ +510,2 @@
>          mozilla::Maybe<CompileOptions> options;
> +        if (xdr->hasOptions() && (scriptBits & (1 << OwnSource))) {

This patch add the OwnSource check here, which is used to distinguish if we are on the top-level JSScript or not.

OwnSource is not made to check for the top-level but the outer most script which is used to own the ScriptSource when encoding/decoding would be copied on all inner function and lazy functions which are referenced by it.  The fact that we encode the top-level with the XDRIncrementalEncoder makes the correspondence between the top-level and the OwnSource flag a thing that we can check against.

In our case, we are also guarding with the xdr->hasOptions() condition, which is only used by the XDROffThreadDecoder [1] because we have to verify that the CompileOption corresponds with the compiled script. (Otherwise we fail with the Failure_WrongCompileOption code)

The problem here is that we were doing that even for inner function, which are expected to have the NoScriptRVal flag unset, as opposed to the top-level loaded by the nsScriptLoader, but like the JS Shell test cases of the XDROffThreadDecoder.

This patch undo this check (added in Bug 1316078) to only use it on the top-level JSScript and no longer on the inner functions.

luke reviewed this patch over IRC [2]

[1] http://searchfox.org/mozilla-central/search?q=symbol:_ZNK2js8XDRState10hasOptionsEv&redirect=false
[2] http://logs.glob.uno/?c=mozilla%23jsapi#c840249
Attachment #8860441 - Flags: review+
Pushed by npierron@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f97ef13ab54
XDRScript: Only fail on mismatch between the top-level JSScript and the decoding CompileOption. r=luke
Attachment #8860441 - Flags: review?(shu)
https://hg.mozilla.org/mozilla-central/rev/0f97ef13ab54
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.