New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 786450 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

OOP HP e2e test malloc expectations fails on Android.

Project Member Reported by erikc...@chromium.org, Nov 17 2017

Issue description

backtrace returns an identical [empty] backtrace for all call sites. As a result, all allocations get glommed together and the OOP HP e2e malloc expectations fail. 

This appears to be device/OS dependent. It fails on a nexus 5x kitkat device [trybot], but passes on a local nexus 5x oreo device.
 
Whoops, to clarify, this fails on Nexus 5 kitkat, but passes on Nexus 5x oreo.
Cc: mariakho...@chromium.org dskiba@chromium.org
Labels: Restrict-View-Google
Adding internal links, so adding R-V-G. I chatted with dskiba and maria. I will attempt to summarize here, please correct me if I got anything wrong.

There are 3 different APKs, which depend on Android version. See go/clank. These are Chrome, ChromeModern, and MonoChrome. 

On 32-bit devices, we always ship 32-bit Chrome.
On 64-bit devices, on stable, we always ship 32-bit chrome.
On 64-bit devices, on canary/dev, on Chrome/ChromeModern, we ship 64-bit Chrome.
On 64-bit devices, on canary/dev, we ship MonoChrome, which has 32-it and 64-bit instructions. When a 64-bit app uses a Webview, that uses a 64-bit browser process and 32-bit renderer process. When Chrome is launched by itself, we use a 32-bit browser and 32-bit renderer.

This is relevant because on 64-bit ARM devices, we turn on frame pointers, so OOP HP will always work. https://cs.chromium.org/chromium/src/build/config/compiler/compiler.gni?type=cs&q=enable_frame_pointers&sq=package:chromium&l=112

On 32-bit ARM devices, we use backtrace() for OOP HP. This is observed to not work correctly on Nexus 5 kitkat, using a release, non-official build.

Note that if we attempt to turn on frame pointers, backtrace() will crash:
https://bugs.chromium.org/p/chromium/issues/detail?id=670464#c12. The problem appears to be an issue with libc++ used in r12 NDK. Thankfully, if we turn on framepointers, we won't be using backtrace(). Furthermore, there's ongoing work to move to the r16 NDK: https://bugs.chromium.org/p/chromium/issues/detail?id=771171#c9.

I think we need to figure out: 

1. Which platforms/configurations in the wild does OOP HP currently work on? 

I've confirmed that we don't work on Pixel 2 Oreo, Chrome Canary. See go/crash/dd5710c729637c4b. The report uploads correctly and has MDPs, but backtrace() returns nothing. This is expected, since we exclude unwind tables, and Monochrome uses 32-bit build. I think the answer is: Chrome and ChromeModern on 64-bit ARM devices, which should have frame pointers.

2. Which platforms/configurations do we want to support that we currently do not? 

Supporting all platforms is not necessary. We just need sufficient coverage to get back interesting data in the field. There are also other solutions to getting backtraces from Android in the field. 

3. How do we ensure we have test coverage for these platforms?


Cc: primiano@chromium.org
2. I agree that we don't need to support everything, but it seems like not supporting ANYTHING that's running Android N+ is going to be bad news for us.
I think we have two realistic options on arm32:

