Enable extracting unwind table on official builds without channel

The original cl was here:
https://chromium-review.googlesource.com/c/chromium/src/+/990092
This CL fixes the following problems with the original CL:
1. The apk_merger script fails because the unwind tables were only added
   in 32-bit apk. The merger script expects all the files to be same and
   the ones different should be checked.
 1a. The resources.arsc is non-hermetic and ordering is affected by
     adding file to only one apk. As a workaround for crbug/828528,
     add an empty (valid) unwind table file to the 64 bit monochrome
     apk to make the resource.arsc consistent.
 1b. The merger script simply adds all the files in apk which are not
     same. To keep the script simple and functional, the unwind resource
     is renamed to unwind_cfi_32 and unwind_cfi_empty in respective
     builds and the app_merger is updated to specify this file is
     expected to be different and included. This causes an extra file
     (4 byte) in the merged apk.

2. The unwind tables were always generated for "libchrome.so" for all
   chrome apks. The different chrome_apk(s) have different shared
   libraries like libchromefortest, etc.. So, update the unwind asset to
   get unwind table for the right library for each apk. Only adds assets
   to *_public_apk(s).

3. The monochrome_apk_checker was failing because the unwind file
   included was different in chrome_apk and monochrome_apk. This CL adds
   the asset to all apk at the same time and adds exception for this
   file.

BUG=819888
TBR=dpranke@chromium.org

Change-Id: Ibceeeacc19fa424d519891b8c17e349ee6c2dfd6
Reviewed-on: https://chromium-review.googlesource.com/991236
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547993}

android_webview/tools/apk_merger.py

@@ -166,7 +166,7 @@ def GetSecondaryAbi(apk_zipfile, shared_library):
 def MergeApk(args, tmp_apk, tmp_dir_32, tmp_dir_64):
   # Expected files to copy from 32- to 64-bit APK together with whether to
   # compress within the .apk.
-  expected_files = {'snapshot_blob_32.bin': False}
+  expected_files = {'snapshot_blob_32.bin': False, 'unwind_cfi_32': False}
   if args.shared_library:
     expected_files[args.shared_library] = not args.uncompress_shared_libraries
 

base/BUILD.gn

@@ -1808,6 +1808,7 @@ buildflag_header("debugging_buildflags") {
     "ENABLE_PROFILING=$enable_profiling",
     "CAN_UNWIND_WITH_FRAME_POINTERS=$can_unwind_with_frame_pointers",
     "UNSAFE_DEVELOPER_BUILD=$is_unsafe_developer_build",
+    "CAN_UNWIND_WITH_CFI_TABLE=$can_unwind_with_cfi_table",
   ]
 }
 

base/trace_event/cfi_backtrace_android.cc

