1341317 - Assertion failure: stack_[*size_].script() == script, at js/src/vm/GeckoProfiler.cpp:228 with enableGeckoProfiling and evalInCooperativeThread

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d84beb192e57 (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):

enableGeckoProfiling();
evaluate(`
evalInCooperativeThread(\`
  setInterruptCallback(function() { 
	cooperativeYield();
  });
  interruptIf(true);
\`);
`);



Backtrace:

 received signal SIGSEGV, Segmentation fault.
0x0000000000b12c88 in js::GeckoProfiler::exit (this=0x7ffff695e230, script=0x7ffff078f1a8, maybeFun=<optimized out>) at js/src/vm/GeckoProfiler.cpp:228
#0  0x0000000000b12c88 in js::GeckoProfiler::exit (this=0x7ffff695e230, script=0x7ffff078f1a8, maybeFun=<optimized out>) at js/src/vm/GeckoProfiler.cpp:228
#1  0x0000000000bb8b0c in js::probes::ExitScript (popProfilerFrame=true, maybeFun=<optimized out>, script=<optimized out>, cx=0x7ffff6921000) at js/src/vm/Probes-inl.h:62
#2  js::InterpreterFrame::epilogue (this=0x7ffff02330b0, cx=0x7ffff6921000, pc=0x7ffff0311533 "\231\220Ә\a\210\003") at js/src/vm/Stack.cpp:269
#3  0x000000000052d443 in Interpret (cx=0x7ffff6921000, state=...) at js/src/vm/Interpreter.cpp:4235
#4  0x0000000000533232 in js::RunScript (cx=cx@entry=0x7ffff6921000, state=...) at js/src/vm/Interpreter.cpp:409
#5  0x0000000000535ce1 in js::ExecuteKernel (cx=cx@entry=0x7ffff6921000, script=..., script@entry=..., envChainArg=..., newTargetValue=..., evalInFrame=..., evalInFrame@entry=..., result=result@entry=0x7ffff0233090) at js/src/vm/Interpreter.cpp:690
#6  0x00000000005360a8 in js::Execute (cx=cx@entry=0x7ffff6921000, script=script@entry=..., envChainArg=..., rval=rval@entry=0x7ffff0233090) at js/src/vm/Interpreter.cpp:723
#7  0x00000000008c79ba in ExecuteScript (cx=cx@entry=0x7ffff6921000, scope=scope@entry=..., script=script@entry=..., rval=rval@entry=0x7ffff0233090) at js/src/jsapi.cpp:4475
#8  0x00000000008d8888 in JS_ExecuteScript (cx=cx@entry=0x7ffff6921000, scriptArg=scriptArg@entry=..., rval=...) at js/src/jsapi.cpp:4501
#9  0x00000000004581eb in Evaluate (cx=0x7ffff6921000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:1812
#10 0x00000000005387ca in js::CallJSNative (cx=cx@entry=0x7ffff6921000, native=0x457d20 <Evaluate(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:281
[...]
#23 0x0000000000438fee in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8320
rax	0x0	0
rbx	0x7ffff695e230	140737330405936
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbca0	140737488338080
rsp	0x7fffffffbc70	140737488338032
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x0	0
r13	0x7ffff078f1a8	140737227846056
r14	0x7ffff69201c0	140737330151872
r15	0x7ffff0311533	140737223136563
rip	0xb12c88 <js::GeckoProfiler::exit(JSScript*, JSFunction*)+648>
=> 0xb12c88 <js::GeckoProfiler::exit(JSScript*, JSFunction*)+648>:	movl   $0x0,0x0
   0xb12c93 <js::GeckoProfiler::exit(JSScript*, JSFunction*)+659>:	ud2

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/fad2e60d7843
user:        Brian Hackett
date:        Fri Feb 17 05:13:11 2017 -0700
summary:     Bug 1337968 - Add API and shell harness for cooperative multithreading, r=jandem.

This iteration took 0.479 seconds to run.

Comment 2

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

Brian, is bug 1337968 a likely regressor?

Comment 3

[Brian Hackett [Laid off!]]

Attachment: patch

The Gecko profiler has a stack of profile entries, whose consistency can't really be maintained easily in a cooperatively multithreaded runtime.  This patch makes sure that the runtime is single threaded while the profiler is enabled, using the machinery added in bug 1341321.  Eventually it would be really good I think for the profiler to support multithreaded execution, but doing that might require pretty substantial changes inside and outside spidermonkey.

Comment 5

[Shu-yu Guo [:shu]]

Comment on attachment 8841226 [details] [diff] [review]
patch

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

::: js/src/vm/GeckoProfiler.cpp
@@ +553,5 @@
>  JS_FRIEND_API(void)
>  js::EnableContextProfilingStack(JSContext* cx, bool enabled)
>  {
> +    if (!cx->runtime()->geckoProfiler().enable(enabled))
> +        MOZ_CRASH("Execution in this runtime should already be single threaded");

How is it ensured that the runtime should already be single threaded?

Comment 6

[Shu-yu Guo [:shu]]

A related question: what kind of misuse results in enable(true) returning false? If the embedder's {Begin,End}SingleThreadedExecutionCallbacks are buggy?

Comment 7

[Brian Hackett [Laid off!]]

(In reply to Shu-yu Guo [:shu] from comment #6)
> A related question: what kind of misuse results in enable(true) returning
> false? If the embedder's {Begin,End}SingleThreadedExecutionCallbacks are
> buggy?

This is not necessarily misuse, but normally when we force the embedding into single threaded mode the callback will need to yield while other threads finish whatever they are doing.  If during that process a second thread tries to enter single threaded mode then runtime->beginSingleThreadedExecution() will return false --- the first thread has dibs on being the one in single threaded mode, basically.

The MOZ_CRASH in EnableContextProfilingStack is kind of a placeholder --- right now the profiler code outside Spidermonkey doesn't handle multithreaded runtimes, so there's no way right now that we can hit this crash.  When the profiler does begin supporting multithreaded runtimes, there are a few options:

- This crash can be changed to a 'return false' and the failure bubbled up the call stack.  I looked into doing this for this patch but the calling profiler code is not so straightforward and it wasn't clear how it should fail in some cases.

- The calling code can interface with the Quantum DOM scheduling stuff to make sure that when it enables profiling it is always the only running cooperative thread, and doesn't need to worry about the beginSingleThreadedExecution() call failing.

- The profiler inside and outside Spidermonkey can be changed to handle multiple cooperative threads running at a time, and avoid the need to ensure the runtime is single threaded.

I don't know which of these options is best, so it seemed better to punt on handling this until work starts on supporting multithreaded runtimes from the Gecko side.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c385308aa772
Require runtimes to be single threaded while the Gecko profiler is in use, r=shu.

Comment 9

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

https://hg.mozilla.org/mozilla-central/rev/c385308aa772