1815863 - Assertion failure: IsAbsolutePath(loadPathStr), at js/src/shell/ModuleLoader.cpp:53

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[lukas.bernhard]

Steps to reproduce:

This bug only affects the reprl fuzzing mode of the js-shell.
In reprl mode, function ProcessArgs might be executed multiple times. Within this function, the global variable processWideModuleLoadPath is defined:
https://searchfox.org/mozilla-central/rev/28c0d45a553fd2817ac14e1562435e86dc0aa403/js/src/shell/js.cpp#10454

Access to this variable is racy, as worker threads from within WorkerMain might read this variable as well.
https://searchfox.org/mozilla-central/rev/28c0d45a553fd2817ac14e1562435e86dc0aa403/js/src/shell/js.cpp#4171

The order of events leading to the crash is as follows:

• t1: ProcessArgs initializes processWideModuleLoadPath
• t2 starts...
• t1: Next iteration of ProcessArgs starts reassignment of processWideModuleLoadPath
• t1: old value of processWideModuleLoadPath is removed
• t2: runs JS::ConstUTF8CharsZ path(processWideModuleLoadPath.get(), strlen(processWideModuleLoadPath.get())); -> processWideModuleLoadPath currently empty
• t1: processWideModuleLoadPath is assigned a new value
• t2: IsAbsolutePath(loadPathStr) fails because loadPathStr is empty

Unfortunately, I don't have a deterministic reproducer; the time window is rather narrow.

Comment 1

[Nicolas B. Pierron [:nbp]]

Jon, once you have time to look at it, this might make the life nicer for fuzzers.

Comment 2

[Jon Coppeard (:jonco)]

It sounds like the fuzzer is restarting the main runtime while there are still workers running. This shouldn't be possible, and may not be a good idea as it may generate false positive crashes.

A lot of cleanup is done by RAII classes in the shell's main(), e.g. shutdownShellThreads. Perhaps the fuzzer code and this cleanup code need to be rearranged relative to each other?

I don't know how the fuzzer is doing this (I can't actually see ProcessArgs being called more than once). Lukas, do you know how this works?

Comment 3

[lukas.bernhard]

During fuzzing, the reprl_mode variable is true. This allows the loop ending here https://searchfox.org/mozilla-central/rev/b579290e6b830d1b3f0a941203b0c0e1e56c42a3/js/src/shell/js.cpp#11343 to enter ProcessArgs multiple times.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1815863 - Part 0: Move shell options setup into a separate function r?nbp

Code motion only. This factors out a function to initialize the OptionParser.

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1815863 - Factor out module loader initialization and only perform it once r?nbp

Depends on D170779

Comment 6

[Phabricator Automation]

Comment on attachment 9319495 [details]
Bug 1815863 - Part 0: Move shell options setup into a separate function r?nbp

Revision D170769 was moved to bug 1818516. Setting attachment 9319495 [details] to obsolete.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/199ff6a3f68d
Factor out module loader initialization and only perform it once r=nbp

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/199ff6a3f68d