Closed
Bug 1333392
Opened 7 years ago
Closed 7 years ago
Assertion failure: !mEntered, at dist/include/mozilla/Vector.h:472 or various crashes with evalInWorker and SPS profiling
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla54
Tracking | Status | |
---|---|---|
firefox51 | --- | unaffected |
firefox52 | --- | unaffected |
firefox53 | --- | unaffected |
firefox54 | --- | fixed |
People
(Reporter: decoder, Assigned: lth)
Details
(4 keywords, Whiteboard: [fuzzblocker] [jsbugmon:update])
Attachments
(1 file, 1 obsolete file)
7.25 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 8ff550409e1d (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu --enable-simulator=arm, run with --fuzzing-safe --thread-count=2 --baseline-eager --ion-offthread-compile=off): for(i=0; i < 100; ++i) evalInWorker(` try { evaluate(\` enableSPSProfiling() enableSingleStepProfiling(); \`); } catch(exc1) {} `); Backtrace: received signal SIGSEGV, Segmentation fault. [Switching to Thread 0xf12ffb40 (LWP 10564)] 0x080a400e in mozilla::Vector<mozilla::Vector<char16_t, 0u, js::SystemAllocPolicy>, 0u, js::SystemAllocPolicy>::back (this=0x8c972c0 <stacks>) at dist/include/mozilla/Vector.h:472 #0 0x080a400e in mozilla::Vector<mozilla::Vector<char16_t, 0u, js::SystemAllocPolicy>, 0u, js::SystemAllocPolicy>::back (this=0x8c972c0 <stacks>) at dist/include/mozilla/Vector.h:472 #1 0x0809e19f in SingleStepCallback (arg=<optimized out>, sim=<optimized out>, pc=<optimized out>) at js/src/shell/js.cpp:4899 #2 0x08460381 in js::jit::Simulator::execute<false> (this=0xf407c000) at js/src/jit/arm/Simulator-arm.cpp:4748 #3 js::jit::Simulator::callInternal (this=0xf407c000, entry=0xf7b3ca68 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>) at js/src/jit/arm/Simulator-arm.cpp:4838 #4 0x084606a1 in js::jit::Simulator::call (this=<optimized out>, entry=0xf7b3ca68 "\360O-\351\004\320M\342\020\212-\355\r\200\240\341h\220\235\345\r\260\240\341t\240\235", <incomplete sequence \345>, argument_count=<optimized out>) at js/src/jit/arm/Simulator-arm.cpp:4921 #5 0x0899d64f in EnterBaseline (cx=cx@entry=0xf4283000, data=...) at js/src/jit/BaselineJIT.cpp:157 [...] #12 0x0809d05f in WorkerMain (arg=0xf404e660) at js/src/shell/js.cpp:3371 #13 0x0809fa2b in js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::callMain<0u> (this=0xf790b1a0) at js/src/threading/Thread.h:234 #14 js::detail::ThreadTrampoline<void (&)(void*), WorkerInput*&>::Start (aPack=0xf790b1a0) at js/src/threading/Thread.h:227 #15 0xf7fb228a in start_thread (arg=0xf12ffb40) at pthread_create.c:333 #16 0xf7cd44ce in clone () from /lib32/libc.so.6 eax 0x0 0 ebx 0x8c95ff4 147415028 ecx 0xf7d9f864 -136710044 edx 0x0 0 esi 0x22 34 edi 0x8c95ff4 147415028 ebp 0xf12fe8e8 4046448872 esp 0xf12fe8e0 4046448864 eip 0x80a400e <mozilla::Vector<mozilla::Vector<char16_t, 0u, js::SystemAllocPolicy>, 0u, js::SystemAllocPolicy>::back()+78> => 0x80a400e <mozilla::Vector<mozilla::Vector<char16_t, 0u, js::SystemAllocPolicy>, 0u, js::SystemAllocPolicy>::back()+78>: movl $0x0,0x0 0x80a4018 <mozilla::Vector<mozilla::Vector<char16_t, 0u, js::SystemAllocPolicy>, 0u, js::SystemAllocPolicy>::back()+88>: ud2
Updated•7 years ago
|
status-firefox51:
--- → unaffected
status-firefox52:
--- → unaffected
status-firefox53:
--- → unaffected
Reporter | ||
Comment 1•7 years ago
|
||
This crashes in many different ways on ARM, most crashes are intermittent. Marking as fuzzblocker.
Summary: Assertion failure: !mEntered, at dist/include/mozilla/Vector.h:472 with evalInWorker and SPS profiling → Assertion failure: !mEntered, at dist/include/mozilla/Vector.h:472 or various crashes with evalInWorker and SPS profiling
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][fuzzblocker]
Updated•7 years ago
|
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 2•7 years ago
|
||
The code for SingleStepCallback on ARM uses a global data structure without a lock, which should at least be fixed first. It is credible that a race here could cause bugs, and evalInWorker just forks off code to run, so there could be plenty of concurrent workers trying to access that data at the same time. In fact a race is almost inevitable, given the structure of the test case.
Assignee | ||
Comment 3•7 years ago
|
||
Um, that's in shell/js.cpp.
Assignee | ||
Comment 4•7 years ago
|
||
This protects the global shell-internal variable 'stacks' with a lock. The solution has the unfortunate effect that the threads will now move singly through the critical section and that may change the effects you're looking for -- it depends on what you're looking for! To do better we would probably look into some lock-free / immutable data structure but the complexity of the patch would go up quite a bit :-}
Attachment #8830807 -
Flags: feedback?(choller)
Reporter | ||
Comment 5•7 years ago
|
||
Comment on attachment 8830807 [details] [diff] [review] bug1333392-no-racy-globals.patch This seems to fix the problems, thanks! :)
Attachment #8830807 -
Flags: feedback?(choller) → feedback+
Comment 6•7 years ago
|
||
I'm not familiar with this code, but could we move this Vector into ShellContext? That's what we do for other per-thread fields in the shell.
Flags: needinfo?(jdemooij)
Assignee | ||
Comment 7•7 years ago
|
||
Right. Looks like this code was introduced by bug 1027885, which *possibly* predates the ShellContext, and might not have been translated when ShellContext was introduced. I've not bothered to dig deeply; clearly the vector is per-thread. Better fix coming up.
Assignee | ||
Comment 8•7 years ago
|
||
Assignee: nobody → lhansen
Attachment #8830807 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8831065 -
Flags: review?(jdemooij)
Comment 9•7 years ago
|
||
Comment on attachment 8831065 [details] [diff] [review] bug1333392-singlestep-profiling-stacks.patch Review of attachment 8831065 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for fixing!
Attachment #8831065 -
Flags: review?(jdemooij) → review+
Assignee | ||
Comment 10•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0e8bedb9a623
Assignee | ||
Comment 11•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b771ead300e82a2e2f0a63d01a3e5256de96f126 Bug 1333392 - single-step profiling stacks are per-thread. r=jandem
Comment 12•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b771ead300e8
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Updated•7 years ago
|
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:update]
You need to log in
before you can comment on or make changes to this bug.
Description
•