1358521 - XDROffThreadDecoder fails to decode inner functions (Gecko)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Nicolas B. Pierron [:nbp]]

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.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: XDRScript: Only fail on mismatch between the top-level JSScript and the decoding CompileOption.

Comment 2

[Nicolas B. Pierron [:nbp]]

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

Comment 3

[Pulsebot]

Pushed by [e-mail]:
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

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0f97ef13ab54