CyclicModuleRecord's Evaluate method attempts to use empty CycleRoot when called again after an evaluation error

[jonco3]

Take for example the module 'a' which simply throws:

throw 1;

After calling Evaluate the module will have Status |evaluated| and CycleRoot |empty|. The former is set by Evaluate step 9.a.ii and the latter is not set because InnerModuleEvaluation step 13 performs ReturnIfAbrupt on the result of ExecuteModule before this can happen.

If EvaluateModule is called a second time on the same module, we will set the module variable to the empty CycleRoot in step 3 and then attempt to access its fields in step 4.

The same problem arises if a second module 'b' that imports the former module is evaluated:

import {} from "a";

This time we access the CycleRoot field in InnerModuleEvaluation step 11.d.iv.1.

For synchronously executed modules, evaluation terminates in InnerModuleEvaluation before cycles have been identified, so it's reasonable to expect CycleRoot to be empty for a module with status evaluated if EvaluationError has been set.

I believe it is valid to call Execute twice for the same module. The description implies this when it says "The first time it is called on a module in a given strongly connected component". (The description also say "Evaluate transitions this module's Status from linked to either evaluating-async or evaluated" which implies the opposite, however that conflicts with the assertion in step 2 which allows it to be called on modules in any of those states, not just linked.)

[jonco3]

Previously this worked in SpiderMonkey due to a bug where modules with errors were treated differently. It works in V8 because there is an extra check for errors at the start of Evaluate.

[davidot]

I don't see any way in the ECMA spec to call evaluate on the same module twice.
But this is a real issue for a browser.
From the HTML spec:
https://html.spec.whatwg.org/multipage/webappapis.html#fetch-a-single-module-script
Uses a module map to cache modules
This means that a page like:

<script type="module" src="runtimefail.js"></script>
<script type="module" src="runtimefail.js"></script>

Will call evaluate on the module a second time even if it had an evaluation error, see https://html.spec.whatwg.org/multipage/webappapis.html#run-a-module-script .

This can seemingly be solved by copying step 4. of https://tc39.es/ecma262/#sec-moduleevaluation also as a step before switching to the cycle root.

I have also made an accompanying whatwg/html issue whatwg/html#8553 .

[devsnek]

it seems like this just needs a check for EvaluationError, feel free to open a pr if you'd like.

[linusg]

Fixed in #3583.