Reland "Enable extracting unwind table on official builds without cha…

…nnel"

This reverts commit ef18e86.

Reason for revert: Make sure apk_merger.py works with and without the
unwind file, till downstream cl lands. Also rebase on:
https://chromium-review.googlesource.com/c/chromium/src/+/994545
and remove workaround for crbug/828528.

Original change's description:
> Revert "Enable extracting unwind table on official builds without channel"
>
> This reverts commit 16e808d.
>
> Reason for revert: Merge step still failing.
>
> Original change's description:
> > 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}
>
> TBR=boliu@chromium.org,dpranke@chromium.org,mariakhomenko@chromium.org,changwan@chromium.org,ssid@chromium.org,agrieve@chromium.org
>
> Change-Id: I0a96e213133b6cb21c36db365b7c72f0f4642c8e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 819888
> Bug: 828879
> Reviewed-on: https://chromium-review.googlesource.com/995697
> Reviewed-by: Anthony Berent <aberent@chromium.org>
> Commit-Queue: Anthony Berent <aberent@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#548066}

TBR=dpranke@chromium.org,mariakhomenko@chromium.org

Bug: 819888, 828879
Change-Id: I2eba81de32632bea90171ece4cba1a4144c55d25
Reviewed-on: https://chromium-review.googlesource.com/996272
Commit-Queue: Siddhartha S <ssid@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Changwan Ryu <changwan@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548249}

android_webview/tools/apk_merger.py

@@ -174,7 +174,9 @@ def MergeApk(args, tmp_apk, tmp_dir_32, tmp_dir_64):
   UnpackApk(args.apk_64bit, tmp_dir_64)
   UnpackApk(args.apk_32bit, tmp_dir_32)
 
-  ignores = ['META-INF', 'AndroidManifest.xml']
+  # TODO(ssid): unwind file should be included in monochrome apk once all the
+  # official builds start including the file. https://crbug.com/819888.
+  ignores = ['META-INF', 'AndroidManifest.xml', 'unwind_cfi']
   if args.ignore_classes_dex:
     ignores += ['classes.dex', 'classes2.dex']
   if args.debug:

base/BUILD.gn

@@ -1810,6 +1810,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/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/config/android/extract_unwind_tables.gni

@@ -31,10 +31,8 @@ template("unwind_table_asset") {
       "--dump_syms_path",
       rebase_path("$root_out_dir/dump_syms", root_build_dir),
     ]
-    deps = [
-      ":${invoker.library_target}",
-      "//third_party/breakpad:dump_syms",
-    ]
+    deps = invoker.deps
+    deps += [ "//third_party/breakpad:dump_syms" ]
   }
   android_assets(target_name) {
     if (defined(invoker.testonly)) {

chrome/android/BUILD.gn

@@ -1112,11 +1112,20 @@ template("chrome_public_apk_tmpl_shared") {
   }
 }
 
+# Unwind tables are added to only official builds (public_apk(s)) so that
+# developer builds are not affected.
+_add_unwind_tables_in_chrome_public_apk =
+    can_unwind_with_cfi_table && 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") {
@@ -1125,6 +1134,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",
   ]
@@ -1135,6 +1148,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
@@ -1175,6 +1192,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_chrome_public_apk
+  if (_add_unwind_tables_in_chrome_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,22 @@ 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
+      }
+
+      library_target = invoker.shared_library_for_unwind_asset
+      deps = invoker.shared_libraries
+    }
+  }
+
   android_apk(target_name) {
     forward_variables_from(invoker, "*")
     exclude_xxxhdpi = true
@@ -99,6 +117,11 @@ template("chrome_public_apk_tmpl") {
     }
     command_line_flags_file = "chrome-command-line"
     product_version_resources_dep = "//chrome/android:product_version_resources"
+
+    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', # 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",
+          ]
         }
       }