android: crazy_linker: Simplify device API level detection

It is possible to detect the device's API level from native
code directly, so do it instead of relying on the client
passing the coresponding value through a special global
variable that must be set before anything else is called
in the linker library.

This simplifies the crazy linker API, its implementation
and linker_jni.cc all at the same time.

BUG=NONE
R=agrieve@chromium.org, pasko@chromium.org, cjgrant@chromium.org

Change-Id: I08a6afd82ef0e329957728f6004050bcc04fad18
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1502972
Commit-Queue: David Turner <digit@chromium.org>
Reviewed-by: David Turner <digit@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638163}

base/android/linker/linker_jni.cc

@@ -131,47 +131,6 @@ bool InitFieldId(JNIEnv* env,
   return true;
 }
 
-// Initialize a jfieldID corresponding to the static field of a given |clazz|,
-// with name |field_name| and signature |field_sig|.
-// |env| is the current JNI environment handle.
-// On success, return true and set |*field_id|.
-bool InitStaticFieldId(JNIEnv* env,
-                       jclass clazz,
-                       const char* field_name,
-                       const char* field_sig,
-                       jfieldID* field_id) {
-  *field_id = env->GetStaticFieldID(clazz, field_name, field_sig);
-  if (!*field_id) {
-    LOG_ERROR("Could not find ID for static field '%s'", field_name);
-    return false;
-  }
-  LOG_INFO("Found ID %p for static field '%s'", *field_id, field_name);
-  return true;
-}
-
-// Initialize a jint corresponding to the static integer field of a class
-// with class name |class_name| and field name |field_name|.
-// |env| is the current JNI environment handle.
-// On success, return true and set |*value|.
-bool InitStaticInt(JNIEnv* env,
-                   const char* class_name,
-                   const char* field_name,
-                   jint* value) {
-  jclass clazz;
-  if (!InitClassReference(env, class_name, &clazz))
-    return false;
-
-  jfieldID field_id;
-  if (!InitStaticFieldId(env, clazz, field_name, "I", &field_id))
-    return false;
-
-  *value = env->GetStaticIntField(clazz, field_id);
-  LOG_INFO("Found value %d for class '%s', static field '%s'",
-           *value, class_name, field_name);
-
-  return true;
-}
-
 // A class used to model the field IDs of the org.chromium.base.Linker
 // LibInfo inner class, used to communicate data with the Java side
 // of the linker.
@@ -290,22 +249,6 @@ class ScopedLibrary {
   crazy_library_t* lib_;
 };
 
-// Retrieve the SDK build version and pass it into the crazy linker. This
-// needs to be done early in initialization, before any other crazy linker
-// code is run.
-// |env| is the current JNI environment handle.
-// On success, return true.
-bool InitSDKVersionInfo(JNIEnv* env) {
-  jint value = 0;
-  if (!InitStaticInt(env, "android/os/Build$VERSION", "SDK_INT", &value))
-    return false;
-
-  crazy_set_sdk_build_version(static_cast<int>(value));
-  LOG_INFO("Set SDK build version to %d", static_cast<int>(value));
-
-  return true;
-}
-
 }  // namespace
 
 // Use Android ASLR to create a random address into which we expect to be
