android_crazy_linker: Disable r_brk() call for release builds.

Completely remove the ability to call r_brk() at runtime for
release builds. Normally, this is controlled at runtime by
calling crazy_set_debugger_support(false), which is actually
performed by the //base/android/linker/legacy_linker.cc.

However, due to yet-unsolved reasons, runtime crashes in
r_brk() still happen on Intel-based devices, even though
this function is now supposed to never be called.

This patch is an ugly hack that removes the ability to
call r_brk() entirely from the library's code, to unlock
the release.

A proper fix would understand what's really going on (e.g.
passing of dangling pointers in deferred task callbacks?)
and address that accordingly. Unfortunately, the issue is
really difficult to reproduce locally.

BUG=796938
R=agrieve@chromium.org,pasko@chromium.org,rmcilroy@chromium.org

Change-Id: I10a08fe3d551be39b6b9919420224b925b8be9a0
Reviewed-on: https://chromium-review.googlesource.com/889757Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: David Turner <digit@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532429}

base/android/linker/BUILD.gn

@@ -6,17 +6,6 @@ import("//build/config/android/config.gni")
 
 assert(is_android)
 
-declare_args() {
-  # Set this variable to true to enable GDB support in release builds.
-  #
-  # The default is to disable it to reduce the likelyhood of runtime crashes
-  # on devices that use machine translation (i.e. that run ARM binaries on
-  # x86 CPUs with a translation layer like Intel's Houdini). For full details
-  # see https://crbug.com/796938.
-  #
-  chromium_android_linker_enable_release_debugging = false
-}
-
 shared_library("chromium_android_linker") {
   sources = [
     "android_dlext.h",
@@ -26,11 +15,6 @@ shared_library("chromium_android_linker") {
     "linker_jni.h",
   ]
 
-  # Disable GDB support for release builds, unless explicitly wanted.
-  if (!is_debug && !chromium_android_linker_enable_release_debugging) {
-    defines = [ "LEGACY_LINKER_DISABLE_DEBUGGER_SUPPORT" ]
-  }
-
   # The NDK contains the crazy_linker here:
   #   '<(android_ndk_root)/crazy_linker.gyp:crazy_linker'
   # However, we use our own fork.  See bug 384700.

base/android/linker/legacy_linker_jni.cc

@@ -454,11 +454,6 @@ bool LegacyLinkerJNIInit(JavaVM* vm, JNIEnv* env) {
   if (!InitSDKVersionInfo(env))
     return false;
 
-// Disable debugger support when needed. See https://crbug.com/796938
-#if defined(LEGACY_LINKER_DISABLE_DEBUGGER_SUPPORT)
-  crazy_set_debugger_support(false);
-#endif
-
   // Register native methods.
   jclass linker_class;
   if (!InitClassReference(env,

third_party/android_crazy_linker/BUILD.gn

@@ -106,6 +106,14 @@ template("crazy_linker_library") {
         "UNIT_TESTS",
         "CRAZY_DEBUG=1",
       ]
+    } else {
+      # Disable r_brk() calls for release builds. This is a work-around for
+      # unexplained runtime crashes that happen on Intel-based Android
+      # devices that run ARM binaries through Houdini. For more details
+      # see https://crbug.com/796938
+      if (!is_debug) {
+        defines = [ "CRAZY_DISABLE_R_BRK" ]
+      }
     }
 
     public_configs = [ ":crazy_config" ]

third_party/android_crazy_linker/README.chromium

@@ -92,6 +92,6 @@ Local Modifications:
 - Add custom operator new(size_t) and operator delete(void*) in order to
   reduce the size of the final binaries (24 kiB on ARM, 64 kiB on AARCH4).
 
-- Allow client disable debugger support.
+- Define CRAZY_DISABLE_R_BRK to disable debugger support.
 
 - Enable integration and unit tests when building with Chromium.

third_party/android_crazy_linker/src/include/crazy_linker.h

@@ -35,13 +35,6 @@ extern "C" {
 // Maximum path length of a file in a zip file.
 const size_t kMaxFilePathLengthInZip = 256;
 
-// Enable or disable debugger support. Disabling debugger support makes
-// shared libraries loaded by the crazy linker invisible to GDB (though they
-// will still be visible in stack traces and Breakpad minidumps), but allows
-// rare runtime crashes on certain Android devices. For more details, see
-// https://crbug.com/796938.
-void crazy_set_debugger_support(bool enabled) _CRAZY_PUBLIC;
-
 // Status values returned by crazy linker functions to indicate
 // success or failure. They were chosen to match boolean values,
 // this allows one to test for failures with:

third_party/android_crazy_linker/src/src/crazy_linker_api.cpp

@@ -65,11 +65,6 @@ void crazy_context_t::ResetSearchPaths() {
 
 extern "C" {
 
-void crazy_set_debugger_support(bool enabled) {
-  ScopedGlobalLock lock;
-  Globals::GetRDebug()->SetDebuggerSupport(enabled);
-}
-
 void crazy_set_sdk_build_version(int sdk_build_version) {
   *Globals::GetSDKBuildVersion() = sdk_build_version;
 }

third_party/android_crazy_linker/src/src/crazy_linker_rdebug.cpp

@@ -213,7 +213,6 @@ bool RDebug::Init() {
   // The address of '_r_debug' is in the DT_DEBUG entry of the current
   // executable.
   init_ = true;
-  call_r_brk_ = true;
 
   size_t dynamic_addr = 0;
   size_t dynamic_size = 0;
@@ -278,21 +277,11 @@ bool RDebug::Init() {
   return false;
 }
 
-void RDebug::SetDebuggerSupport(bool enabled) {
-  LOG("%s: Setting debugger support to: %s", __FUNCTION__,
-      enabled ? "enabled" : "DISABLED");
-  call_r_brk_ = enabled;
-}
-
-bool RDebug::GetDebuggerSupport() const {
-  return call_r_brk_;
-}
-
 void RDebug::CallRBrk(int state) {
-  if (call_r_brk_) {
-    r_debug_->r_state = state;
-    r_debug_->r_brk();
-  }
+#if !defined(CRAZY_DISABLE_R_BRK)
+  r_debug_->r_state = state;
+  r_debug_->r_brk();
+#endif  // !CRAZY_DISABLE_R_BRK
 }
 
 namespace {

third_party/android_crazy_linker/src/src/crazy_linker_rdebug.h

@@ -122,6 +122,13 @@
 // linker's actions, so to avoid clashing with it we may need to try and
 // move 'r_map' updates to a different thread, to serialize them with
 // other system linker activity.
+//
+// TECHNICAL NOTE: If CRAZY_DISABLE_R_BRK is defined at compile time,
+// then the crazy linker will never try to call the r_brk() GDB Hook
+// function. This can be useful to avoid runtime crashes on certain
+// Android devices with x86 processors, running ARM binaries with
+// a machine translator like Houdini. See http://crbug.com/796938
+//
 namespace crazy {
 
 struct link_map_t {
@@ -197,17 +204,6 @@ class RDebug {
 
   r_debug* GetAddress() { return r_debug_; }
 
-  // Debugger support, which is the default, implies calling the special
-  // hook function r_brk(). Unfortunately, this sometimes results in rare
-  // runtime crashes (see https://crbug.com/796938). This method allows the
-  // client to disable this operation. If |enabled| is false, shared
-  // libraries loaded through the crazy linker will *not* be visible to GDB
-  // (but will continue to appear in stack traces).
-  void SetDebuggerSupport(bool enabled);
-
-  // Return the state of debugger support.
-  bool GetDebuggerSupport() const;
-
  private:
   // Try to find the address of the global _r_debug variable, even
   // though there is no symbol for it. Returns true on success.
@@ -258,7 +254,6 @@ class RDebug {
   r_debug* r_debug_;
   bool init_;
   bool readonly_entries_;
-  bool call_r_brk_;
   rdebug_callback_poster_t post_for_later_execution_;
   void* post_for_later_execution_context_;
 };