grit: Replace current resource whitelisting implementation with a bet…

…ter one. The new implementation works by embedding debug info in object files instead of parsing compiler warning output. As a result, our official builds are much less noisy. See the new header file ui/base/resource/whitelist.h for details on how it works. It is also simpler because it does not rely on wrapper scripts for collecting the set of whitelisted resource ids. As a result we no longer need, and can remove, the wrapper scripts for the compiler and ar. It is also theoretically more precise because it only whitelists resource ids mentioned in archive members that were chosen by the linker, although I didn't see an improvement in monochrome. It may be possible to gain even more precision by extending the compiler (see comments on bug) but first we'd need to investigate whether that would be worth it. Bug: 648475, 684788 Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet Change-Id: I12d83a8aaffa42c3afcdcc834f7dd7536b7a9248 Reviewed-on: https://chromium-review.googlesource.com/1174012 Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Reviewed-by: Nico Weber <thakis@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Steven Holte <holte@chromium.org> Commit-Queue: Peter Collingbourne <pcc@chromium.org> Cr-Commit-Position: refs/heads/master@{#584129}
hylschwy · Aug 17, 2018 · f26ebd6 · f26ebd6
1 parent a8ae8cd
commit f26ebd6
Show file tree

Hide file tree

Showing 27 changed files with 170 additions and 426 deletions.
diff --git a/android_webview/BUILD.gn b/android_webview/BUILD.gn
@@ -443,7 +443,7 @@ if (enable_resource_whitelist_generation) {
     deps = [
       ":libwebviewchromium",
     ]
-    input = "$root_out_dir/libwebviewchromium$shlib_extension.whitelist"
+    input = "$root_out_dir/lib.unstripped/libwebviewchromium$shlib_extension"
     output = system_webview_pak_whitelist
   }
 }

diff --git a/build/android/resource_sizes.py b/build/android/resource_sizes.py
@@ -99,9 +99,9 @@ def _PatchedDecodeExtra(self):
 }
 _DUMP_STATIC_INITIALIZERS_PATH = os.path.join(
     host_paths.DIR_SOURCE_ROOT, 'tools', 'linux', 'dump-static-initializers.py')
-# Pragma exists when enable_resource_whitelist_generation=true.
-_RC_HEADER_RE = re.compile(
-    r'^#define (?P<name>\w+) (?:_Pragma\(.*?\) )?(?P<id>\d+)$')
+# Macro definitions look like (something, 123) when
+# enable_resource_whitelist_generation=true.
+_RC_HEADER_RE = re.compile(r'^#define (?P<name>\w+).* (?P<id>\d+)\)?$')
 _RE_NON_LANGUAGE_PAK = re.compile(r'^assets/.*(resources|percent)\.pak$')
 _RE_COMPRESSED_LANGUAGE_PAK = re.compile(
     r'\.lpak$|^assets/(?!stored-locales/).*(?!resources|percent)\.pak$')

diff --git a/build/toolchain/gcc_ar_wrapper.py b/build/toolchain/gcc_ar_wrapper.py
diff --git a/build/toolchain/gcc_compile_wrapper.py b/build/toolchain/gcc_compile_wrapper.py
diff --git a/build/toolchain/gcc_solink_wrapper.py b/build/toolchain/gcc_solink_wrapper.py
@@ -86,9 +86,6 @@ def main():
                       required=True,
                       help='Final output shared object file',
                       metavar='FILE')
-  parser.add_argument('--resource-whitelist',
-                      help='Merge all resource whitelists into a single file.',
-                      metavar='PATH')
   parser.add_argument('command', nargs='+',
                       help='Linking command')
   args = parser.parse_args()
@@ -97,11 +94,6 @@ def main():
   fast_env = dict(os.environ)
   fast_env['LC_ALL'] = 'C'
 
