To clarify, ffmpeg only removes "default_stack_frames" on posix. It's not immediately clear to me how it's removing common_optimize_on_cflags on windows. It looks like it's adding back in at https://cs.chromium.org/chromium/src/third_party/ffmpeg/BUILD.gn?type=cs&q=optimize_no_wpo+package:%5Echromium$&l=224.

+ dalecurtis, ffmpeg owner.

May not be necessary anymore; it's only been used in the past when compilation would fail without it; e.g., some inline assembly functions run out of registers. So if it's being removed and compilation works we can probably ditch it.

FYI I also filed issue 699863 for cleaning up base/debug/stack_trace.*, in particular by moving the frame-pointer based unwinding logic out, since it's not generally applicable.

> 1) This makes the assumption that all Chrome code is compiled either with, or without frame pointers. This assumption is incorrect.
Also:
> 1b) IIRC JIT-ed code emitted by (one of the four) V8 compiler does not produce frame pointers. 
> 1c) when we call a third_party library, there is no way we can control whether that has or not frame pointers.

Not sure where I'm going off-topic, but *I think* that the safest choice in the long term w.r.t behavior of StackTrace() is:
 - Give HAVE_TRACE_STACK_FRAME_POINTERS the semantic "code has generally frame pointers, but we have to be ready to deal with holes"
 - Have the frame-pointer unwinding code in base/debug be able to do bunny hops by detecting when the frame pointer chain is broken, and using the stack scanner to recover it. Depending on how this is implemented, it might mean that we would not emit any addresses in the unwinder for the frames that don't have a frame pointer. Still we would be able to jump to the last frame which had a frame pointer and recover the unwinding (which is fine IMHO).
