Closed Bug 1501154 Opened 10 months ago Closed 10 months ago

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

Categories

(Core :: JavaScript Engine, defect, P1, critical)

x86_64
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla65
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed

People

(Reporter: gkw, Assigned: jonco)

References

(Blocks 1 open bug)

Details

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

Attachments

(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
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?
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 jcoppeard@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/6f6a67ed98c7
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in before you can comment on or make changes to this bug.