1) Use stack scanning (dskiba wrote some initial support, not sure what the full state of that is).
Pro: if combined with some caching* could be as fast as frame pointer unwinding
Con: it has false positives (See explanation here: https://bugs.chromium.org/p/chromium/issues/detail?id=719679#c3)

2) Sideload (i.e. download from the network) *some* unwind tables to do accurate unwinding when we are in the heap profiler form the field experiment.
Now, what do I mean with *some*? IIRC the full unwind tables are ~1.5 MB for arm32 (at least 2 years ago when I last checked). Could be okay to just download them over WiFi but feels a bit harder for users over mobile data connections. unwind tables, however, contain way more data than what we need. In fact they allow to recover the stack from any random point int he execution.
This is a must for crash reporting, where we can crash in random points, and hence need to be able to unwind from random contexts. But it's not a must for heap profiling, where we only unwind from known places.
We need to do some research here, but the high level proposal here is (warning: I am writing the sort of stuff that triggers fireworks in dskiba@'s mind, so fasten his seatbelts :P):
can we postprocess the symbols of release binaries and reconstruct some minimal unwind tables that allow to walk back the stack only from branch instructions? Before we even try to do that, we should figure out how big such tables would be to check whether it's worth pursuing this road at all. A way to figure out an upper bound would be something like:
- How many "branch and link" instructions we have in total?
- How much data would we need to keep for each instruction pointer? (I guess the offsets to fetch the return address from the stack)
- What is a realistic minimal encoding for this?
Let's try to answer these three questions and use this to decide whether to adventure ourselves here at all or not?

* This doc has some handwavy proposal for caching: https://docs.google.com/document/d/1s1PcaNwFgF272OMLfgmh0MI_G78DX4bh9HUS_qUZXDU/edit
So reading through this, it sounds like there are 2 independent dimensions to work through.

(1) How to get data from Monochrome (Api 24+)
(2) What to do with arm32


For (1), what is stopping us from always shipping a 64-bit browser in Monochrome?  Would there be objections if I tried to make this change?

For (2), AFAICT, stack scanning still depends on the compiler emitting frame pointers. The scanning is just for correcting a bad guess at unwind but ultimately it is still retrieving the PC via finding a frame pointer to anchro from.

Can we enable framepointers in arm32? If we can, then I think problem is solved.

If not, then we either have to leave a blank hole in the 32-bit data, or we have to do something fancier...possibly along the lines of what primiano is suggesting. Note, I actually do not think you can reduce the unwind tables in a large manner. Even though the domain of inputs on the top-level frame is restricted to the various allocator calls, the rest of the call stack is effectively random.  The ARM EH-ABI entries with the unwind bytecode, IIRC, roughly corresponds to a scope (as opposed to every line of code) so it's already somewhat compact. I think you'd only be able to prune out functions that were somehow guaranteed to be only used inside a call graph where there's a long branch of non-allocating calls...
There is no magic in Monochrome, it's just a 32-bit app. The magic is in the fact that it has 32/64-bit webview implementations, and the proper one is chosen based on the host app. So when webview starts a renderer, ABI of the app is used, and that is arm32.

(A) We could make Monochrome 64-bit, but that would badly regress memory (because both browser / renderers would be 64-bit). And webview memory would regress too (because of the renderer). Maybe we could do that only for dev/canary, similarly how ChromeModern is 64-bit in those configurations?

(B) We also could compile in arm mode (as opposed to thumb) on 32-bit arm platform, and that would allow us to enable frame pointers (which are broken in thumb mode). Again, maybe we could do that only for dev/canary where performance regression from arm/frame pointers won't matter much?

(C) Another option is to just fix frame pointers in thumb mode in clang. Maybe the fix is not that hard, considering that frame-pointer code is generated, it just sets fp incorrectly.

Overall it looks to me that arm mode / frame pointers is safer bet than 64-bit / frame pointers, but that needs measurements.


Personally I like Primiano's second point (and fireworks :)), but don't know much about dwarf to validate it. I can take a look once I finish the startup performance stuff (hopefully ~2 months from now).
Re #6

> what is stopping us from always shipping a 64-bit browser in Monochrome? 

This is an extremely delicate discussion which keeps going back and forth at very high levels every ~6 months. TL;DR is what dskiba said, arm64 has a very high binary and memory cost, and little runtime benefits (if any). Is going to be very hard to convince an exec to switch to arm64 just because we'd like to unwind.

> For (2), AFAICT, stack scanning still depends on the compiler emitting frame pointers.

AFAICT we don't have frame pointers on arm. +dskiba, maybe I am misremembering and we never had stack scanning on arm?

> Can we enable framepointers in arm32? If we can, then I think problem is solved.
It is not because of https://bugs.llvm.org/show_bug.cgi?id=18505
Essentially on arm32 the current ABI doesn't mandate that the offset between the frame pointer and the return address is constant, which makes frame pointers useless.

> (B) We also could compile in arm mode (as opposed to thumb) on 32-bit arm platform

That would make the release model quite complicated, as we would need different build flags for different channels, which is something that, AFAICT, today we don't do and would get some quite big pushback from release managers.


> (C) Another option is to just fix frame pointers in thumb mode in clang

I am not sure that you can do that without breaking the android-eabi.
From that LLVM bug APCS seems the only Procedure Call Standard (PCS) that guarantees fixed offsets between return address and frame pointer.
According to http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042f/IHI0042F_aapcs.pdf, APCS is deprecated in favor of AAPCS, which seems to unify the arm and thumb PCS.
If you look at the section "Stage C – Assignment of arguments to registers and stack" the mapping of arguments onto the final stack layout seems quite convoluted. So I *think* (but I am not really sure) that what described on that LLVM bugs happens to comply with all those rules. Maybe ask advice to some arm compiler expert?

Regarding thinning out the unwind table, even if we did that, we'd have to implement some sort of storage and serving infrastructure that synced with our builds which would then have to be maintained. The complexity cost of that sounds high (and fragile).

Re framepointers in arm, they are enabled for arm64. See:

https://chromium.googlesource.com/chromium/src/+/7375379aaf6be6005d07c1a46e10cf300f8db67f/build/config/compiler/compiler.gni#110

And the scanning code is only enabled if CAN_UNWIND_WITH_FRAME_POINTERS is true, which is set to exclude arm32 + thumb:

https://cs.chromium.org/chromium/src/build/config/compiler/compiler.gni?type=cs&q=CAN_UNWIND_WITH_FRAME_POINTERS&sq=package:chromium&l=129

So scanning doesn't work w/o fp support...which makes sense because the code is specifically trying to find candidate frame pointers.

From this, it still sounds like enabling arm32 framepointers is the most straightforward solution.  Aside from the arm32 thumb bug codegen "bug", what other potential blockers would there be to enabling frame pointers in arm32?

Reading through http://infocenter.arm.com/help/topic/com.arm.doc.espc0002/ATPCS.pdf, r4-r11 are all callee saved registers. Thus I think the ordering in thumb mode is largely an artifact of register number as opposed to intention by the compiler writers... I wonder if we can just reorder the dump/store to put r7 next to lr when emitting code in thumb mode...  Will follow up on the llvm bug.
... I just found

https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMCallingConv.td#L257

Looks like https://github.com/llvm-mirror/llvm/commit/6e490efa619ad4f6f731f365255c2607697bc14c might have fixed this in some cases.

Time to go disassemble some ARM code....
per discussion on the llvm bug thread:
  https://bugs.llvm.org/show_bug.cgi?id=18505

Looks like llvm generates useful framepointers in thumb mode now. Gcc still doesn't, but we don't use gcc for this build anyways, so that's fine.

I think the next step is to turn on framepointers in arm32 canary and see what happens.

Any objections?
Wow, great news that frame pointers are fixed in thumb! I wonder if stack unwinding "just works" with those, or needs some tweaking? Have you tried running heap profiler in thumb mode with frame pointers?
Labels: OS-Android
I'm WFH with a fever today so didn't quite have the energy to figure out a real android chromium build...will try that tomorrow.

However, reading through your stack scanning code, I believe it actually should work. AFAIK, thumb does not change stack alignment so the IsStackFrameValid() looks correct.

Also, FYI, I think we can tighten the alignment check here:

  https://cs.chromium.org/chromium/src/base/debug/stack_trace.cc?q=NextStack&sq=package:chromium&l=66

to require 8-bytes on all ARM platforms per:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka4127.html
Finally got things building and running. The stack scanning as is does not quite work.

Currently blocked on getting a debugger attached.

I'll have to debug more on monday.
Good news! The stack trace actually does work!  I was just verfiying the addresses against the wrong file (ugh) so they were non-sensical.

Gonna attempt to enable framepointers on all arm32 builds and see what happens.
Project Member

Comment 17 by bugdroid1@chromium.org, Dec 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c29b757124e1ceb921ad61d023b6b741eda92d4

commit 8c29b757124e1ceb921ad61d023b6b741eda92d4
Author: Albert J. Wong <ajwong@chromium.org>
Date: Thu Dec 14 18:48:00 2017

Enable frame pointers on arm32.

LLVM fixed the bug where framepointers were not located next to the return
address in thumb mode. https://bugs.llvm.org/show_bug.cgi?id=18505

With that fix, enabling framepointers on arm32 bring it in line with other
build configurations and allows for better stack traces.

Bug: 786450
Change-Id: I01a1c9f82fac22b2bbc15c6f52deed8aca7263ea
Reviewed-on: https://chromium-review.googlesource.com/826255
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524127}
[modify] https://crrev.com/8c29b757124e1ceb921ad61d023b6b741eda92d4/build/config/compiler/compiler.gni
[modify] https://crrev.com/8c29b757124e1ceb921ad61d023b6b741eda92d4/tools/gn/bootstrap/bootstrap.py