@@ -137,7 +137,7 @@ void CFIBacktraceAndroid::Initialize() {
   executable_start_addr_ = reinterpret_cast<uintptr_t>(&__executable_start);
 
   // This file name is defined by extract_unwind_tables.gni.
-  static constexpr char kCfiFileName[] = "assets/unwind_cfi";
+  static constexpr char kCfiFileName[] = "assets/unwind_cfi_32";
   MemoryMappedFile::Region cfi_region;
   int fd = base::android::OpenApkAsset(kCfiFileName, &cfi_region);
   if (fd < 0)

base/trace_event/heap_profiler_allocation_context_tracker.cc

@@ -15,6 +15,11 @@
 #include "base/threading/platform_thread.h"
 #include "base/threading/thread_local_storage.h"
 #include "base/trace_event/heap_profiler_allocation_context.h"
+#include "build/build_config.h"
+
+#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
+#include "base/trace_event/cfi_backtrace_android.h"
+#endif
 
 #if defined(OS_LINUX) || defined(OS_ANDROID)
 #include <sys/prctl.h>
@@ -214,20 +219,26 @@ bool AllocationContextTracker::GetContextSnapshot(AllocationContext* ctx) {
 // kMaxFrameCount + 1 frames, so that we know if there are more frames
 // than our backtrace capacity.
 #if !defined(OS_NACL)  // We don't build base/debug/stack_trace.cc for NaCl.
-#if BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
+#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
+        const void* frames[Backtrace::kMaxFrameCount + 1];
+        static_assert(arraysize(frames) >= Backtrace::kMaxFrameCount,
+                      "not requesting enough frames to fill Backtrace");
+        size_t frame_count = CFIBacktraceAndroid::GetInstance()->Unwind(
+            frames, arraysize(frames));
+#elif BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
         const void* frames[Backtrace::kMaxFrameCount + 1];
         static_assert(arraysize(frames) >= Backtrace::kMaxFrameCount,
                       "not requesting enough frames to fill Backtrace");
         size_t frame_count = debug::TraceStackFramePointers(
             frames, arraysize(frames),
             1 /* exclude this function from the trace */);
-#else   // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
+#else
         // Fall-back to capturing the stack with base::debug::StackTrace,
         // which is likely slower, but more reliable.
         base::debug::StackTrace stack_trace(Backtrace::kMaxFrameCount + 1);
         size_t frame_count = 0u;
         const void* const* frames = stack_trace.Addresses(&frame_count);
-#endif  // BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS)
+#endif
 
         // If there are too many frames, keep the ones furthest from main().
         size_t backtrace_capacity = backtrace_end - backtrace;

base/trace_event/memory_dump_manager.cc

@@ -41,8 +41,13 @@
 
 #if defined(OS_ANDROID)
 #include "base/trace_event/java_heap_dump_provider_android.h"
+
+#if BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
+#include "base/trace_event/cfi_backtrace_android.h"
 #endif
 
+#endif  // defined(OS_ANDROID)
+
 namespace base {
 namespace trace_event {
 
@@ -273,8 +278,15 @@ bool MemoryDumpManager::EnableHeapProfiling(HeapProfilingMode profiling_mode) {
       break;
 
     case kHeapProfilingModeNative:
-      // If we don't have frame pointers then native tracing falls-back to
-      // using base::debug::StackTrace, which may be slow.
+#if defined(OS_ANDROID) && BUILDFLAG(CAN_UNWIND_WITH_CFI_TABLE)
+    {
+      bool can_unwind =
+          CFIBacktraceAndroid::GetInstance()->can_unwind_stack_frames();
+      DCHECK(can_unwind);
+    }
+#endif
+      // If we don't have frame pointers and unwind tables then native tracing
+      // falls-back to using base::debug::StackTrace, which may be slow.
       AllocationContextTracker::SetCaptureMode(
           AllocationContextTracker::CaptureMode::NATIVE_STACK);
       break;

build/android/gyp/extract_unwind_tables.py

@@ -57,6 +57,9 @@
 Usage:
   extract_unwind_tables.py --input_path [root path to unstripped chrome.so]
       --output_path [output path] --dump_syms_path [path to dump_syms binary]
+  [OR]
+  extract_unwind_tables.py --generate-empty-tables
+      --output_path [output path] --dump_syms_path [path to dump_syms binary]
 """
 
 import argparse
@@ -264,18 +267,29 @@ def _ParseCfiData(sym_file, output_path):
 
 def main():
   parser = argparse.ArgumentParser()
-  parser.add_argument(
-      '--input_path', required=True,
-      help='The input path of the unstripped binary')
   parser.add_argument(
       '--output_path', required=True,
       help='The path of the output file')
   parser.add_argument(
       '--dump_syms_path', required=True,
       help='The path of the dump_syms binary')
 
+  group = parser.add_mutually_exclusive_group(required=True)
+  group.add_argument(
+      '--input_path', required=False,
+      help='The input path of the unstripped binary')
+  group.add_argument(
+      '--generate-empty-tables', required=False,
+      help='Generates an empty valid unwind table file.',
+      action="store_true")
+
   args = parser.parse_args()
 
+  if args.generate_empty_tables:
+    with open(args.output_path, 'wb') as out_file:
+      _WriteCfiData({}, out_file)
+    return 0
+
   with tempfile.NamedTemporaryFile() as sym_file:
     out = subprocess.call(
         ['./' +args.dump_syms_path, args.input_path], stdout=sym_file)

build/config/android/extract_unwind_tables.gni

@@ -3,10 +3,19 @@
 # found in the LICENSE file.
 
 import("//build/config/android/rules.gni")
+import("//build/config/compiler/compiler.gni")
 
 template("unwind_table_asset") {
   _unwind_action = "${target_name}__extract"
-  _asset_path = "${target_gen_dir}/${target_name}/unwind_cfi"
+
+  # Monochrome 64 bit apk needs to include an empty unwind file with different
+  # name so that the resource table matches when merging monochrome apk.
+  if (can_unwind_with_cfi_table) {
+    assert(current_cpu == "arm")
+    _asset_path = "${target_gen_dir}/${target_name}/unwind_cfi_32"
+  } else {
+    _asset_path = "${target_gen_dir}/${target_name}/unwind_cfi_empty"
+  }
 
   action(_unwind_action) {
     if (defined(invoker.testonly)) {
@@ -17,25 +26,32 @@ template("unwind_table_asset") {
     outputs = [
       _asset_path,
     ]
-    inputs = [
-      "$root_out_dir/lib.unstripped/$shlib_prefix${invoker.library_target}$shlib_extension",
-    ]
 
     args = [
-      "--input_path",
-      rebase_path(
-          "$root_out_dir/lib.unstripped/$shlib_prefix${invoker.library_target}$shlib_extension",
-          root_build_dir),
       "--output_path",
       rebase_path(_asset_path, root_build_dir),
       "--dump_syms_path",
       rebase_path("$root_out_dir/dump_syms", root_build_dir),
     ]
-    deps = [
-      ":${invoker.library_target}",
-      "//third_party/breakpad:dump_syms",
-    ]
+
+    if (can_unwind_with_cfi_table) {
+      _input_lib = "$root_out_dir/lib.unstripped/$shlib_prefix${invoker.library_target}$shlib_extension"
+      inputs = [
+        _input_lib,
+      ]
+
+      args += [
+        "--input_path",
+        rebase_path(_input_lib, root_build_dir),
+      ]
+    } else {
+      args += [ "--generate-empty-tables" ]
+    }
+
+    deps = invoker.deps
+    deps += [ "//third_party/breakpad:dump_syms" ]
   }
+
   android_assets(target_name) {
     if (defined(invoker.testonly)) {
       testonly = invoker.testonly

chrome/android/BUILD.gn

@@ -1092,11 +1092,24 @@ template("chrome_public_apk_tmpl_shared") {
   }
 }
 
+# Unwind tables are added to only official builds (public_apk(s)) so that
+# developer builds are not affected. In monochrome we add the asset file to both
+# 32 and 64 bit versions so that the resources.arsc is same, as workaround for
+# https://crbug.com/828528. The extract script assigns different names for these
+# versions.
+_add_unwind_tables_in_chrome_public_apk =
+    can_unwind_with_cfi_table && is_official_build
+_add_unwind_tables_in_monochrome_public_apk = is_official_build
+
 chrome_public_apk_tmpl_shared("chrome_public_apk") {
   android_manifest = chrome_public_android_manifest
   android_manifest_dep = ":chrome_public_android_manifest"
   apk_name = "ChromePublic"
   shared_libraries = [ ":libchrome" ]
+  add_unwind_tables_in_apk = _add_unwind_tables_in_chrome_public_apk
+  if (_add_unwind_tables_in_chrome_public_apk) {
+    shared_library_for_unwind_asset = "chrome"
+  }
 }
 
 chrome_public_apk_tmpl_shared("chrome_public_apk_for_test") {
@@ -1105,6 +1118,10 @@ chrome_public_apk_tmpl_shared("chrome_public_apk_for_test") {
   android_manifest_dep = ":chrome_public_android_manifest"
   apk_name = "ChromePublicForTest"
   shared_libraries = [ ":libchromefortest" ]
+  add_unwind_tables_in_apk = _add_unwind_tables_in_chrome_public_apk
+  if (_add_unwind_tables_in_chrome_public_apk) {
+    shared_library_for_unwind_asset = "chromefortest"
+  }
   deps = [
     "//chrome/browser/profiling_host:profiling_host_java_test_support",
   ]
@@ -1115,6 +1132,10 @@ chrome_public_apk_tmpl_shared("chrome_modern_public_apk") {
   android_manifest_dep = ":chrome_modern_public_android_manifest"
   apk_name = "ChromeModernPublic"
   shared_libraries = [ ":libchrome" ]
+  add_unwind_tables_in_apk = _add_unwind_tables_in_chrome_public_apk
+  if (_add_unwind_tables_in_chrome_public_apk) {
+    shared_library_for_unwind_asset = "chrome"
+  }
 
   if (!is_java_debug) {
     png_to_webp = true
@@ -1155,6 +1176,11 @@ monochrome_public_apk_tmpl("monochrome_public_apk") {
     "//chrome/android:chrome_java",
     "//chrome/android:class_register_java",
   ]
+
+  add_unwind_tables_in_apk = _add_unwind_tables_in_monochrome_public_apk
+  if (_add_unwind_tables_in_monochrome_public_apk && can_unwind_with_cfi_table) {
+    shared_library_for_unwind_asset = "monochrome"
+  }
 }
 
 chrome_public_apk_tmpl_shared("chrome_sync_shell_apk") {

chrome/android/chrome_public_apk_tmpl.gni

@@ -5,6 +5,8 @@
 import("//base/android/linker/config.gni")
 import("//build/config/android/rules.gni")
 import("//build/config/locales.gni")
+import("//build/config/android/extract_unwind_tables.gni")
+import("//build/config/compiler/compiler.gni")
 import("//chrome/common/features.gni")
 import("//third_party/leakcanary/config.gni")
 import("channel.gni")
@@ -24,6 +26,25 @@ default_chrome_public_jinja_variables = [
 ]
 
 template("chrome_public_apk_tmpl") {
+  # Adds unwind table asset to the chrome apk for the given library target. This
+  # is not part of generic apk assets target since it depends on the main shared
+  # library of the apk, to extract unwind tables.
+  if (defined(invoker.add_unwind_tables_in_apk) &&
+      invoker.add_unwind_tables_in_apk) {
+    _unwind_asset = "${target_name}_unwind_assets"
+    unwind_table_asset(_unwind_asset) {
+      if (defined(invoker.testonly)) {
+        testonly = invoker.testonly
+      }
+
+      # This value is only used when unwinding is possible.
+      if (can_unwind_with_cfi_table) {
+        library_target = invoker.shared_library_for_unwind_asset
+      }
+      deps = invoker.shared_libraries
+    }
+  }
+
   android_apk(target_name) {
     forward_variables_from(invoker, "*")
     exclude_xxxhdpi = true
@@ -98,6 +119,11 @@ template("chrome_public_apk_tmpl") {
           !use_lld && (use_chromium_linker || requires_sdk_api_level_23)
     }
     command_line_flags_file = "chrome-command-line"
+
+    if (defined(invoker.add_unwind_tables_in_apk) &&
+        invoker.add_unwind_tables_in_apk) {
+      deps += [ ":$_unwind_asset" ]
+    }
   }
 }
 

chrome/android/monochrome/scripts/monochrome_apk_checker.py

@@ -54,6 +54,7 @@ def BuildFileMatchRegex(*file_matchers):
     r'resources\.arsc',
     r'classes\.dex',
     r'res/.*\.xml', # Resource id isn't same
+    r'assets/unwind_cfi_32', # Generated from apk's shared library
      # All pak files except chrome_100_percent.pak are different
     r'assets/resources\.pak',
     r'assets/am\.pak',

testing/test.gni

@@ -110,6 +110,9 @@ template("test") {
         unwind_table_asset(_unwind_table_asset_name) {
           testonly = true
           library_target = _library_target
+          deps = [
+            ":$_library_target",
+          ]
         }
       }