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

Issue 1996243002: Workaround ARM tracing issues for non-component builds. (Closed)

Created:
4 years, 7 months ago by Dmitry Skiba
Modified:
4 years, 3 months ago
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Workaround Android tracing issues for non-component builds. On Android/ARM native allocation tracing has two issues: 1. When built in arm mode it crashes sooner or later when unwinding calls from JNI, see https://crbug.com/602701#c18 2. When built in thumb mode unwinding simply doesn't work because stack frames are not stable, and there can be arbitrary number of registers between r7 and lr, see https://llvm.org/bugs/show_bug.cgi?id=18505#c5 This change fixes both issues in non-component builds by relying on a fact that all Chrome code lives in a single mapped region. So there is a simple and fast way to check whether given PC is from Chrome. That check is used to solve both issues: 1. In arm mode unwinding stops as soon as first non-Chrome PC is detected. System libraries on both Linux and Android are built without frame pointers anyway, so there is no point in unwinding further. 2. In thumb mode, where lr (r14) register can be up to 7 registers away from r7 on the stack, lr is searched by probing each possible value to be inside Chrome's code region. This method sometimes finds false positives (which symbolize to things like $d.232), but overall works well and produces readable traces. Note that this is only applicable for Clang, because in GCC builds even r7 is not set up correctly. BUG=602701

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+218 lines, -61 lines) Patch
M base/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M base/debug/stack_trace.h View 1 chunk +11 lines, -1 line 0 comments Download
M base/debug/stack_trace.cc View 3 chunks +89 lines, -1 line 0 comments Download
M base/third_party/symbolize/symbolize.h View 1 chunk +17 lines, -0 lines 0 comments Download
M base/third_party/symbolize/symbolize.cc View 4 chunks +100 lines, -59 lines 0 comments Download

Messages

Total messages: 6 (3 generated)
Dmitry Skiba
PTAL, alternative to https://codereview.chromium.org/1975393002/
4 years, 7 months ago (2016-05-20 05:06:01 UTC) #4
Primiano Tucci (use gerrit)
On 2016/05/20 05:06:01, Dmitry Skiba wrote: > PTAL, alternative to https://codereview.chromium.org/1975393002/ Thanks for looking at ...
4 years, 7 months ago (2016-05-20 14:21:13 UTC) #5
Dmitry Skiba
4 years, 7 months ago (2016-05-20 17:03:16 UTC) #6
On 2016/05/20 14:21:13, Primiano Tucci wrote:
> On 2016/05/20 05:06:01, Dmitry Skiba wrote:
> > PTAL, alternative to https://codereview.chromium.org/1975393002/
> 
> Thanks for looking at this. See my comment in the bug. I much more prefer
> https://codereview.chromium.org/1975393002/
> Rationale:
> 1. there is precedence (and opportunity for code sharing) in oilpan. 
> 2. Checking that a PC is inside the FP doesn't guarantee you that following
the
> FP chain is going to not segfault.
> Also, purely from a code viewpoint, this one seems to bake lot of complex
logic
> inside of chrome and make lot of pretty strong assumptions (name of the .so,
the
> fact that you can read /proc/self/maps). For instance, in the case of bpf
> sandbox you'll no longer be able to read maps, so this will have a rather
short
> life.

Agree, lets focus on the other CL.

Powered by Google App Engine
This is Rietveld 408576698