Enabling frame pointers grew our apk by 2MB.

https://chromeperf.appspot.com/report?sid=d4ad0c0ca13532bc603ccb2fbf832ac334993838534598c9087515088ede48e2&rev=524127

Not sure exactly what frame pointers gives us, but I suspect it's not worth a 2MB hit to apk size.
Oh wow, 2MiB. I wonder what about performance? Last time I tried enabling frame pointers (which also required switching thumb->arm) I got ~15% perf regressions.

Still, maybe we can enabled frame pointers for dev/canary? With them we'll be able to get much more detailed heap profiles from the field.
I think having them enabled on dev/canary only is a good compromise.

I'm going to leave the change in for a bit to see what the other bots all say.

Comment 21 by mef@chromium.org, Dec 14 2017

Cc: mef@chromium.org
Project Member

Comment 22 by bugdroid1@chromium.org, Dec 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f4bb44bdd8d3cee8685dfe08851383bbfba13535

commit f4bb44bdd8d3cee8685dfe08851383bbfba13535
Author: Albert J. Wong <ajwong@chromium.org>
Date: Fri Dec 15 05:39:30 2017

Revert "Enable frame pointers on arm32."

This reverts commit 8c29b757124e1ceb921ad61d023b6b741eda92d4.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Enable frame pointers on arm32.
> 
> LLVM fixed the bug where framepointers were not located next to the return
> address in thumb mode. https://bugs.llvm.org/show_bug.cgi?id=18505
> 
> With that fix, enabling framepointers on arm32 bring it in line with other
> build configurations and allows for better stack traces.
> 
> Bug: 786450
> Change-Id: I01a1c9f82fac22b2bbc15c6f52deed8aca7263ea
> Reviewed-on: https://chromium-review.googlesource.com/826255
> Reviewed-by: Brett Wilson <brettw@chromium.org>
> Commit-Queue: Albert J. Wong <ajwong@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#524127}