IIRC dskiba@ did a lot of heavy lifting here, and what I described already works on Android (building with frame pointer and NOT in thumb mode). The trick is that the frame layout (specifically the distance on the stack between the saved frame pointer and the return address) is far away from being standardized.
So, doing this reliably requires a bit of research in compiler & ABIs (example of inconsistency on arm https://bugs.llvm.org/show_bug.cgi?id=18505).

>  Since frame pointer emission varies TU by TU, this can potentially cause ODR violations.
Uh? I am a bit lost here, how is -fomit, -fno-omit related with ODR violation? If you are in the situation where two linker unit define the same symbol, that's already an ODR violation, even if they both agree on the omit/no-omit frame pointers.

> Here's what we need to do:
+1 to the idea of making the config and build files sane (just, as said above, keep in mind you'll always have to deal with arbitrary lack of frame pointers).

> - Give HAVE_TRACE_STACK_FRAME_POINTERS the semantic "code has generally frame
> pointers, but we have to be ready to deal with holes"

I think that the concept of HAVE_TRACE_STACK_FRAME_POINTERS is fundamentally broken, and we should remove it altogether. Even if we’re in control of our entire build and can reach the point where every single translation unit agrees on whether stack frames have frame pointers, we can’t control all of the code that allocates frames on our stacks. We call into system libraries, and some of them call us back. Third-party modules load themselves into our processes and find ways to run on our threads. We can’t control whether system libraries or third-party modules use frame pointers.

Since we already have to be able to deal with a mix of frame pointers and no frame pointers, this macro is nonsense.

It sounds like Erik’s points (6) and (7) are pushing in the direction of removing it entirely. I agree with this.

> > Since frame pointer emission varies TU by TU, this can potentially cause ODR
> > violations.
>
> Uh? I am a bit lost here, how is -fomit, -fno-omit related with ODR violation?
> If you are in the situation where two linker unit define the same symbol,
> that's already an ODR violation, even if they both agree on the omit/no-omit
> frame pointers.

Presumably this could happen if a header provided different implementations of an inline function based on the value of HAVE_STACK_TRACE_FRAME_POINTERS.

So, there are two concrete cases today for HAVE_TRACE_STACK_FRAME_POINTERS,

1. In the Android JNI generator, if we see that the rest of the code is build with frame pointers, we do the extra work for maintaining the frame pointer chain to be able to unwind JNI call frames.

2. when starting chrome with --enable-heap-profiling=native, if we know that there is no way that we will be able to unwind the stack, we just tell the user: "Save your time, there is no way this will work".

In essence today that macro means "Using frame pointer unwinding will have good chances of getting a reasonable stacktraces. still you can have holes because of the cases  discussed above".
Maybe the name of the macro could be reworked to reflect this. My only concern is: if we remove it, how do we deal with the cases above?

Why do things have to be doomed without frame pointers? There’s CFI (but we strip it on Android?), and we can save unwind info according to whatever convention we choose in JITted code.

It may not be high priority work right now, but I think it’d be good to eventually be agnostic as to whether the “probably uses frame pointers sometimes” bit is present.

> There’s CFI (but we strip it on Android?)
As Erik pointed out in the past, we strip that in official bulds android, linux and OSX.

https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?rcl=e6b2fb315a37379020c437f9e28d7b921e273410&l=65


> and we can save unwind info according to whatever convention we choose in JITted code.
not sure it's realistic to ask v8 folks "hey, can you please emit this super crazy CFI for our heap profiler?". Also, we have no powerz on Android (at least the versions that have shipped)

(Sorry for the late reply, I'm traveling now.)

Let me explain what I meant HAVE_TRACE_STACK_FRAME_POINTERS to be:

It says whether TraceStackFramePointers and ScopedStackFrameLinker support the current platform (arch / bitness / etc.)

That's it. I.e. if HAVE_TRACE_STACK_FRAME_POINTERS is true, you have theoretical possibility of unwinding using frame pointers on the current platform. For example for Windows it should always be false in 64-bit mode, because fps are not possible there even in theory. And that's why it's false in thumb builds - because both GCC and Clang are broken and emit garbage fps.

Next question, whether frame pointers are actually there, is answered by looking at BUILDFLAG(ENABLE_PROFILING) - because at the time native heap profiling involved special (profiling) build.

And since BUILDFLAG(ENABLE_PROFILING) means depending on debugging_flags.h (which is fragile), all checks for BUILDFLAG(ENABLE_PROFILING) happen only in base/ .cc files only.

That's why native heap profiling CHECKs if you try to enable it outside of BUILDFLAG(ENABLE_PROFILING), originally I tried to conditionally define AllocationContextTracker::CaptureMode::NATIVE_STACK member, but that was flaky because of debugging_flags.h.

Regarding linking frame pointers in JNI generator: it really improves stack quality on Android. However, the way it works is tricky - since I didn't want to depend on debugging_flags.h, I pass enable_profiling build flag directly to jni_generator.py, which emits additional stuff for profiling builds. That additional stuff is just macros, which are expanded into code only when HAVE_TRACE_STACK_FRAME_POINTERS is set (i.e. we support fp-based unwinding and hence fp stack linking). And to complete the look, relevant code in jni_android.cc is compiled only if BUILDFLAG(ENABLE_PROFILING).


Now we have Windows (no frame pointers at all), and we want to use native profiling with release builds (some platforms have fps even in release builds).

Obvious choice is to introduce a replacement for BUILDFLAG(ENABLE_PROFILING), which would reflect whether frame pointers are currently enabled for the current build, and then change several things:

1. Use the macro (together with HAVE_TRACE_STACK_FRAME_POINTERS) in AllocationContextTracker to decide whether to unwind with TraceStackFramePointers or StackFrame::Addresses.
2. Remove CHECK for native heap profiler mode from MemoryDumpManager (since now AllocationContextTracker always works)
3. Don't touch jni_android.h/cc (i.e. continue to link frame pointers only in profiling builds)

Given that, I think that your CL does too much (I'll comment there).

FWIW, as noted on the revert CL for my heap profiling cleanup, including
build-flags headers from headers is fine, it's just that the header
generator rule isn't quite right - should be straightforward to fix.

Thanks dskiba for providing background on the original intention of many of these macros. This helps clarify the situation a lot.

The default compiler options will either cause TUs to emit frame pointers, or not. We can provide a user hint [e.g. ENABLE_PROFILING] but that's only a hint. e.g. ENABLE_PROFILING on 64-bit Windows clearly has no effect. Furthermore, the name of the flag does not accurately describe the effect. Unifying under something like ENABLED_FRAME_POINTERS seems like a reasonable way forward. 

[As mentioned earlier in this thread, the concept of ENABLED_FRAME_POINTERS is still broken - see c#6.]

There is currently a bunch of preprocessor logic to set the value for HAVE_TRACE_STACK_FRAME_POINTERS, which  controls whether or not we define TraceStackFramePointers and ScopedStackFrameLinker. After the unification of ENABLED_FRAME_POINTERS described above, the only conditional left over is (!THUMB), since we still can't reliably walk stacks with frame pointers on thumb architectures. 

So now we get two macros:
1) ENABLED_FRAME_POINTERS - do we expect most frames to have frame pointers.
2) CAN_UNWIND_WITH_FRAME_POINTERS - if all frames have frame pointers, can we unwind the stack by following frame pointers.

(2) could technically be a function, but then we might end up defining some functions we will never use, but don't get stripped. I personally don't care if this is a function or macro.

> (2) could technically be a function, but then we might end up defining some functions we will never use, but don't get stripped. I personally don't care if this is a function or macro.

Not too strong either, but perhaps making it a function might be more convenient as:
- If this is a macro generated by a buildflag_header, there seem to be issues (causing build errors) when including that in header files.
- If this is a new macro appended to *all* translation unit, that will go against brettw@ advice in https://groups.google.com/a/chromium.org/d/msg/chromium-dev/5c4ySpPsV14/JJfAeDdyAgAJ

So just a function defined in the .h should be okay.

>  but then we might end up defining some functions we will never use, but don't get stripped
Yeah but even then, that function should be really lot of #if and, at the end, just return true/false. If that is defined in a .h header, very likely will not generate any code at all (bust just const-propagate to the call sites, if any)

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

commit f7c8a0df4253be5271276dc7f3c1da5ce9b677c1
Author: erikchen <erikchen@chromium.org>
Date: Thu Apr 06 21:15:27 2017

Explicitly specify whether to emit frame pointers by default.

All platforms now specify whether to emit frame pointers by default, rather than
relying on default compiler options.

This CL moves the logic from config("default_stack_frames") into compiler.gni.
The former is actually the right place for the logic to live, but there exists
code that relies on whether a frame pointer is emitted by default. Right now,
that logic is being duplicated/guessed by the code in question. This CL at least
unifies the logic in a single location.

There current exists code that uses a preprocessor definition
HAVE_TRACE_STACK_FRAME_POINTERS. Despite the name, the code really wants to know
if most stacks can be unwound using stack pointers. I've renamed it to
CAN_UNWIND_WITH_FRAME_POINTERS. Arguably, any code that uses
CAN_UNWIND_WITH_FRAME_POINTERS is broken and should be removed, since it relies
on the assumption that all stacks will either have or not have frame pointers,
but that can vary TU by TU.

BUG=706116,  706654 

Review-Url: https://codereview.chromium.org/2782063005
Cr-Commit-Position: refs/heads/master@{#462622}

[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/BUILD.gn
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/android/jni_android.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/android/jni_android.h
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/debug/stack_trace.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/debug/stack_trace.h
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/debug/stack_trace_unittest.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/trace_event/heap_profiler_allocation_context_tracker.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/build/config/compiler/BUILD.gn
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/build/config/compiler/compiler.gni
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/chrome/browser/chromeos/libc_close_tracking.cc
[modify] https://crrev.com/f7c8a0df4253be5271276dc7f3c1da5ce9b677c1/tools/gn/bootstrap/bootstrap.py

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

commit bec44505ea17b6b8fbd366ab22ca724f3c448fe6
Author: wez <wez@chromium.org>
Date: Fri Apr 14 23:55:20 2017

Remove explicit -fomit_frame_pointer from ARM 32-bit builds.

-fomit_frame_pointer causes the assembly generated from SkEdge::setLine
to trigger a CPU errata in ARM A12/A17 devices, so we are temporarily
removing the flag until the toolchain can be fixed to avoid that.

Revert this when  issue 711784  is resolved.

BUG= 710131 ,  706654 ,  711784 

Review-Url: https://codereview.chromium.org/2820803003
Cr-Commit-Position: refs/heads/master@{#464832}

[modify] https://crrev.com/bec44505ea17b6b8fbd366ab22ca724f3c448fe6/build/config/compiler/BUILD.gn

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

commit 3dcba7c1facb5bb49535476185c5a396669d52f0
Author: Wez <wez@chromium.org>
Date: Mon Apr 17 22:20:54 2017

Remove explicit -fomit_frame_pointer from ARM 32-bit builds.

-fomit_frame_pointer causes the assembly generated from SkEdge::setLine
to trigger a CPU errata in ARM A12/A17 devices, so we are temporarily
removing the flag until the toolchain can be fixed to avoid that.

Revert this when  issue 711784  is resolved.

BUG= 710131 ,  706654 ,  711784 

Review-Url: https://codereview.chromium.org/2820803003
Cr-Commit-Position: refs/heads/master@{#464832}
(cherry picked from commit bec44505ea17b6b8fbd366ab22ca724f3c448fe6)

Review-Url: https://codereview.chromium.org/2819373002 .
Cr-Commit-Position: refs/branch-heads/3071@{#26}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/3dcba7c1facb5bb49535476185c5a396669d52f0/build/config/compiler/BUILD.gn

dianders: Assigning to you to get us some input on what the desired state is for frame pointers on non-Android ARM devices, in particular:
- Should we explicitly narrow down the non-Android configuration we're talking about to is_chromeos?
- Do we need to force enable frame pointers for some ChromeOS configs, e.g. CWP?

I think laszio@ is probably a better owner here, with input from gmx@chromium.org

Basically:

1. I agree that we should be explicit about frame pointer / no frame pointer.

2. The current change (https://codereview.chromium.org/2820803003) is definitely short term and should be reverted as soon as we fix  bug #711784 .

3. If CWP needs something special for frame pointers, we should make a clean change for that.

gmx: If the current expectation is that Chrome builds with the ChromeOS toolchain default, and that is to enable frame pointers, then it's easy for us to reflect that properly in Chrome's build configuration, but bear in mind that if ChromeOS later moves away from frame pointers then Chrome's configuration would need updating separately.

If you want us to force enable no-omit-frame-pointers for ChromeOS then let me know; that's a straightforward change for us to make. :)

wez: Relying on ChromeOS toolchain to enable frame pointers explicitly works for CWP. We have a close relationship with the toolchain team. If ChromeOS decides to move away from frame pointers as a whole, they probably discussed the decision with us and then disabling frame pointers for chrome makes sense as well.

We are also evaluating the use of LBR callstacks for profiling, which doesn't need frame pointers, but there is only a small fraction of devices that support that feature now.

Reading some of the latest comments, I am not sure if the explicit omit of frame pointers applies to x86-64 as well or only to ARM. Frame pointer unwinding on ARM32 is broken anyway, so we don't care if frame pointers are omitted on ARM32, but we care strongly about x86-64 and we also want them for ARM64.

* From a Chrome OS point of view, there are currently no arm64 builds of Chrome in use that I'm aware of.  In the future we might do that, but not today.
* I think today we might be the exact opposite of what you want.  ARM32 now has frame pointers to work around the A12/A17 bug and (I think everyone else has frame pointers off).

@21: I know that there are no ARM64 ChromeOS builds, but if they become available in the future, we would like frame pointers.

Hopefully the 2nd point can be fixed. At least have a mechanism to explicitly request frame pointers on ChromeOS.

Re #20 & #21:  Sorry, I found the comments tricky to parse, so here's my
attempt to summarize what we want, vs what we have:
1. As of crrev.com/2782063005 we ALWAYS explicitly configure omit-fp or
no-omit-fp, based on configuration, platform, etc.
2. [gmx' comment] We never want frame pointers under ARM32, since frame
unwinding is broken. Instead we currently have:
  a. My latest patch which hacks things so that we don't explicitly enable
nor disable frame pointers under ARM32, which we should revert ASAP.
  b. Otherwise, we would enable frame pointers for ARM32 only in Debug &
profiling builds, basically (see
https://cs.chromium.org/chromium/src/build/config/compiler/compiler.gni?l=84
).
  d. We have a special-case to disable frame pointer based unwinding under
ARM-Thumb. (see
https://cs.chromium.org/chromium/src/build/config/compiler/compiler.gni?l=93).
It sounds like you're saying that fp unwinding never works even under
non-Thumb, so perhaps that should be broadened?
3. [gmx comment] We always want frame pointers under ARM64.
  a. We always enable frame pointers under ARM64, even after Erik's and my
patches. It would be good to augment the comment to clarify why we want
that.

So I think the immediate change to make is to replace my quick-fix with one
which explicitly enables frame pointers even under ARM32, then we'll switch
that to explicitly disable them when we have Chicken Bits.  WDYT?

wez: Regarding your 2nd point, we don't care if frame pointers are enabled or not on ARM32. They can stay on, but we don't use them.

Our stack unwinds are always asynchronous, triggered from interrupts. Maybe there is something smarter that we can do, but we don't know if the code is Thumb or non-Thumb. Moreover, the callchains may cross code compiled in the two modes. As a result, we just don't use callstacks collected from ARM32.

There are no ARM64 ChromeOS builds, but like with x86-64, we would like to have the option to explicitly enable frame pointers from the ChromeOS toolchain in case that such builds become available in the future.

> Our stack unwinds are always asynchronous, triggered from interrupts. 
> Maybe there is something smarter that we can do, but we don't know if 
> the code is Thumb or non-Thumb

What's your exact logging mechanism?  In general in ARM devices "Thumb" mode is signaled by odd addresses.  ...so if you can make sure you log that you should be able to tell ARM vs. Thumb.

When you're crawling the stack, the even/odd in return addresses should also indicate ARM vs. Thumb modes.

The stack unwinds are performed by perf inside the kernel on event counter overflows.

I imagine that some addresses are odd, not all of them? That would require changes to the kernel, to decode a few instructions in each frame to find if any addresses are odd or not.

It's not the addresses of the instructions that are odd, but the addresses of the pointers.  AKA: to jump to ARM code located at 0x1000, you jump to 0x1000.  To jump to Thumb code located at 0x1000, you jump to 0x1001.  In both cases the code is located at 0x1000, but the lowest bit indicates the CPU mode we should be executing as.

Re #27-#28-#29, I think this is going OT, if you want to carry forward can you please fork another bug to avoid confusing this bug (sorry, just trying to keep order on this one, we are already at comment num 29)?

This bug isn't about how we could/should unwind the stack for CWP on arm32, is just about being explicit about frame pointers in the build files.
 

Copied & updated from  issue 711794 :

--
Discussing this offline with a few folks:
- Currently ChromeOS builds w/ frame pointers for x64, to support CWP.
- Erik's patch switched off frame pointers in Chrome except for specific configs, so we broke CWP.
- My patch only re-enables them for ARM/Thumb, so still leaves CWP broken.

Since we want CWP working, we should tweak the Chrome compiler.gni to accept an enable_frame_pointers argument, with the default setting based on the current enabled_frame_pointers logic, but have ChromeOS explicitly set that where necessary.
--

Why not just have them on unconditionally everywhere? Hooray for fewer flags and all that.

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

commit 38f02400540606a7c418f9fa5148d3a07fd8b4fa
Author: wez <wez@chromium.org>
Date: Thu Apr 20 18:41:09 2017

Enable frame pointers explicitly under ARM32.

This replaces the quick-fix in crrev.com/2820803003, to address issues
with ARM32 builds when frame pointers are disabled.

This CL explicitly enables frame pointers in ARM32 builds, and pulls out
the ARM32 and ARM64 special-cases to be handling separately from Debug,
profiling and sanitizer build configurations, for clarity.

BUG= 710131 ,  706654 ,  711784 

Review-Url: https://codereview.chromium.org/2829433003
Cr-Commit-Position: refs/heads/master@{#466080}

[modify] https://crrev.com/38f02400540606a7c418f9fa5148d3a07fd8b4fa/build/config/compiler/BUILD.gn
[modify] https://crrev.com/38f02400540606a7c418f9fa5148d3a07fd8b4fa/build/config/compiler/compiler.gni

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

commit 1e22c07d34fdb782f532155ae5fca2f048a5e2f3
Author: Wez <wez@chromium.org>
Date: Thu Apr 20 20:46:57 2017

Enable frame pointers explicitly under ARM32.

This replaces the quick-fix in crrev.com/2820803003, to address issues
with ARM32 builds when frame pointers are disabled.

This CL explicitly enables frame pointers in ARM32 builds, and pulls out
the ARM32 and ARM64 special-cases to be handling separately from Debug,
profiling and sanitizer build configurations, for clarity.

BUG= 710131 ,  706654 ,  711784 

Review-Url: https://codereview.chromium.org/2829433003
Cr-Commit-Position: refs/heads/master@{#466080}
(cherry picked from commit 38f02400540606a7c418f9fa5148d3a07fd8b4fa)

Review-Url: https://codereview.chromium.org/2831733005 .
Cr-Commit-Position: refs/branch-heads/3071@{#98}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/1e22c07d34fdb782f532155ae5fca2f048a5e2f3/build/config/compiler/BUILD.gn
[modify] https://crrev.com/1e22c07d34fdb782f532155ae5fca2f048a5e2f3/build/config/compiler/compiler.gni

32: Wez, are you still planning to add an enable_frame_pointers flag that can be set by an external toolchain?

Re #37: Yes, the patch in #34/#35 actually does that; my CL description is inadequate and fails to mention it - sorry.

wez: did you see comment 33?

Wez, thanks. I didn't realize that it's already done.

Yes, I did! It's definitely worth considering but tricky; we have several
use-cases for frame pointers and/or CFI, and different tradeoffs and
limitations around them.

Most notably:
- Some consumers want to be able to strip CFI to reduce binary size, so
want to have frame pointers to allow unwinding.
- Some consumers want frame pointers to support faster unwinding than with
CFI.
- Frame pointers impact performance on some architectures (essentially
64-bit vs 32-bit to a close approximation).
- Some platforms don't support them. :)

The aim in adding the argument was to allow the CrOS CWP use-case to be
supported without adding an explicit support_cwp parameter, and without
baking that requirement directly in to the GN.

Perhaps you're right though and we can bake this in on the Chromium side.

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

commit 8f2e1bde3eb4c84fcfe13962a2f6b48223526f9b
Author: wez <wez@chromium.org>
Date: Wed Apr 26 18:28:23 2017

Remove enable_frame_pointers and enable frame-pointers under ChromeOS.

As suggested by thakis@, this avoids adding yet another build argument
which can potentially be set inconsistently with other things.

Since ChromeOS needs frame-pointers enabled in x64 builds, to support
CWP, and temporarily in ARM/Thumb builds, to avoid CPU errata, we now
always enable them under ChromeOS. The ChromeOS toolchain already set
frame-pointers always on so this restores the pre-M59 behaviour.

BUG=BUG= 710131 ,  706654 ,  711784 

Review-Url: https://codereview.chromium.org/2845433002
Cr-Commit-Position: refs/heads/master@{#467386}

[modify] https://crrev.com/8f2e1bde3eb4c84fcfe13962a2f6b48223526f9b/build/config/compiler/compiler.gni

Erik: I think this is all working as intended now, following your cleanup, and the fixes.