1400458 - Critical garbage collection segfault in latest SpiderMonkey 45 standalone release

Status: RESOLVED INACTIVE

(Core :: JavaScript Engine, defect, P1)

Description

[nathanrogers]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.79 Safari/537.36

Steps to reproduce:

For completeness' sake, I am using Ubuntu 14.04, although I don't expect that to matter.

This bug originally was found *without* JS_GC_ZEAL.  In the case where garbage collection was triggered in the middle of parsing, and a partially initialized unboxed object happened to get traced, and an optimizing compiler that had performed the legal optimization of removing a NULL check on a |this| pointer was used to compile the code (which happened with using GCC 6.3.0 targeting x64 with -O2), a segfault would happen.  This would reliably happen when parsing scripts that perfectly met the above conditions, although I was able to reproduce it without any input by using GCC 6.3.0 and JS_GC_ZEAL set to collect every allocation.  I confirmed that the two bugs were identical by finding a fix for the zealous version, and then verifying that it also fixed the issue when using the original particular scripts as input.

Download and set up GCC 6.3.0.  That exact script that I used to build and install it can be found here https://cobalt.googlesource.com/cobalt/+/master/src/starboard/linux/x64x11/gcc/6.3/download_gcc.sh.

Download the latest SpiderMonkey 45 standalone release, found in mozjs-45.0.2.tar.bz2, at https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Releases/45.

# The following step assumes GCC 6.3.0 was installed from the script linked to above, modify the paths in this line to wherever your actual installation is.
$ export CC=~/cobalt-toolchains/x86_64-linux-gnu-gcc-6.3.0/gcc/bin/gcc CXX=~/cobalt-toolchains/x86_64-linux-gnu-gcc-6.3.0/gcc/bin/g++ LD_LIBRARY_PATH=~/cobalt-toolchains/x86_64-linux-gnu-gcc-6.3.0/gcc/lib64
$ tar -xvf mozjs-45.0.2.tar.bz2
$ cd mozjs-45.0.2
$ cd js/src
$ autoconf # My system autoconf is version 2.13
$ mkdir my_build
$ cd my_build
$ ../configure --enable-gczeal
$ make -j$(nproc)


Actual results:

After following the repro build steps:

$ dist/bin/js --gc-zeal=2,1
Segmentation fault (core dumped)

$ gdb --args dist/bin/js --gc-zeal=2,1
(gdb) r
Program received signal SIGSEGV, Segmentation fault.
DispatchToTracer<JSObject*> (trc=0x7ffff696c240, thingp=0x0, name=0xc4326d "parser.object")
    at <...>/mozjs-45.0.2/js/src/gc/Marking.cpp:596
596	        return DoMarking(static_cast<GCMarker*>(trc), *thingp);

The program will crash due to dereferencing a NULL pointer.  The reason that this happens is because the NULL check is performed on the |this| pointer in a member function.  The C++ standard specifies that a |this| pointer can never be NULL, and therefore it is a legal optimization to convert a NULL check into a no-op, which GCC 6.3.0 -O2 does.


Expected results:

The resulting JS shell binary should be able to successfully initialize without segfaulting.

After encountering this issue, I checked out trunk SpiderMonkey from the readonly git mirror of gecko-dev at https://github.com/mozilla/gecko-dev.  I built it using the same steps listed in the repro steps above, and the segfault did not happen.  After examining the code, this problem was apparently fixed during the refactoring that took place in commit 41c582e943b05324519f42e464273fffb5e19db7 (Bug 1233121 - Refactor ObjectBox tracing r=terrence).  After the refactorings in that commit were applied, the NULL check takes place outside of the member function, which is thankfully not undefined behavior.

Since SpiderMonkey 45.0.2 is the latest release of standalone SpiderMonkey, can this commit be cherry picked into it or a followup LTS update?  I will gladly do it if someone can point me to instructions on how to do so.  If this work has already been done, then can a large warning (preferably in a  large font and capital letters) about this issue and other known stabillity/security issues that affect 45.0.2 be put near the download link on the official release page?

Thanks.

Comment 1

[Jon Coppeard (:jonco)]

Thanks for the report.  We're working on a standalone release of version 52 in bug 1379541.  This contains the fix.

Comment 2

[Nicolas B. Pierron [:nbp]]

(In reply to nathanrogers from comment #0)
> Since SpiderMonkey 45.0.2 is the latest release of standalone SpiderMonkey,
> can this commit be cherry picked into it or a followup LTS update? 

I apologized that this issue went under our radar. The good news is that newer LTS after 45, such as 52 and 60, should have this fix included.

We do not have any mechanism for patching versions which are now deprecated. Therefore it is safer to follow SpiderMonkey releases, which are following releases / LTSs branches of Firefox.

The release process of SpiderMonkey used to be a manual process handled by a few SpiderMonkey developer, which is now replaced by an automatic process, which follow the release process of Firefox.  Therefore, similar issue should no longer be a problem in the future.