Skip to content

Commit

Permalink
Android: Make enable_resource_whitelist_generation GN arg work when s…
Browse files Browse the repository at this point in the history
…etting to true

Also:
 * Default the arg to true for all release builds that contain debug info.
   (most bots set strip_debug_info=true though)
 * Add better failure messages for when no debug info is present.

Bug: 864878
Change-Id: Ied849d6e24a672b896c802be6df6c0958ecabfe0
Reviewed-on: https://chromium-review.googlesource.com/1142193
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588734}
  • Loading branch information
agrieve authored and Commit Bot committed Sep 5, 2018
1 parent 88a71c1 commit 90205ff
Show file tree
Hide file tree
Showing 8 changed files with 37 additions and 15 deletions.
6 changes: 0 additions & 6 deletions build/config/android/config.gni
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,3 @@ if (is_android || is_chromeos) {
android_libcpp_lib_dir = "${android_libcpp_root}/libs/${android_app_abi}"
}
}

declare_args() {
# Enables used resource whitelist generation. Set for official builds only
# as a large amount of build output is generated.
enable_resource_whitelist_generation = is_android && is_official_build
}
3 changes: 0 additions & 3 deletions build/config/compiler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ declare_args() {
# Allow projects that wish to stay on C++11 to override Chromium's default.
use_cxx11 = false

# Strip the debug info of symbols file in lib.unstripped to reduce size.
strip_debug_info = false

# Path to an AFDO profile to use while building with clang, if any. Empty
# implies none.
clang_sample_profile_path = ""
Expand Down
5 changes: 5 additions & 0 deletions build/config/compiler/compiler.gni
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ declare_args() {
# -1 means auto-set according to debug/release and platform.
symbol_level = -1

# Android-only: Strip the debug info of libraries within lib.unstripped to
# reduce size. As long as symbol_level > 0, this will still allow stacks to be
# symbolized.
strip_debug_info = false

# Compile in such a way as to enable profiling of the generated code. For
# example, don't omit the frame pointer and leave in symbols.
enable_profiling = false
Expand Down
1 change: 1 addition & 0 deletions build/toolchain/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/config/android/config.gni")
import("//build/config/clang/clang.gni")
import("//build/config/sysroot.gni") # Imports android/config.gni.
import("//build/toolchain/gcc_toolchain.gni")
Expand Down
17 changes: 16 additions & 1 deletion build/toolchain/gcc_toolchain.gni
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//build/config/android/config.gni")
import("//build/config/clang/clang.gni")
import("//build/config/compiler/compiler.gni")
import("//build/config/sanitizers/sanitizers.gni")
import("//build/config/v8_target_cpu.gni")
import("//build/toolchain/cc_wrapper.gni")
Expand All @@ -18,6 +18,21 @@ if (is_nacl) {
import("//build/config/nacl/config.gni")
}

declare_args() {
# Enables whitelist generation for IDR_ grit defines seen by the compiler.
# Currently works only on Android and enabled by default for release builds.
# Requires debug info, so disabled for symbol_level=0 & strip_debug_info=true.
enable_resource_whitelist_generation =
is_android && !is_debug &&
(is_official_build || (!strip_debug_info && symbol_level > 0))
}

# When the arg is set via args.gn, it applies to all toolchains. In order to not
# hit the assert in grit_rule.gni, explicitly disable for host toolchains.
if (is_linux && target_os == "android") {
enable_resource_whitelist_generation = false
}

# Path to the Clang static analysis wrapper script.
# REVIEWERS: can you suggest a better location for this?
# GN is really picky about dead stores of variables except at the global scope.
Expand Down
6 changes: 4 additions & 2 deletions tools/grit/grit/format/data_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,10 @@ def RePack(output_file, input_files, whitelist_file=None,
input_info_files = [filename + '.info' for filename in input_files]
whitelist = None
if whitelist_file:
whitelist = util.ReadFile(whitelist_file, util.RAW_TEXT).strip().split('\n')
whitelist = set(map(int, whitelist))
lines = util.ReadFile(whitelist_file, util.RAW_TEXT).strip().splitlines()
if not lines:
raise Exception('Whitelist file should not be empty')
whitelist = set(int(x) for x in lines)
inputs = [(p.resources, p.encoding) for p in input_data_packs]
resources, encoding = RePackFromDataPackStrings(
inputs, whitelist, suppress_removed_key_output)
Expand Down
8 changes: 5 additions & 3 deletions tools/grit/grit_rule.gni
Original file line number Diff line number Diff line change
Expand Up @@ -84,18 +84,20 @@
# # You can also put deps here if the grit source depends on generated
# # files.
# }
import("//build/config/android/config.gni")
import("//build/config/chrome_build.gni")
import("//build/config/compiler/compiler.gni")
import("//build/config/compute_inputs_for_analyze.gni")
import("//build/config/crypto.gni")
import("//build/config/features.gni")
import("//build/config/ui.gni")
import("//build/toolchain/gcc_toolchain.gni") # For enable_resource_whitelist_generation
import("//third_party/closure_compiler/closure_args.gni")

assert(
!enable_resource_whitelist_generation || (is_android && symbol_level > 0),
"resource whitelist generation only works on android with symbol_level > 0")
!enable_resource_whitelist_generation ||
(is_android && symbol_level > 0 && !strip_debug_info &&
!is_component_build),
"resource whitelist generation only works on non-component android builds with debug info enabled.")

grit_defines = []

Expand Down
6 changes: 6 additions & 0 deletions tools/resources/generate_resource_whitelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ def WriteResourceWhitelist(args):
readelf = subprocess.Popen(
['readelf', '-p', '.debug_str', args.input], stdout=subprocess.PIPE)
resource_ids = set()
output_exists = False
for line in readelf.stdout:
output_exists = True
# Read a line of the form " [ 123] WhitelistedResource<456>". We're
# only interested in the string, not the offset. We're also not interested
# in header lines.
Expand All @@ -38,6 +40,10 @@ def WriteResourceWhitelist(args):
resource_ids.add(int(s[len('WhitelistedResource<'):-len('>')-1]))
except ValueError:
continue
if not output_exists:
raise Exception('No debug info was dumpped. Ensure GN arg "symbol_level" '
'!= 0 and that the file is not stripped.')

for id in sorted(resource_ids):
args.output.write(str(id) + '\n')
exit_code = readelf.wait()
Expand Down

0 comments on commit 90205ff

Please sign in to comment.