Closed Bug 1501154 Opened 3 years ago Closed 3 years ago
Assertion failure: throwing, at js/src/vm/JSContext
The following testcase crashes on mozilla-central revision 2872e7a3606d (build with --enable-debug, run with --fuzzing-safe --no-threads --no-baseline --no-ion): // Adapted from randomly chosen test: js/src/tests/test262/language/module-code/dynamic-import/syntax/valid/nested-block-empty-str-is-valid-assign-expr.js import(""); Backtrace: #0 0x0000559dff27d9c5 in JSContext::getPendingException (this=0x7f429e618000, rval=...) at js/src/vm/JSContext.cpp:1449 #1 0x0000559dfeadbece in js::GetAndClearException (cx=0x7f429e618000, res=...) at js/src/vm/Interpreter.cpp:5077 #2 0x0000559dfeb6277d in js::RejectPromiseWithPendingError (cx=<optimized out>, promise=...) at js/src/builtin/Promise.cpp:3412 #3 js::StartDynamicModuleImport (cx=0x7f429e618000, referencingPrivate=..., specifierArg=...) at js/src/builtin/ModuleObject.cpp:1849 #4 0x0000559dfead4bd7 in Interpret (cx=0x7f429e618000, state=...) at js/src/vm/Interpreter.cpp:4737 #5 0x0000559dfeac3a93 in js::RunScript (cx=0x7f429e618000, state=...) at js/src/vm/Interpreter.cpp:447 /snip For detailed crash information, see attachment.
autobisectjs shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/41812db6caba user: Jon Coppeard date: Mon Oct 22 11:28:17 2018 +0100 summary: Bug 1499140 - Support dynamic module import in the shell r=jandem Jon, is bug 1499140 a likely regressor?
Assignee: nobody → jcoppeard
There's a couple of things going on here. The first is that we accept the empty string as a valid module specifier. For HTML this has to be a URL or a relative path. For the shell we can do whatever we want but "" clearly should be rejected. The second is that the shell's FileAsString function to read the contents of a file doesn't cope well with errors. What's happening for this testcase is that we end up resolving the module's path to a directory and attempting to read that. The call to ftell() fails but the return value is not checked. In this case it returns -1 which we use as the length to read. When allocation fails we don't throw an exception which causes this assertion. The patch disallows the empty string as a module specifier and also tidies up the error handling in FileAsString.
Attachment #9019339 - Flags: review?(bbouvier)
Attachment #9019339 - Flags: review?(bbouvier) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/6f6a67ed98c7 Disallow the empty string as a module specifier in the shell and improve error handling in js::shell::FileAsString() r=bbouvier
You need to log in before you can comment on or make changes to this bug.