Skip to content

Commit

Permalink
Revert "Android: Make enable_resource_whitelist_generation GN arg wor…
Browse files Browse the repository at this point in the history
…k when setting to true"

This reverts commit 90205ff.

Reason for revert: Breaks cronet build; see e.g. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8936264155658998032/+/steps/generate_build_files/0/stdout

Original change's description:
> Android: Make enable_resource_whitelist_generation GN arg work when setting 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}

TBR=torne@chromium.org,pcc@chromium.org,agrieve@chromium.org,lizeb@chromium.org

Change-Id: Ia7ca1c880cb1f2d5b8eda7f458cb0a3b2b8133ba
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 864878
Reviewed-on: https://chromium-review.googlesource.com/1206334
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#588844}
  • Loading branch information
sheepmaster authored and Commit Bot committed Sep 5, 2018
1 parent 5a1630f commit 3a9bbf8
Show file tree
Hide file tree
Showing 8 changed files with 15 additions and 37 deletions.
6 changes: 6 additions & 0 deletions build/config/android/config.gni
Original file line number Diff line number Diff line change
Expand Up @@ -355,3 +355,9 @@ 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: 3 additions & 0 deletions build/config/compiler/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ 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: 0 additions & 5 deletions build/config/compiler/compiler.gni
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ 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: 0 additions & 1 deletion build/toolchain/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
# 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: 1 addition & 16 deletions 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,21 +18,6 @@ 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: 2 additions & 4 deletions tools/grit/grit/format/data_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,10 +221,8 @@ def RePack(output_file, input_files, whitelist_file=None,
input_info_files = [filename + '.info' for filename in input_files]
whitelist = None
if whitelist_file:
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)
whitelist = util.ReadFile(whitelist_file, util.RAW_TEXT).strip().split('\n')
whitelist = set(map(int, whitelist))
inputs = [(p.resources, p.encoding) for p in input_data_packs]
resources, encoding = RePackFromDataPackStrings(
inputs, whitelist, suppress_removed_key_output)
Expand Down
8 changes: 3 additions & 5 deletions tools/grit/grit_rule.gni
Original file line number Diff line number Diff line change
Expand Up @@ -84,20 +84,18 @@
# # 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 && !strip_debug_info &&
!is_component_build),
"resource whitelist generation only works on non-component android builds with debug info enabled.")
!enable_resource_whitelist_generation || (is_android && symbol_level > 0),
"resource whitelist generation only works on android with symbol_level > 0")

grit_defines = []

Expand Down
6 changes: 0 additions & 6 deletions tools/resources/generate_resource_whitelist.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ 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 @@ -40,10 +38,6 @@ 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 3a9bbf8

Please sign in to comment.