From 8532ace0b1a5f61d0ba94e7584b78981d1d82e82 Mon Sep 17 00:00:00 2001 From: Paul Jensen Date: Tue, 19 Sep 2017 16:03:20 +0000 Subject: [PATCH] Fix x86 Android size regression from removing -mstackrealign -mstackrealign implied -fno-omit-frame-pointer which greatly shrank the unwind tables (.eh_frame section). Removing the flag (in crrev.com/499401) therefore improved code size but requires emission of full unwind tables, significantly impacting overall binary size. When building for x86 Android and optimizing for size and generating unwind tables, this fix enables frame pointers. Bug: 762629 Change-Id: I43515e6159a2aa1ef06e362c52f6525d2d67bf4f Reviewed-on: https://chromium-review.googlesource.com/657566 Reviewed-by: Wez Reviewed-by: Nico Weber Reviewed-by: Erik Chen Commit-Queue: Nico Weber Cr-Original-Commit-Position: refs/heads/master@{#502860} Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src Cr-Mirrored-Commit: a0c6c3f8dd59d7392d60ad37879ffc4490458704 --- config/compiler/BUILD.gn | 11 ----------- config/compiler/compiler.gni | 24 ++++++++++++++++++++++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/config/compiler/BUILD.gn b/config/compiler/BUILD.gn index 42bae2a14..b0763938d 100644 --- a/config/compiler/BUILD.gn +++ b/config/compiler/BUILD.gn @@ -69,17 +69,6 @@ declare_args() { msvs_xtree_patched = false } - # Omit unwind support in official builds to save space. - # We can use breakpad for these builds. - exclude_unwind_tables = (is_chrome_branded && is_official_build) || - (is_chromecast && !is_cast_desktop_build && !is_debug) - - # If true, optimize for size. Does not affect windows builds. - # Linux & Mac favor speed over size. - # TODO(brettw) it's weird that Mac and desktop Linux are different. We should - # explore favoring size over speed in this case as well. - optimize_for_size = is_android || is_ios - # Enable fatal linker warnings. Building Chromium with certain versions # of binutils can cause linker warning. # See: https://bugs.chromium.org/p/chromium/issues/detail?id=457359 diff --git a/config/compiler/compiler.gni b/config/compiler/compiler.gni index ea2f8133c..5dd2b1dd6 100644 --- a/config/compiler/compiler.gni +++ b/config/compiler/compiler.gni @@ -66,6 +66,20 @@ declare_args() { use_pic = true } +# Exclude unwind tables for official builds as unwinding can be done from stack +# dumps produced by Crashpad at a later time "offline" in the crash server. +# For unofficial (e.g. development) builds and non-Chrome branded (e.g. Cronet +# which doesn't use Crashpad, crbug.com/479283) builds it's useful to be able +# to unwind at runtime. +exclude_unwind_tables = (is_chrome_branded && is_official_build) || + (is_chromecast && !is_cast_desktop_build && !is_debug) + +# If true, optimize for size. Does not affect windows builds. +# Linux & Mac favor speed over size. +# TODO(brettw) it's weird that Mac and desktop Linux are different. We should +# explore favoring size over speed in this case as well. +optimize_for_size = is_android || is_ios + # Determine whether to enable or disable frame pointers, based on the platform # and build arguments. if (is_mac || is_ios) { @@ -84,8 +98,14 @@ if (is_mac || is_ios) { # there to avoid the unnecessary overhead. enable_frame_pointers = current_cpu != "arm" } else if (is_android) { - # Ensure that stacks from arm64 crash dumps are usable (crbug.com/391706). - enable_frame_pointers = current_cpu == "arm64" + enable_frame_pointers = + # Ensure that stacks from arm64 crash dumps are usable (crbug.com/391706). + current_cpu == "arm64" || + # For x86 Android, unwind tables are huge without frame pointers + # (crbug.com/762629). Enabling frame pointers grows the code size slightly + # but overall shrinks binaries considerably by avoiding huge unwind + # tables. + (current_cpu == "x86" && !exclude_unwind_tables && optimize_for_size) } else { # Explicitly ask for frame pointers, otherwise: # * Stacks may be missing for sanitizer and profiling builds.