TBR=ajwong@chromium.org,brettw@chromium.org

Change-Id: Ie327529394021473cfffd0ee80baecce4bd83e2a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 786450
Reviewed-on: https://chromium-review.googlesource.com/828607
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524337}
[modify] https://crrev.com/f4bb44bdd8d3cee8685dfe08851383bbfba13535/build/config/compiler/compiler.gni
[modify] https://crrev.com/f4bb44bdd8d3cee8685dfe08851383bbfba13535/tools/gn/bootstrap/bootstrap.py

FYI - right now the Android release bots assert that the libchrome.so built for each channel is the same, so there will be further obstacles to having compiler flags differ per-channel. I believe the reason somewhat that builds don't take too long, but moreso that we don't have to upload different symbols to breakpad for each channel.
Oh yeah I forgot about #23. I think that (having a different libchrome.so) would require to revisit all the assumptions baked in our release, crash reporting and symbol access code that the libchrome.so symbols are identical for all the channels (and recursively for monochrome's release).

Another proposal dskiba@ had was to check what is the gzipped size of unwind tables, and if acceptable ship them in the pak files (rarhter than in the binary) and uncompress them only when using the HP.
tl;dr: Two things hurt us on arm32 disproportionately: (1) STL generating _lots_ of small functions (2) Thumb -> ARM transition with FPs is extra costly as it requires 32-bit opcodes.

I created 3 builds:
  (a) No-framepointers (baseline config)
  (b) -fno-omit-frame-pointers (similar to the patch earlier)
  (c) cflags = [ "-Xclang", "-momit-leaf-frame-pointer" ] (no frame pointers on leaf functions)

I looked very closely at the generated code between the (a) anc (c) versions. The conclusions should hold true for (b) as well.

Build was done with following args.gn (the framepointer flag was hacked in by hand):

enable_nacl=false
use_goma=true
goma_dir="/opt/google/goma/"
remove_webcore_debug_symbols=true
target_os="android"
is_official_build=true

Sizes are as follows:

   text    data     bss     dec     hex filename
48491485        2044012 1335752 51871249        3177e11 out/scratch/libchrome-good.so
54330857        2044012 1335752 57710621        370981d out/Clank-keep-fp/lib.unstripped/libchrome.so
54250981        2044012 1335752 57630745        36f6019 out/scratch/libchrome-non-leaf.so