@@ -506,11 +449,6 @@ Java_org_chromium_base_library_1loader_Linker_nativeUseSharedRelro(
 static bool LinkerJNIInit(JavaVM* vm, JNIEnv* env) {
   LOG_INFO("Entering");
 
-  // Initialize SDK version info.
-  LOG_INFO("Retrieving SDK version info");
-  if (!InitSDKVersionInfo(env))
-    return false;
-
   // Find LibInfo field ids.
   LOG_INFO("Caching field IDs");
   if (!s_lib_info_fields.Init(env)) {

third_party/android_crazy_linker/BUILD.gn

@@ -100,6 +100,8 @@ if (is_android) {
         "src/src/crazy_linker_shared_library.h",
         "src/src/crazy_linker_system.cpp",
         "src/src/crazy_linker_system.h",
+        "src/src/crazy_linker_system_android.cpp",
+        "src/src/crazy_linker_system_android.h",
         "src/src/crazy_linker_system_linker.cpp",
         "src/src/crazy_linker_system_linker.h",
         "src/src/crazy_linker_thread_data.cpp",
@@ -542,7 +544,7 @@ if (is_android) {
 # cd $CHROMIUM_SRC_DIR
 # mkdir out/Fuzz
 # gn gen out/Fuzz '--args=use_libfuzzer=true is_asan=true is_debug=false'
-# gn out/Fuzz android_crazy_linker_zip_fuzzer
+# ninja -C out/Fuzz android_crazy_linker_zip_fuzzer
 # out/Fuzz/android_crazy_linker_zip_fuzzer out/Release/apks
 #
 fuzzer_test("android_crazy_linker_zip_fuzzer") {

third_party/android_crazy_linker/src/include/crazy_linker.h

@@ -121,10 +121,6 @@ crazy_status_t crazy_add_search_path_for_address(void* address) _CRAZY_PUBLIC;
 // crazy_context_add_search_path_for_address().
 void crazy_reset_search_paths(void) _CRAZY_PUBLIC;
 
-// Pass the platform's SDK build version to the crazy linker. The value is
-// from android.os.Build.VERSION.SDK_INT.
-void crazy_set_sdk_build_version(int sdk_build_version);
-
 // Opaque handle to a library as seen/loaded by the crazy linker.
 typedef struct crazy_library_t crazy_library_t;
 

third_party/android_crazy_linker/src/src/crazy_linker_api.cpp

@@ -41,12 +41,6 @@ struct crazy_context_t {
 
 extern "C" {
 
-void crazy_set_sdk_build_version(int sdk_build_version) {
-  // NOTE: This must be called before creating the Globals instance,
-  // so do not use Globals::Get() or a ScopedLockedGlobals instance here.
-  Globals::sdk_build_version = sdk_build_version;
-}
-
 crazy_context_t* crazy_context_create() {
   return new crazy_context_t();
 }

third_party/android_crazy_linker/src/src/crazy_linker_globals.cpp

@@ -63,9 +63,6 @@ Globals* Globals::Get() {
   return &s_storage.globals;
 }
 
-// static
-int Globals::sdk_build_version = 0;
-
 // TECHNICAL NOTE: The mutex below does not have to be recursive, unlike the
 // other one, because it should be only operated on during the following
 // operations:

third_party/android_crazy_linker/src/src/crazy_linker_globals.h

@@ -62,11 +62,6 @@ class Globals {
   // Return current minimum JNI version number.
   int minimum_jni_version() const { return min_jni_version_; }
 
-  // Android API level for the current device (if known).
-  // This is static because it must be set before the first call to Get().
-  // See LibraryList() constructor comment for details.
-  static int sdk_build_version;
-
   // Convenience function to get the global RDebug instance.
   static RDebug* GetRDebug() { return Get()->rdebug(); }
 

third_party/android_crazy_linker/src/src/crazy_linker_library_list.cpp

@@ -17,13 +17,14 @@
 #include "crazy_linker_system_linker.h"
 #include "crazy_linker_util.h"
 
+#ifdef __ANDROID__
+#include "crazy_linker_system_android.h"
+#endif
+
 namespace crazy {
 
 namespace {
 
-// From android.os.Build.VERSION_CODES.LOLLIPOP.
-static const int SDK_VERSION_CODE_LOLLIPOP = 21;
-
 // A helper struct used when looking up symbols in libraries.
 struct SymbolLookupState {
   void* found_addr = nullptr;
@@ -55,10 +56,8 @@ struct SymbolLookupState {
 }  // namespace
 
 LibraryList::LibraryList() {
-  // NOTE: This constructor is called from the Globals::Globals() constructor,
-  // hence it is important that Globals::sdk_build_version is a static member
-  // that can be set before Globals::Get() is called for the first time.
-  const int sdk_build_version = Globals::sdk_build_version;
+#ifdef __ANDROID__
+  const int sdk_build_version = GetAndroidDeviceApiLevel();
 
   // If SDK version is Lollipop or earlier, we need to load anything
   // listed in LD_PRELOAD explicitly, because dlsym() on the main executable
@@ -77,8 +76,9 @@ LibraryList::LibraryList() {
   //
   // For more, see:
   //   https://code.google.com/p/android/issues/detail?id=74255
-  if (sdk_build_version <= SDK_VERSION_CODE_LOLLIPOP)
+  if (sdk_build_version <= ANDROID_SDK_VERSION_CODE_LOLLIPOP)
     LoadPreloads();
+#endif  // __ANDROID__
 }
 
 LibraryList::~LibraryList() {

third_party/android_crazy_linker/src/src/crazy_linker_system_android.cpp

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "crazy_linker_system_android.h"
+
+#include "crazy_linker_debug.h"
+
+#ifndef __ANDROID__
+#error "This source file should only be compiled for Android."
+#endif
+
+#include <stdlib.h>
+#include <sys/system_properties.h>
+
+namespace crazy {
+
+// Technical note regarding reading system properties.
+//
+// Try to use the new __system_property_read_callback API that appeared in
+// Android O / API level 26 when available. Otherwise use the deprecated
+// __system_property_get function.
+//
+// For more technical details from an NDK maintainer, see:
+// https://bugs.chromium.org/p/chromium/issues/detail?id=392191#c17
+
+// Use a weak symbol import to resolve at runtime whether the function is
+// available.
+extern "C" void __system_property_read_callback(
+    const prop_info* info,
+    void (*callback)(void* cookie,
+                     const char* name,
+                     const char* value,
+                     uint32_t serial),
+    void* cookie) __attribute__((weak));
+
+static int GetSystemPropertyAsInt(const char* name) {
+  int result = 0;
+  if (__system_property_read_callback) {
+    const prop_info* info = __system_property_find(name);
+    if (info) {
+      auto callback = [](void* cookie, const char*, const char* value,
+                         uint32_t) { *(int*)cookie = atoi(value); };
+      __system_property_read_callback(info, callback, &result);
+    }
+  } else {
+    char value[PROP_VALUE_MAX] = {};
+    if (__system_property_get(name, value) >= 1)
+      result = atoi(value);
+  }
+  return result;
+}
+
+int GetAndroidDeviceApiLevel() {
+  static int s_api_level = -1;
+  if (s_api_level < 0) {
+    s_api_level = GetSystemPropertyAsInt("ro.build.version.sdk");
+    LOG("Device API level: %d", s_api_level);
+  }
+  return s_api_level;
+}
+
+}  // namespace crazy

third_party/android_crazy_linker/src/src/crazy_linker_system_android.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CRAZY_LINKER_SYSTEM_ANDROID_H
+#define CRAZY_LINKER_SYSTEM_ANDROID_H
+
+namespace crazy {
+
+// From android.os.Build.VERSION_CODES.LOLLIPOP.
+// Using a macro since constexpr in headers are problematic in C++.
+#define ANDROID_SDK_VERSION_CODE_LOLLIPOP 21
+
+// Return the current Android device's API level.
+int GetAndroidDeviceApiLevel();
+
+}  // namespace crazy
+
+#endif  // CRAZY_LINKER_SYSTEM_ANDROID_H