Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(349)

Issue 11362048: GTTF: Make Linux stack dump signal handler async-signal safe. (Closed)

Created:
8 years, 1 month ago by Paweł Hajdan Jr.
Modified:
8 years, 1 month ago
CC:
chromium-reviews, erikwright+watch_chromium.org, Markus (顧孟勤)
Visibility:
Public.

Description

GTTF: Make Linux stack dump signal handler async-signal safe. This also re-enables in-process stack dumping on Linux. BUG=101155 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=167714

Patch Set 1 #

Total comments: 13

Patch Set 2 : fixes #

Total comments: 2

Patch Set 3 : fixes #

Total comments: 12

Patch Set 4 : fixes #

Patch Set 5 : fixes #

Total comments: 7

Patch Set 6 : fixes #

Patch Set 7 : trybot debugging ios & android #

Total comments: 2

Patch Set 8 : fixes #

Patch Set 9 : hopefully the ultimate android fix #

Total comments: 4

Patch Set 10 : shess' comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+336 lines, -66 lines) Patch
M base/base.gyp View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M base/debug/debugger_posix.cc View 1 3 chunks +15 lines, -2 lines 0 comments Download
M base/debug/stack_trace.h View 1 2 3 4 5 7 1 chunk +14 lines, -0 lines 0 comments Download
M base/debug/stack_trace_posix.cc View 1 2 3 4 5 6 7 8 9 8 chunks +208 lines, -60 lines 0 comments Download
M base/debug/stack_trace_unittest.cc View 1 2 3 4 5 6 7 5 chunks +95 lines, -4 lines 0 comments Download

Messages