We're seeing about a 5.8meg (~12%) increase in text size when framepointers are enabled. If frame pointers for leaf functions are omitted, there is a small 0.16% savings only so not worth the effort.

Using nm --defined-only -S, we can extract 585708 symbols in (a) and 585701 in (c).  Matching up symbol names and summing up the deltas, we can find 2307896 byte (~2.3meg) increase in generated symbols. This does NOT account for all of the text increase which is closer to 5.8meg...but 2.3meg is still enough to be prohibitive so I did not investigate this divergence.  (Note, when I looked at the elf headers, none of the other segments shifted in size significantly.)

Histogramming the symbol size difference, there is a normal-ish distribution of size changes (with framepointer enabled, some symbols _did_ get significantly smaller) but the majority of symbols grew from between 2 to ~40 bytes. Here is the center of the histogram:

detla: -4, num: 396 tot_kb: -2
detla: -2, num: 327 tot_kb: -1
detla: 0, num: 239327 tot_kb: 0
detla: 2, num: 154767 tot_kb: 302
detla: 4, num: 39760 tot_kb: 155
detla: 6, num: 8172 tot_kb: 47
detla: 8, num: 39659 tot_kb: 309
detla: 10, num: 26723 tot_kb: 260
detla: 12, num: 36989 tot_kb: 433
detla: 14, num: 13716 tot_kb: 187
detla: 16, num: 11425 tot_kb: 178
detla: 18, num: 2285 tot_kb: 40
detla: 20, num: 3825 tot_kb: 74
detla: 22, num: 1219 tot_kb: 26
detla: 24, num: 1734 tot_kb: 40
detla: 26, num: 315 tot_kb: 7
detla: 28, num: 988 tot_kb: 27
detla: 30, num: 191 tot_kb: 5
detla: 32, num: 674 tot_kb: 21
detla: 34, num: 112 tot_kb: 3
detla: 36, num: 449 tot_kb: 15
detla: 38, num: 60 tot_kb: 2
detla: 40, num: 325 tot_kb: 12


These numbers start to make sense if you think about arm/thumb calling conventions.  In ARM mode, r11 is the fp. In Thumb mode, r7 is fp. (see 6.2.5 of ARM-THUMB Procedure Call Standard). There is some flexibility on register choice in thumb mode, but it CANNOT be r11.

In all arm32 calls, the frame-pointer is callee saved. That means to get a framepointer usable for unwinding (note GCC does not do this right still), the prologue must do 2 things:
  (1) Push r7/r11 right before lr.
  (2) Adjust r7/r11 to point at stack location of prior value [this is where GCC does it wrong]
In the epilogue, the reverse must be done.

In ARM push/pop of multiple registers can be done in one opcode IFF the natural ordering of the registers is all you need (this constrait causes problems later). However, assuming the best case, where register allocation and pushing is unaffected by reserving r7 or r11 for fp, there still needs to be 1 extra instruction to adjust the fp. In Thumb mode, that's a 16-bit opcode (2 bytes) and in ARM, it's 4-bytes. Combined, this accounts for 302+155=457kb of increase.

The world gets more fun when going Thumb -> ARM. When transitioning to ARM mode, in addition to the steps above the fp needs to be moved from r7 to r11 and r11 needs to be spilled onto the stack. In the epilogue, r11 needs to be restored as well. Unfortunately, in Thumb mode, only r1-r7 may be pushed on to the stack with 16-bit opcodes; pushing/popping r11 requires 32-bit strb.w ldr.w instructions adding an addition 8 bytes to any function that transitions out of thumb mode. As a silver lining the 32-bit strb.w instruction as a "!" modifier that will set r11 to the correct framepointer value _without_ needing another operation. Anyways, this acccounts for the 10-byte deltas 260kb.

How frequently do we go from Thumb (likely small simple functions) to ARM? A lot. STL containers often create small wrapper functions (which are greate candidates for thumb mode) that will call a more complex constructor or destructor (esp with move semantics) that will be in ARM mode. Grepping the symbols for "tree.*eraseE" shows ~1000 such functions for just erase() instantiations. (Side note: these have nearly identical code and just tree::erase() worth about 91kb of text space which points to some de-templating optimization opportunity in libcxx). Many other functions in Chrome also have this behavior.

Why though does this transition sometimes cause a 12-byte penality? That's cause of the push/pop ordering problem listed earlier. 

