1212349 - Crash [@ IsInsideNursery] with evalInWorker

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 727d765a5ed8 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --enable-simulator=arm --enable-debug, run with --fuzzing-safe --thread-count=2 --baseline-eager --ion-check-range-analysis --arm-asm-nop-fill=1 --ion-extra-checks --ion-eager --ion-offthread-compile=off --arm-hwcap=vfp --nursery-size=128):

var lfcode = new Array();
lfcode.push = loadFile;
lfcode.push(`
    enableLastWarning();
    evalInWorker(\`
        try { 
            f(); 
        } catch ({StructType,uint8,float32}) { 
            Uint8Array(typeof(f), "object");     
        }  
    \`);
`);
lfcode.push("");
function loadFile(lfVarx) {
    if (lfVarx.substr(-3) != ".js" && lfVarx.length != 1) {
        evaluate(lfVarx);
    } else if (!isNaN(lfVarx)) {}
}



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
IsInsideNursery (cell=0xee0001a0) at ../../dist/include/js/HeapAPI.h:331
#0  IsInsideNursery (cell=0xee0001a0) at ../../dist/include/js/HeapAPI.h:331
#1  MustSkipMarking<JSObject*> (obj=0xee0001a0) at js/src/gc/Marking.cpp:650
#2  DoMarking<JSObject> (gcmarker=0xf7a2af2c, thing=0xee0001a0) at js/src/gc/Marking.cpp:686
#3  0x084d317c in operator()<JSObject> (this=<synthetic pointer>, gcmarker=0xf7a2af2c, t=<optimized out>) at js/src/gc/Marking.cpp:698
#4  DispatchTyped<DoMarkingFunctor<JS::Value>, js::GCMarker*&> (val=<synthetic pointer>, f=...) at ../../dist/include/js/Value.h:1841
#5  DoMarking<JS::Value> (thing=..., gcmarker=0xf7a2af2c) at js/src/gc/Marking.cpp:705
#6  DispatchToTracer<JS::Value> (trc=0xf7a2af2c, thingp=0x97dac60 <gLastWarning+16>, name=0x8ad71dd "PersistentRooted<Value>") at js/src/gc/Marking.cpp:556
#7  0x084b2c6e in markChain<js::TraceNullableRoot<JS::Value> > (name=0x8ad71dd "PersistentRooted<Value>", list=..., trc=0xf7a2af2c) at js/src/gc/RootMarking.cpp:224
#8  js::gc::MarkPersistentRootedChainsInLists (roots=..., trc=trc@entry=0xf7a2af2c) at js/src/gc/RootMarking.cpp:244
#9  0x084b2eb1 in js::gc::MarkPersistentRootedChains (trc=trc@entry=0xf7a2af2c) at js/src/gc/RootMarking.cpp:258
#10 0x084b30f0 in js::gc::GCRuntime::markRuntime (this=this@entry=0xf7a29210, trc=trc@entry=0xf7a2af2c, traceOrMark=traceOrMark@entry=js::gc::GCRuntime::MarkRuntime) at js/src/gc/RootMarking.cpp:290
#11 0x087fe26c in js::gc::GCRuntime::beginMarkPhase (this=this@entry=0xf7a29210, reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:3983
#12 0x08811023 in js::gc::GCRuntime::incrementalCollectSlice (this=this@entry=0xf7a29210, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:5938
#13 0x0881202d in js::gc::GCRuntime::gcCycle (this=this@entry=0xf7a29210, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6212
#14 0x088125fb in js::gc::GCRuntime::collect (this=this@entry=0xf7a29210, nonincrementalByAPI=nonincrementalByAPI@entry=true, budget=..., reason=reason@entry=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6328
#15 0x08812972 in js::gc::GCRuntime::gc (this=0xf7a29210, gckind=GC_NORMAL, reason=JS::gcreason::DESTROY_CONTEXT) at js/src/jsgc.cpp:6392
#16 0x08812b16 in js::DestroyContext (cx=cx@entry=0xf7a7c020, mode=mode@entry=js::DCM_FORCE_GC) at js/src/jscntxt.cpp:186
#17 0x08754fb8 in JS_DestroyContext (cx=0xf7a7c020) at js/src/jsapi.cpp:799
#18 0x080ccc70 in DestroyContext (withGC=true, cx=<optimized out>) at js/src/shell/js.cpp:5748
#19 main (argc=12, argv=0xffffce04, envp=0xffffce38) at js/src/shell/js.cpp:6588
eax	0xee0ffff0	-300941328
ebx	0x979c51c	158975260
ecx	0xee0001a0	-301989472
edx	0xf7a2af2c	-140333268
esi	0xee0001a0	-301989472
edi	0x97dac50	159231056
ebp	0xffffc618	4294952472
esp	0xffffc5f0	4294952432
eip	0x84cced7 <DoMarking<JSObject>(js::GCMarker*, JSObject*)+39>
=> 0x84cced7 <DoMarking<JSObject>(js::GCMarker*, JSObject*)+39>:	mov    (%eax),%eax
   0x84cced9 <DoMarking<JSObject>(js::GCMarker*, JSObject*)+41>:	test   %eax,%eax

Comment 1

[Christian Holler (:decoder)]

Needinfo from jonco for this one.

Comment 2

[Jon Coppeard (:jonco)]

This is a shell-only bug, so not s-s.

Specifically, the shell's global variables are being accessed from the main thread and from worker threads.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1212349-shell-globals

The issue the testcase causes is that gLastWarning can be accessed from the main runtime and from workers.  A value written by one a worker that then dies can been observed by a racy access as pointing into free memory.

Here's a patch to add ShellRuntime which encapsulates the shell state specific to a runtime.  Previously these were all global variables in js.cpp which were shared between the main runtime and workers.

I wasn't too sure about the watchdog thread state, but it seems that WatchdogMain gets a JSRuntime* so I guess it needs to be specific to a single runtime.  This means that workers will each get a separate watchdog thread if they do anything that needs it.

I also made some variables const where appropriate.

Comment 4

[Jan de Mooij [:jandem]]

Comment on attachment 8671237 [details] [diff] [review]
bug1212349-shell-globals

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

Nice fix!

::: js/src/shell/js.cpp
@@ +273,5 @@
>  extern JS_EXPORT_API(void)   add_history(char* line);
>  } // extern "C"
>  #endif
>  
> +ShellRuntime::ShellRuntime()

Nit: also initialize sleepWakeup to nullptr.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f46ca0a518a

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1f46ca0a518a