Total messages: 26 (0 generated)
Paweł Hajdan Jr.
Please review: Jim: entire CL Julien: is there some way for me to share code ...
8 years, 1 month ago (2012-11-01 22:54:46 UTC) #1
jln (very slow on Chromium)
On 2012/11/01 22:54:46, Paweł Hajdan Jr. wrote: > Please review: > > Jim: entire CL ...
8 years, 1 month ago (2012-11-02 01:17:48 UTC) #2
jar (doing other things)
I didn't have time to go though the rest of the code, and stopped after ...
8 years, 1 month ago (2012-11-02 02:36:57 UTC) #3
Paweł Hajdan Jr.
On 2012/11/02 01:17:48, Julien Tinnes wrote: > Excellent to see this! sandbox/linux/sandbox-bpf can use base, ...
8 years, 1 month ago (2012-11-06 17:58:45 UTC) #4
jln (very slow on Chromium)
On 2012/11/06 17:58:45, Paweł Hajdan Jr. wrote: > On 2012/11/02 01:17:48, Julien Tinnes wrote: > ...
8 years, 1 month ago (2012-11-06 18:46:30 UTC) #5
jar (doing other things)
Much better... and I just realized something else to make it much much simpler. You ...
8 years, 1 month ago (2012-11-06 20:40:03 UTC) #6
Paweł Hajdan Jr.
PTAL
8 years, 1 month ago (2012-11-07 00:03:56 UTC) #7
jar (doing other things)
https://codereview.chromium.org/11362048/diff/5/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): https://codereview.chromium.org/11362048/diff/5/base/debug/stack_trace_posix.cc#newcode256 base/debug/stack_trace_posix.cc:256: if (i < 0) { This should only be ...
8 years, 1 month ago (2012-11-07 00:32:07 UTC) #8
Paweł Hajdan Jr.
https://codereview.chromium.org/11362048/diff/5/base/debug/stack_trace_unittest.cc File base/debug/stack_trace_unittest.cc (right): https://codereview.chromium.org/11362048/diff/5/base/debug/stack_trace_unittest.cc#newcode161 base/debug/stack_trace_unittest.cc:161: EXPECT_EQ("-9223372036854775808", On 2012/11/07 00:32:07, jar wrote: > On a ...
8 years, 1 month ago (2012-11-07 00:52:28 UTC) #9
Scott Hess - ex-Googler
Sorry about the late and nit-picky comments. This feels kind of excessive, like maybe rather ...
8 years, 1 month ago (2012-11-07 01:02:26 UTC) #10
jar (doing other things)
Windows is only built in 32 bit mode, and hence pointers will only be 32 ...
8 years, 1 month ago (2012-11-07 01:07:04 UTC) #11
jar (doing other things)
FYI: Linux is built and shipped both as a 32 and 64 bit binary. I'd ...
8 years, 1 month ago (2012-11-07 01:14:49 UTC) #12
Paweł Hajdan Jr.
PTAL I'm trying to keep itoa_r general, so if it needs to be re-used later ...
8 years, 1 month ago (2012-11-07 17:34:17 UTC) #13
Scott Hess - ex-Googler
https://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): https://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc#newcode64 base/debug/stack_trace_posix.cc:64: #endif // defined(USE_SYMBOLIZE) I might be missing something, but ...
8 years, 1 month ago (2012-11-07 17:58:07 UTC) #14
Paweł Hajdan Jr.
http://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): http://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc#newcode64 base/debug/stack_trace_posix.cc:64: #endif // defined(USE_SYMBOLIZE) On 2012/11/07 17:58:07, shess wrote: > ...
8 years, 1 month ago (2012-11-07 19:45:11 UTC) #15
Scott Hess - ex-Googler
https://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): https://codereview.chromium.org/11362048/diff/1006/base/debug/stack_trace_posix.cc#newcode64 base/debug/stack_trace_posix.cc:64: #endif // defined(USE_SYMBOLIZE) On 2012/11/07 19:45:11, Paweł Hajdan Jr. ...
8 years, 1 month ago (2012-11-07 20:29:26 UTC) #16
Paweł Hajdan Jr.
On 2012/11/07 20:29:26, shess wrote: > This is general-purpose code which the signal handler happens ...
8 years, 1 month ago (2012-11-07 21:13:20 UTC) #17
Scott Hess - ex-Googler
On 2012/11/07 21:13:20, Paweł Hajdan Jr. wrote: > On 2012/11/07 20:29:26, shess wrote: > > ...
8 years, 1 month ago (2012-11-07 21:48:24 UTC) #18
jar (doing other things)
Commenting only on the itoa (which looks much much better now), and leaving it to ...
8 years, 1 month ago (2012-11-09 04:20:41 UTC) #19
Paweł Hajdan Jr.
PTAL This restores some of the older logic for demangling. http://codereview.chromium.org/11362048/diff/24001/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): http://codereview.chromium.org/11362048/diff/24001/base/debug/stack_trace_posix.cc#newcode249 ...
8 years, 1 month ago (2012-11-12 23:47:48 UTC) #20
Paweł Hajdan Jr.
On 2012/11/07 21:48:24, shess wrote: > As noted on the bug, this is in a ...
8 years, 1 month ago (2012-11-12 23:50:12 UTC) #21
jar (doing other things)
itoa looks nice, safe, and clean. The tests for the function also look great... so ...
8 years, 1 month ago (2012-11-14 00:16:48 UTC) #22
Scott Hess - ex-Googler
I still don't feel convinced that the problem you are solving is an actual problem ...
8 years, 1 month ago (2012-11-14 01:12:35 UTC) #23
Paweł Hajdan Jr.
Thank you for the review! https://codereview.chromium.org/11362048/diff/10004/base/debug/stack_trace_posix.cc File base/debug/stack_trace_posix.cc (right): https://codereview.chromium.org/11362048/diff/10004/base/debug/stack_trace_posix.cc#newcode185 base/debug/stack_trace_posix.cc:185: strncat(buf, "Received signal ", ...
8 years, 1 month ago (2012-11-14 17:40:11 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/phajdan.jr@chromium.org/11362048/13013
8 years, 1 month ago (2012-11-14 17:40:31 UTC) #25
commit-bot: I haz the power
8 years, 1 month ago (2012-11-14 19:32:25 UTC) #26
Change committed as 167714

Powered by Google App Engine
This is Rietveld 408576698