-  if args.resource_whitelist:
-    whitelist_candidates = wrapper_utils.ResolveRspLinks(args.command)
-    wrapper_utils.CombineResourceWhitelists(
-        whitelist_candidates, args.resource_whitelist)
-
   # First, run the actual link.
   command = wrapper_utils.CommandToRun(args.command)
   result = wrapper_utils.RunLinkWithOptionalMapFile(command, env=fast_env,

diff --git a/build/toolchain/gcc_toolchain.gni b/build/toolchain/gcc_toolchain.gni
@@ -285,16 +285,8 @@ template("gcc_toolchain") {
       depsformat = "gcc"
       description = "CC {{output}}"
       outputs = [
-        # The whitelist file is also an output, but ninja does not
-        # currently support multiple outputs for tool("cc").
         "$object_subdir/{{source_name_part}}.o",
       ]
-      if (enable_resource_whitelist_generation) {
-        compile_wrapper =
-            rebase_path("//build/toolchain/gcc_compile_wrapper.py",
-                        root_build_dir)
-        command = "$python_path \"$compile_wrapper\" --resource-whitelist=\"{{output}}.whitelist\" $command"
-      }
     }
 
     tool("cxx") {
@@ -303,16 +295,8 @@ template("gcc_toolchain") {
       depsformat = "gcc"
       description = "CXX {{output}}"
       outputs = [
-        # The whitelist file is also an output, but ninja does not
-        # currently support multiple outputs for tool("cxx").
         "$object_subdir/{{source_name_part}}.o",
       ]
-      if (enable_resource_whitelist_generation) {
-        compile_wrapper =
-            rebase_path("//build/toolchain/gcc_compile_wrapper.py",
-                        root_build_dir)
-        command = "$python_path \"$compile_wrapper\" --resource-whitelist=\"{{output}}.whitelist\" $command"
-      }
     }
 
     tool("asm") {
@@ -327,30 +311,29 @@ template("gcc_toolchain") {
     }
 
     tool("alink") {
-      rspfile = "{{output}}.rsp"
-      whitelist_flag = " "
-      if (enable_resource_whitelist_generation) {
-        whitelist_flag = " --resource-whitelist=\"{{output}}.whitelist\""
+      if (current_os == "aix") {
+        # AIX does not support either -D (deterministic output) or response
+        # files.
+        command = "$ar -X64 {{arflags}} -r -c -s {{output}} {{inputs}}"
+      } else {
+        rspfile = "{{output}}.rsp"
+        rspfile_content = "{{inputs}}"
+        command = "\"$ar\" {{arflags}} -r -c -s -D {{output}} @\"$rspfile\""
       }
 
-      # This needs a Python script to avoid using simple sh features in this
-      # command, in case the host does not use a POSIX shell (e.g. compiling
-      # POSIX-like toolchains such as NaCl on Windows).
-      ar_wrapper =
-          rebase_path("//build/toolchain/gcc_ar_wrapper.py", root_build_dir)
-
-      if (current_os == "aix") {
-        # We use slightly different arflags for AIX.
-        extra_arflags = "-r -c -s"
+      # Remove the output file first so that ar doesn't try to modify the
+      # existing file.
+      if (host_os == "win") {
+        tool_wrapper_path =
+            rebase_path("//build/toolchain/win/tool_wrapper.py", root_build_dir)
+        command = "cmd /c $python_path $tool_wrapper_path delete-file {{output}} && $command"
       } else {
-        extra_arflags = "-r -c -s -D"
+        command = "rm -f {{output}} && $command"
       }
 
       # Almost all targets build with //build/config/compiler:thin_archive which
       # adds -T to arflags.
-      command = "$python_path \"$ar_wrapper\"$whitelist_flag --output={{output}} --ar=\"$ar\" \"{{arflags}} $extra_arflags\" @\"$rspfile\""
       description = "AR {{output}}"
-      rspfile_content = "{{inputs}}"
       outputs = [
         "{{output_dir}}/{{target_output_name}}{{output_extension}}",
       ]
@@ -367,11 +350,6 @@ template("gcc_toolchain") {
       sofile = "{{output_dir}}/$soname"  # Possibly including toolchain dir.
       rspfile = sofile + ".rsp"
       pool = "//build/toolchain:link_pool($default_toolchain)"
-      whitelist_flag = " "
-      if (enable_resource_whitelist_generation) {
-        whitelist_file = "$sofile.whitelist"
-        whitelist_flag = " --resource-whitelist=\"$whitelist_file\""
-      }
 
       if (defined(invoker.strip)) {
         unstripped_sofile = "{{root_out_dir}}/lib.unstripped/$soname"
@@ -408,7 +386,7 @@ template("gcc_toolchain") {
       # The host might not have a POSIX shell and utilities (e.g. Windows).
       solink_wrapper =
           rebase_path("//build/toolchain/gcc_solink_wrapper.py", root_build_dir)
-      command = "$python_path \"$solink_wrapper\" --readelf=\"$readelf\" --nm=\"$nm\" $strip_switch--sofile=\"$unstripped_sofile\" --tocfile=\"$tocfile\"$map_switch --output=\"$sofile\"$whitelist_flag -- $link_command"
+      command = "$python_path \"$solink_wrapper\" --readelf=\"$readelf\" --nm=\"$nm\" $strip_switch--sofile=\"$unstripped_sofile\" --tocfile=\"$tocfile\"$map_switch --output=\"$sofile\" -- $link_command"
 
       if (target_cpu == "mipsel" && is_component_build && is_android) {
         rspfile_content = "-Wl,--start-group -Wl,--whole-archive {{inputs}} {{solibs}} -Wl,--no-whole-archive $solink_libs_section_prefix {{libs}} $solink_libs_section_postfix -Wl,--end-group"
@@ -441,9 +419,6 @@ template("gcc_toolchain") {
         sofile,
         tocfile,
       ]
-      if (enable_resource_whitelist_generation) {
-        outputs += [ whitelist_file ]
-      }
       if (sofile != unstripped_sofile) {
         outputs += [ unstripped_sofile ]
         if (defined(invoker.use_unstripped_as_runtime_outputs) &&

diff --git a/build/toolchain/nacl/BUILD.gn b/build/toolchain/nacl/BUILD.gn
@@ -68,7 +68,7 @@ template("pnacl_toolchain") {
 
     cc = compiler_scriptprefix + toolprefix + "clang" + scriptsuffix
     cxx = compiler_scriptprefix + toolprefix + "clang++" + scriptsuffix
-    ar = scriptprefix + toolprefix + "ar" + scriptsuffix
+    ar = toolprefix + "ar" + scriptsuffix
     readelf = scriptprefix + toolprefix + "readelf" + scriptsuffix
     nm = scriptprefix + toolprefix + "nm" + scriptsuffix
     if (defined(invoker.strip)) {

diff --git a/build/toolchain/wrapper_utils.py b/build/toolchain/wrapper_utils.py
@@ -14,7 +14,6 @@
 import threading
 
 _BAT_PREFIX = 'cmd /c call '
-_WHITELIST_RE = re.compile('whitelisted_resource_(?P<resource_id>[0-9]+)')
 
 
 def _GzipThenDelete(src_path, dest_path):
@@ -82,62 +81,6 @@ def RunLinkWithOptionalMapFile(command, env=None, map_file=None):
   return result
 
 
-def ResolveRspLinks(inputs):
-  """Return a list of files contained in a response file.
-
-  Args:
-    inputs: A command containing rsp files.
-
-  Returns:
-    A set containing the rsp file content."""
-  rspfiles = [a[1:] for a in inputs if a.startswith('@')]
-  resolved = set()
-  for rspfile in rspfiles:
-    with open(rspfile, 'r') as f:
-      resolved.update(shlex.split(f.read()))
-
-  return resolved
-
-
-def CombineResourceWhitelists(whitelist_candidates, outfile):
-  """Combines all whitelists for a resource file into a single whitelist.
-
-  Args:
-    whitelist_candidates: List of paths to rsp files containing all targets.
-    outfile: Path to save the combined whitelist.
-  """
-  whitelists = ('%s.whitelist' % candidate for candidate in whitelist_candidates
-                if os.path.exists('%s.whitelist' % candidate))
-
-  resources = set()
-  for whitelist in whitelists:
-    with open(whitelist, 'r') as f:
-      resources.update(f.readlines())
-
-  with open(outfile, 'w') as f:
-    f.writelines(resources)
-
-
-def ExtractResourceIdsFromPragmaWarnings(text):
-  """Returns set of resource IDs that are inside unknown pragma warnings.
-
-  Args:
-    text: The text that will be scanned for unknown pragma warnings.
-
-  Returns:
-    A set containing integers representing resource IDs.
-  """
-  used_resources = set()
-  lines = text.splitlines()
-  for ln in lines:
-    match = _WHITELIST_RE.search(ln)
-    if match:
-      resource_id = int(match.group('resource_id'))
-      used_resources.add(resource_id)
-
-  return used_resources
-
-
 def CaptureCommandStderr(command, env=None):
   """Returns the stderr of a command.
 

diff --git a/chrome/BUILD.gn b/chrome/BUILD.gn
@@ -1826,7 +1826,7 @@ if (enable_resource_whitelist_generation) {
     deps = [
       "//chrome/android:libchrome",
     ]
-    input = "$root_out_dir/libchrome$shlib_extension.whitelist"
+    input = "$root_out_dir/lib.unstripped/libchrome$shlib_extension"
     output = chrome_resource_whitelist
   }
 }

diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
@@ -1026,7 +1026,7 @@ if (current_toolchain == default_toolchain) {
       ]
 
       input = get_label_info(deps[0], "root_out_dir") +
-              "/libmonochrome$shlib_extension.whitelist"
+              "/lib.unstripped/libmonochrome$shlib_extension"
       output = monochrome_resource_whitelist
     }
 

diff --git a/chrome/browser/ui/webui/settings/about_handler.cc b/chrome/browser/ui/webui/settings/about_handler.cc
@@ -284,11 +284,9 @@ AboutHandler* AboutHandler::Create(content::WebUIDataSource* html_source,
                                         ? IDS_VERSION_UI_OFFICIAL
                                         : IDS_VERSION_UI_UNOFFICIAL),
           base::UTF8ToUTF16(chrome::GetChannelName()),
-#if defined(ARCH_CPU_64_BITS)
-          l10n_util::GetStringUTF16(IDS_VERSION_UI_64BIT)));
-#else
-          l10n_util::GetStringUTF16(IDS_VERSION_UI_32BIT)));
-#endif
+          l10n_util::GetStringUTF16(sizeof(void*) == 8
+                                        ? IDS_VERSION_UI_64BIT
+                                        : IDS_VERSION_UI_32BIT)));
 
   html_source->AddString(
       "aboutProductCopyright",