1233179 - Crash [@ js::ModuleEnvironmentObject::importBindings] or Crash [@ js::shadow::Object::numFixedSlots] Assertion failure: env, at frontend/BytecodeEmitter.cpp:1598 with ES6 Modules

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 749f9328dd76 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug, run with --fuzzing-safe --thread-count=2):

let c = parseModule(`
  function a(x) { return x; }
  function b(x) { return i<40; }
  function d(x) { return x + 3; }
`);
getLcovInfo();



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
js::ModuleEnvironmentObject::importBindings (this=0x0) at js/src/vm/ScopeObject.cpp:408
#0  js::ModuleEnvironmentObject::importBindings (this=0x0) at js/src/vm/ScopeObject.cpp:408
#1  0x0000000000879739 in js::ModuleEnvironmentObject::hasImportBinding (this=<optimized out>, name=..., name@entry=...) at js/src/vm/ScopeObject.cpp:429
#2  0x000000000091c874 in js::frontend::BytecodeEmitter::tryConvertFreeName (this=this@entry=0x7fffffffbe00, pn=pn@entry=0x7ffff698f180) at js/src/frontend/BytecodeEmitter.cpp:1599
#3  0x000000000091cee2 in js::frontend::BytecodeEmitter::bindNameToSlotHelper (this=this@entry=0x7fffffffbe00, pn=pn@entry=0x7ffff698f180) at js/src/frontend/BytecodeEmitter.cpp:1774
#4  0x000000000091d11f in js::frontend::BytecodeEmitter::bindNameToSlot (this=0x7fffffffbe00, pn=0x7ffff698f180) at js/src/frontend/BytecodeEmitter.cpp:1937
#5  0x00000000009213a6 in js::frontend::BytecodeEmitter::emitNameOp (this=0x7fffffffbe00, pn=0x7ffff698f180, callContext=<optimized out>) at js/src/frontend/BytecodeEmitter.cpp:2557
#6  0x00000000009241d4 in js::frontend::BytecodeEmitter::emitTree (this=this@entry=0x7fffffffbe00, pn=0x7ffff698f180, emitLineNote=js::frontend::BytecodeEmitter::EMIT_LINENOTE) at js/src/frontend/BytecodeEmitter.cpp:8579
#7  0x00000000009283f3 in js::frontend::BytecodeEmitter::emitLeftAssociative (this=this@entry=0x7fffffffbe00, pn=pn@entry=0x7ffff698f228) at js/src/frontend/BytecodeEmitter.cpp:7343
#8  0x00000000009242c4 in js::frontend::BytecodeEmitter::emitTree (this=this@entry=0x7fffffffbe00, pn=0x7ffff698f228, emitLineNote=js::frontend::BytecodeEmitter::EMIT_LINENOTE) at js/src/frontend/BytecodeEmitter.cpp:8413
[...]
#15 0x0000000000925230 in js::frontend::BytecodeEmitter::emitFunctionScript (this=0x7fffffffbe00, body=0x7ffff698f0d8) at js/src/frontend/BytecodeEmitter.cpp:3566
#16 0x0000000000925f71 in js::frontend::CompileLazyFunction (cx=cx@entry=0x7ffff6907400, lazy=lazy@entry=..., chars=<optimized out>, length=<optimized out>) at js/src/frontend/BytecodeCompiler.cpp:836
#17 0x0000000000751e51 in JSFunction::createScriptForLazilyInterpretedFunction (cx=cx@entry=0x7ffff6907400, fun=..., fun@entry=...) at js/src/jsfun.cpp:1427
#18 0x0000000000779762 in getOrCreateScript (cx=0x7ffff6907400, this=<optimized out>) at js/src/jsfun.h:389
#19 GenerateLcovInfo (cx=cx@entry=0x7ffff6907400, comp=0x7ffff46d8400, out=...) at js/src/jsopcode.cpp:1993
#20 0x0000000000785579 in js::GetCodeCoverageSummary (cx=cx@entry=0x7ffff6907400, length=length@entry=0x7fffffffca50) at js/src/jsopcode.cpp:2015
#21 0x000000000081d42b in GetLcovInfo (cx=0x7ffff6907400, argc=<optimized out>, vp=0x7ffff46400a0) at js/src/builtin/TestingFunctions.cpp:3017
#22 0x00000000008588c1 in CallJSNative (args=..., native=0x81d390 <GetLcovInfo(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907400) at js/src/jscntxtinlines.h:235
[..]
#34 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:6877
rax	0x0	0
rbx	0x7ffff7e00a00	140737352043008
rcx	0x7fffffffbdd0	140737488338384
rdx	0x7ffff6907400	140737330050048
rsi	0x7fffffffb8e0	140737488337120
rdi	0x0	0
rbp	0x0	0
rsp	0x7fffffffb828	140737488336936
r8	0x7ffff698f180	140737330606464
r9	0x7ffff46eede8	140737294298600
r10	0x3	3
r11	0x2	2
r12	0x7ffff698f180	140737330606464
r13	0x7ffff7e00a00	140737352043008
r14	0x7fffffffb8d0	140737488337104
r15	0x7fffffffb8e0	140737488337120
rip	0x878fe0 <js::ModuleEnvironmentObject::importBindings()>
=> 0x878fe0 <js::ModuleEnvironmentObject::importBindings()>:	mov    0x8(%rdi),%rax
   0x878fe4 <js::ModuleEnvironmentObject::importBindings()+4>:	mov    0x10(%rax),%eax

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/037f7cfaa141
user:        Jon Coppeard
date:        Wed Nov 18 11:09:15 2015 +0000
summary:     Bug 1219288 - Add GETIMPORT instruction for accessing module imports that are not namespace imports r=shu

This iteration took 249.977 seconds to run.

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Jon, is bug 1219288 a likely regressor?

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1233179-module-environment

ModuleObject::environment() returns null until after module declarations have been instantiated to follow the spec, so we have to call initialEnvironment() here to get the object.

This would probably require a bug in the module loader to actually make this happen in real use.

Comment 4

[Shu-yu Guo [:shu]]

(In reply to Jon Coppeard (:jonco) from comment #3)
> Created attachment 8700037 [details] [diff] [review]
> bug1233179-module-environment
> 
> ModuleObject::environment() returns null until after module declarations
> have been instantiated to follow the spec, so we have to call
> initialEnvironment() here to get the object.
> 
> This would probably require a bug in the module loader to actually make this
> happen in real use.

Hm, this change seems a little shady. Should parseModule be able to be called without some barebones module loader logic to begin with?

Comment 5

[Jon Coppeard (:jonco)]

(In reply to Shu-yu Guo [:shu] from comment #4)
> Hm, this change seems a little shady. Should parseModule be able to be
> called without some barebones module loader logic to begin with?

Well, I don't see how we can prevent that.

Actually, looking at this again what's happening is that GetCodeCoverageSummary() is causing the lazy function to be compiled before the module declarations have been instantiated.  I'm not sure there is anything we can do about that.

An alternative might be to get rid of the distinction between ModuleObject::environment() and initialEnvironment().  They're C++ accessors anyway, and won't be called by the loader.  I just did it this way to be more "spec-like" but it probably doesn't matter.  We can make a separate API to get the environment from a module object that follows the spec if and when the module loader needs this.

Comment 6

[Shu-yu Guo [:shu]]

Comment on attachment 8700037 [details] [diff] [review]
bug1233179-module-environment

Review of attachment 8700037 [details] [diff] [review]:
-----------------------------------------------------------------

In light of Jon's comments.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9eb4c8f77d4b

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9eb4c8f77d4b