Closed Bug 1501154 Opened 3 years ago Closed 3 years ago

Assertion failure: throwing, at js/src/vm/JSContext.cpp:1449


(Core :: JavaScript Engine, defect, P1)




Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed


(Reporter: gkw, Assigned: jonco)



(4 keywords, Whiteboard: [jsbugmon:update])


(2 files)

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


#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

For detailed crash information, see attachment.
autobisectjs shows this is probably related to the following changeset:

The first bad revision is:
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?
Blocks: 1499140
Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Priority: -- → P1
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
Disallow the empty string as a module specifier in the shell and improve error handling in js::shell::FileAsString() r=bbouvier
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.