1415571 - Hit MOZ_CRASH(mutable variables can't be exported) at js/src/wasm/WasmModule.cpp:992 or Crash [@ GetGlobalExport]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 4e6df5159df3 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --cpu-count=2 --disable-oom-functions):

See attachment.


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000e070c8 in GetGlobalExport (jsval=..., globalImports=..., globalIndex=1, globals=..., cx=0x7ffff6948000) at js/src/wasm/WasmModule.cpp:992
#0  0x0000000000e070c8 in GetGlobalExport (jsval=..., globalImports=..., globalIndex=1, globals=..., cx=0x7ffff6948000) at js/src/wasm/WasmModule.cpp:992
#1  CreateExportObject (exports=..., globalImports=..., memoryObj=..., tableObj=..., funcImports=..., instanceObj=..., cx=0x7ffff6948000) at js/src/wasm/WasmModule.cpp:1087
#2  js::wasm::Module::instantiate (this=this@entry=0x7ffff4e47ba0, cx=0x7ffff6948000, funcImports=..., funcImports@entry=..., tableImport=..., tableImport@entry=..., memoryImport=..., memoryImport@entry=..., globalImports=..., instanceProto=..., instance=...) at js/src/wasm/WasmModule.cpp:1195
#3  0x0000000000e07f97 in Instantiate (cx=cx@entry=0x7ffff6948000, module=..., importObj=..., importObj@entry=..., instanceObj=..., instanceObj@entry=...) at js/src/wasm/WasmJS.cpp:1091
#4  0x0000000000e08b1f in js::WasmInstanceObject::construct (cx=0x7ffff6948000, argc=<optimized out>, vp=<optimized out>) at js/src/wasm/WasmJS.cpp:1116
#5  0x000000000055e631 in js::CallJSNative (cx=0x7ffff6948000, native=native@entry=0xe08960 <js::WasmInstanceObject::construct(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
[...]
#23 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8962
rax	0x0	0
rbx	0x7ffff691e1e0	140737330143712
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffc000	140737488338944
rsp	0x7fffffffbd60	140737488338272
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7fffffffbef0	140737488338672
r13	0x7ffff691e1d0	140737330143696
r14	0x7fffffffbed0	140737488338640
r15	0x7fffffffbf10	140737488338704
rip	0xe070c8 <js::wasm::Module::instantiate(JSContext*, JS::Handle<JS::GCVector<JSFunction*, 0ul, js::TempAllocPolicy> >, JS::Handle<js::WasmTableObject*>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) const+3736>
=> 0xe070c8 <js::wasm::Module::instantiate(JSContext*, JS::Handle<JS::GCVector<JSFunction*, 0ul, js::TempAllocPolicy> >, JS::Handle<js::WasmTableObject*>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) const+3736>:	movl   $0x0,0x0
   0xe070d3 <js::wasm::Module::instantiate(JSContext*, JS::Handle<JS::GCVector<JSFunction*, 0ul, js::TempAllocPolicy> >, JS::Handle<js::WasmTableObject*>, JS::Handle<js::WasmMemoryObject*>, mozilla::Vector<js::wasm::Val, 0ul, js::SystemAllocPolicy> const&, JS::Handle<JSObject*>, JS::MutableHandle<js::WasmInstanceObject*>) const+3747>:	ud2

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[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/fbe76e704b6b
user:        Luke Wagner
date:        Thu Feb 09 18:32:39 2017 -0600
summary:     Bug 1338002 - Baldr: temporarily accept both 0xd and 0x1 (r=sunfish)

This iteration took 0.613 seconds to run.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Probably gets further than comment 2; looking.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1415571: Fix inline export numbers for globals;

Review commit: https://reviewboard.mozilla.org/r/197746/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197746/

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Attachment: Bug 1415571: Export values of immutable variable globals;

Review commit: https://reviewboard.mozilla.org/r/197748/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/197748/

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

(Note the first patch just relates to our incorrect parsing of inline-exported globals, which I stuttered on when trying to make a simpler test case)

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 8926496 [details]
Bug 1415571: Fix inline export numbers for globals;

https://reviewboard.mozilla.org/r/197746/#review202946

Comment 8

[Luke Wagner [:luke]]

Comment on attachment 8926497 [details]
Bug 1415571: Export values of immutable variable globals;

https://reviewboard.mozilla.org/r/197748/#review202948

Oof, thanks for the fix!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47fcf4a498b8
Fix inline export numbers for globals; r=luke
https://hg.mozilla.org/integration/autoland/rev/5bbbbcc79a5a
Export values of immutable variable globals; r=luke

Comment 10

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8926497 [details]
Bug 1415571: Export values of immutable variable globals;

Approval Request Comment
[Feature/Bug causing the regression]: wasm globals
[User impact if declined]: trivial way to crash the browser, no security risk
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: very little
[Why is the change risky/not risky?]: nicely tested
[String changes made/needed]: n/a

(I think fix to release is optional)

Comment 11

[Benjamin Bouvier [:bbouvier] (inactive)]

(wasm disabled by default on esr52)

Comment 12

[Julien Cristau [:jcristau]]

Too late for 57.

Comment 13

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/47fcf4a498b8
https://hg.mozilla.org/mozilla-central/rev/5bbbbcc79a5a