Profiler overflows stack when default thread stack size is < 160 KiB
Categories
(Core :: Gecko Profiler, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox76 | --- | fixed |
People
(Reporter: mforney, Assigned: mforney)
Details
Attachments
(1 file)
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:73.0) Gecko/20100101 Firefox/73.0
Steps to reproduce:
I built firefox for Linux with musl libc (testing a fix for bug 1615713), and started the profiler.
Actual results:
The profiler crashed due to a stack overflow, since it tries to allocate 160 KiB for backtrace information, but musl's default thread stack size is only 128 KiB (http://git.musl-libc.org/cgit/musl/tree/src/internal/pthread_impl.h#n182).
Expected results:
The profiler should have set the minimum stack size for the sampling thread to be at least as large as it needs to store N_STACK_BYTES to prevent the overflow.
This can be done with pthread_attr_setstacksize.
N_STACK_BYTES are needed to store backtrace information, so use
that plus some extra as the minimum stack size for the sampler
thread to ensure that it doesn't overflow.
|
||
Updated•4 years ago
|
Pushed by nerli@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9930f4d0e821 Set stack size of profiler sampler thread to N_STACK_BYTES + 20 KiB r=gerald,mstange
|
||
Comment 3•4 years ago
|
||
Backed out changeset 9930f4d0e821 (bug 1620549) for causing build bustages on platform-linux-android.cpp
Backout revision https://hg.mozilla.org/integration/autoland/rev/55be89265d71f28184ac168b08ebc2f3403a4b35
Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=292078651&repo=autoland
mforney can you please take a look?
Pushed by mstange@themasta.com: https://hg.mozilla.org/integration/autoland/rev/4074abe0bef9 Set stack size of profiler sampler thread to N_STACK_BYTES + 20 KiB r=gerald,mstange
Backout by nbeleuzu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b4599809c6e0 Backed out changeset 4074abe0bef9 for asan failures on platform.cpp . CLOSED TREE
Narcis, could you provide logs of the ASAN failures? I don't know how I can reproduce them myself.
|
|
||
Updated•4 years ago
|
I was able to reproduce the issue. It looks like the sampler thread needs at least N_STACK_BYTES + 38 KiB, so I bumped the stack size to N_STACK_BYTES + 80 KiB to keep a safe margin above that.
|
||
Updated•4 years ago
|
Pushed by aiakab@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d40b5da02500 Set stack size of profiler sampler thread to N_STACK_BYTES + 80 KiB r=gerald,mstange
|
|
||
Comment 9•4 years ago
|
||
Backed out changeset d40b5da02500 (bug 1620549) for causing xpcshell failures on test_active_configuration.js
Backout revision https://hg.mozilla.org/integration/autoland/rev/535458ddb77cb0140ad7eabb1e7a099e4eb20f36
Failure log https://treeherder.mozilla.org/logviewer.html#?job_id=292760357&repo=autoland
mforney can you please take a look?
|
Assignee | |
Comment 10•4 years ago
|
||
Okay, I managed to reproduce the error when building with the TSAN configuration given in
https://developer.mozilla.org/en-US/docs/Mozilla/Projects/Thread_Sanitizer#Adjusting_the_build_configuration
It appears that when built with TSAN, at least 292 KiB extra stack space (in addition to the 160 KiB for N_STACK_BYTES) are needed, though I don't understand why.
Markus, could you advise on an appropriate stack size for the sampler thread? Is this much stack usage expected? My next guess would be to try N_STACK_BYTES + 800 KiB (around 8 times as big as musl's default thread stack size), but my previous guesses have been way off.
Are there any other build configurations I should test?
|
|
||
Comment 11•4 years ago
|
||
I don't know. I don't have much guidance on how to find good stack sizes. I think our general approach has been "start small and then double it every time we run into a problem, until we no longer run into problems"...
I don't know how TSAN works or through what mechanism it places extra stuff on the stack.
|
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 12•4 years ago
|
||
Okay, let's try 800 KiB then. I updated the phabricator patch.
Markus, can you run this through the try server? I don't want to risk this getting rolled back again.
|
|
||
Comment 13•4 years ago
|
||
Ah, sorry I missed your message on Phabricator. I've started a try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=95f4cd62c4f698b401fffb396547231cf1f04adf
|
||
Comment 14•4 years ago
|
||
Pushed by cbrindusan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d7daa6afda13 Set stack size of profiler sampler thread to 800 KiB r=gerald,mstange
|
||
Comment 15•4 years ago
|
||
| bugherder | ||
Description
•