For the framepointer to be usable for unwinding, it must be pushed at a constant offset fromt he link register. ARM procedure call in fact says it must be next to the link register. In ARM mode, this means the push instruction that stores lr can ONLY push registers r1-r7.  Even though registers r8 and higher could be encoded in one instruction (granted, it'd be a 32-bit instruction instead of 16 bits), the higher numbered registers would be pushed between r7 and lr making the framepointer useless.

This means if allocating r7 to the fp causes us to need r8 to be spilled, an extra (32-bit) instruction will need to be added to the prologue and epilogue, and an extra move will be needed to load the register (likely 2 bytes). Luckily, if r11 needed to be spilled for a Thumb -> ARM transition, both these push/pop costs will be merged. However, it does account for a 2-byte or 10-byte variance of size increases for various thumb functions. This can explain many of the 12 byte changes (433kb).

With that model, we can account for nearly 1.15Megs of increase.

I assume the 14 1nd 16 byte deltas are also some combination of the above with more register spills bewteen function calls which is another 360k or so.

Given this, I think enabling framepointers globally is a no-go.

I'd like to think through turning it on for some specialized build though. I thought we did something similar for ASAN where there was a custom build sent out to some subset of canary users?

Labels: -Restrict-View-Google
Unrestricting since the info is generally interesting and nothing materially sensitive is here.
Cc: laszio@chromium.org bccheng@chromium.org llozano@chromium.org
Cc: manojgupta@chromium.org
Cc: zhizhouy@chromium.org srhines@google.com
Luis asked me to take a look at this.

ajwong@ I am confused if the frame pointers actually work with the llvm generated code for thumb.
There is a comment from Evgenii in the upstream llvm bug. Is the comment about llvm generated code or GCC ?

https://bugs.llvm.org/show_bug.cgi?id=18505

Evgenii Stepanov 2017-12-06 13:51:18 PST:
Yes, but the new frame pointer does not point to the store location of the previous frame pointer. Looks like this can not be used for unwinding.

Parent::VirtualBig(int, BigStruct):
  sub sp, sp, #8
  push {r4, r5, r7, lr}
  add r7, sp, #0


also, it seems the CL that was reverted enabled frame pointers on Chrome OS too. 
That change was not reviewed by anyone on the Chrome OS side. 
Chrome OS currently does not have any use for frame pointers on ARM. So, it is hard to justify to re-enable frame pointers.
Please make sure to add someone from Chrome OS team (chromeos-toolchain@ in this case) for changes that affect Chrome OS.

@manojgupta: llvm framepointers work in thumb now for unwinding. I actually wrote code that tested this. Someone put in a patch for iOS a while ago that fixed it.  The note from Evegnii is just about gcc.

@llozano: Sorry for leaving the CrOS review off. Didn't know the etiquette on that. FYI, CrOS actually currently does have frame pointers enabled on ARM64. Do you know the correctness of the comment that says "ChromeOS generally prefers frame pointers, to support CWP."?  Arm32 seems to be the single chromeOS platform that does not have frame pointers enabled...
Thanks ajwong@. Its definitely good to know that frame pointers work in thumb.
ajwong@ #31. yes, I understand the comment may have misled you. 
ChromeOS Wide Profiling team(CWP) wanted frame pointers enabled but this did not work for a long time so we disabled it. I will check with CWP team about re-enabling.
And you are right to wonder about ARM32 vs ARM64. 
I thought the problem was because of using Thumb so ARM64 should be fine.
(BTW, CHromeOS does not yet use ARM64 for user land, only for the kernel. Which I think disables framepointers explicitly).


Cc: pasko@chromium.org
pasko@: Re: frame pointers and thumb, see #25.
BTW, release builds built with just 'enable_profiling = true' (i.e. frame pointers + thumb) don't produce any traces in heap profiles. I suspect there is some problem with unwinding. Adding 'arm_use_thumb = false' fixes that.


PS. There is a small change required to get frame pointers in thumb mode, I'll send a CL later.
dskiba@: Can you send me some repro steps?  I did my testing with an official build configuration so I'm surprised to see it breaking...
Steps are:

(1) Apply this: https://chromium-review.googlesource.com/c/chromium/src/+/898022

(2) Build with the attached GN config.
args.gn
258 bytes Download

Sign in to add a comment