android: Don't parse /proc/self/maps to prefetch the library.

This commit changes the behavior of the native library prefetcher to
rely on sentinel symbols rather than parsing /proc/self/maps.

Behavior changes:
1. This will make the code no longer prefetch .data (because we don't
   look for it in the mappings), neither the sections that are part of
   the same mapping as .text (for instance, .rodata when using
   ld.gold). This is intended.
2. As a consequence, the UMA metric
   LibraryLoader.PercentageOfResidentCodeBeforePrefetch will move, as
   both sides of the ratio will change, and the population will change
   slightly as well (since code is currently not correctly ordered on
   ARM64 and x86_64)

- Removes the reliance on a somewhat brittle parsing of a file
- Parsing /proc/self/maps costs ~50ms of CPU on a Nexus 5X (on a little
  core, as this is triggered from an AsyncTask)
- Allows to only fetch part of the native library (in a forthcoming CL)

Rationale: 
Change-Id: I0bb7b28af3c3bd4af5745e2ebcc1fbf283bcc0c1
Reviewed-on: https://chromium-review.googlesource.com/874470
Commit-Queue: Benoit L <lizeb@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Matthew Cary <mattcary@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531515}

base/BUILD.gn

@@ -1787,10 +1787,14 @@ buildflag_header("synchronization_flags") {
 buildflag_header("anchor_functions_flags") {
   header = "anchor_functions_flags.h"
   header_dir = "base/android/library_loader"
+  _supports_code_ordering = current_cpu == "arm"
 
   # buildflag entries added to this header must also must be manually added to
   # tools/gn/bootstrap/bootstrap.py
-  flags = [ "USE_LLD=$use_lld" ]
+  flags = [
+    "USE_LLD=$use_lld",
+    "SUPPORTS_CODE_ORDERING=$_supports_code_ordering",
+  ]
 }
 
 # This is the subset of files from base that should not be used with a dynamic

base/android/library_loader/anchor_functions.cc

@@ -4,13 +4,11 @@
 
 #include "base/android/library_loader/anchor_functions.h"
 
-#include "base/android/library_loader/anchor_functions_flags.h"
 #include "base/logging.h"
 #include "build/build_config.h"
 
-// asm() macros below don't compile on x86, and haven't been validated outside
-// ARM.
-#if defined(ARCH_CPU_ARMEL)
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
+
 // These functions are here to, respectively:
 // 1. Check that functions are ordered
 // 2. Delimit the start of .text
@@ -68,20 +66,18 @@ const size_t kStartOfText =
 const size_t kEndOfText =
     reinterpret_cast<size_t>(dummy_function_at_the_end_of_text);
 
-void CheckOrderingSanity() {
+bool IsOrderingSane() {
+  size_t dummy = reinterpret_cast<size_t>(&dummy_function_to_check_ordering);
+  size_t here = reinterpret_cast<size_t>(&IsOrderingSane);
   // The linker usually keeps the input file ordering for symbols.
   // dummy_function_to_anchor_text() should then be after
   // dummy_function_to_check_ordering() without ordering.
   // This check is thus intended to catch the lack of ordering.
-  CHECK_LT(kStartOfText,
-           reinterpret_cast<size_t>(&dummy_function_to_check_ordering));
-  CHECK_LT(kStartOfText, kEndOfText);
-  CHECK_LT(kStartOfText,
-           reinterpret_cast<size_t>(&dummy_function_to_check_ordering));
-  CHECK_LT(kStartOfText, reinterpret_cast<size_t>(&CheckOrderingSanity));
-  CHECK_GT(kEndOfText, reinterpret_cast<size_t>(&CheckOrderingSanity));
+  return kStartOfText < dummy && dummy < kEndOfText && kStartOfText < here &&
+         here < kEndOfText;
 }
 
 }  // namespace android
 }  // namespace base
-#endif  // defined(ARCH_CPU_ARMEL)
+
+#endif  // BUILDFLAG(SUPPORTS_CODE_ORDERING)

base/android/library_loader/anchor_functions.h

@@ -6,11 +6,12 @@
 #define BASE_ANDROID_LIBRARY_LOADER_ANCHOR_FUNCTIONS_H_
 
 #include <cstdint>
+#include "base/android/library_loader/anchor_functions_flags.h"
 
 #include "base/base_export.h"
-#include "build/build_config.h"
 
-#if defined(ARCH_CPU_ARMEL)
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
+
 namespace base {
 namespace android {
 
@@ -18,11 +19,11 @@ namespace android {
 BASE_EXPORT extern const size_t kStartOfText;
 BASE_EXPORT extern const size_t kEndOfText;
 
-// Basic CHECK()s ensuring that the symbols above are correctly set.
-BASE_EXPORT void CheckOrderingSanity();
+// Returns true if the ordering looks sane.
+BASE_EXPORT bool IsOrderingSane();
 
 }  // namespace android
 }  // namespace base
-#endif  // defined(ARCH_CPU_ARMEL)
+#endif  // BUILDFLAG(SUPPORTS_CODE_ORDERING)
 
 #endif  // BASE_ANDROID_LIBRARY_LOADER_ANCHOR_FUNCTIONS_H_

base/android/library_loader/library_loader_hooks.cc

@@ -5,6 +5,7 @@
 #include "base/android/library_loader/library_loader_hooks.h"
 
 #include "base/android/jni_string.h"
+#include "base/android/library_loader/anchor_functions_flags.h"
 #include "base/android/library_loader/library_load_from_apk_status_codes.h"
 #include "base/android/library_loader/library_prefetcher.h"
 #include "base/at_exit.h"
@@ -169,7 +170,12 @@ static jboolean JNI_LibraryLoader_LibraryLoaded(
     const JavaParamRef<jobject>& jcaller) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kMadviseRandomExecutableCode)) {
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
     NativeLibraryPrefetcher::MadviseRandomText();
+#else
+    LOG(WARNING) << switches::kMadviseRandomExecutableCode
+                 << " is not supported on this architecture.";
+#endif
   }
 
   if (g_native_initialization_hook && !g_native_initialization_hook())
@@ -189,19 +195,31 @@ void LibraryLoaderExitHook() {
 static jboolean JNI_LibraryLoader_ForkAndPrefetchNativeLibrary(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz) {
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
   return NativeLibraryPrefetcher::ForkAndPrefetchNativeLibrary();
+#else
+  return false;
+#endif
 }
 
 static jint JNI_LibraryLoader_PercentageOfResidentNativeLibraryCode(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz) {
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
   return NativeLibraryPrefetcher::PercentageOfResidentNativeLibraryCode();
+#else
+  return -1;
+#endif
 }
 
 static void JNI_LibraryLoader_PeriodicallyCollectResidency(
     JNIEnv* env,
     const JavaParamRef<jclass>& clazz) {
-  return NativeLibraryPrefetcher::PeriodicallyCollectResidency();
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
+  NativeLibraryPrefetcher::PercentageOfResidentNativeLibraryCode();
+#else
+  LOG(WARNING) << "Collecting residency is not supported.";
+#endif
 }
 
 void SetVersionNumber(const char* version_number) {

base/android/library_loader/library_prefetcher.h

@@ -8,33 +8,32 @@
 #include <jni.h>
 
 #include <stdint.h>
-#include <string>
-#include <utility>
-#include <vector>
 
-#include "base/debug/proc_maps_linux.h"
+#include "base/android/library_loader/anchor_functions_flags.h"
+#include "base/base_export.h"
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
+
 namespace base {
 namespace android {
 
 // Forks and waits for a process prefetching the native library. This is done in
 // a forked process for the following reasons:
-// - Isolating the main process from mistakes in the parsing. If the parsing
-//   returns an incorrect address, only the forked process will crash.
+// - Isolating the main process from mistakes in getting the address range, only
+//   crashing the forked process in case of mistake.
 // - Not inflating the memory used by the main process uselessly, which could
 //   increase its likelihood to be killed.
 // The forked process has background priority and, since it is not declared to
 // the Android runtime, can be killed at any time, which is not an issue here.
 class BASE_EXPORT NativeLibraryPrefetcher {
  public:
-  using AddressRange = std::pair<uintptr_t, uintptr_t>;
-
-  // Finds the ranges matching the native library, forks a low priority
-  // process pre-fetching these ranges and wait()s for it.
+  // Finds the executable code range, forks a low priority process pre-fetching
+  // it wait()s for the process to exit or die.
   // Returns true for success.
   static bool ForkAndPrefetchNativeLibrary();
+
   // Returns the percentage of the native library code currently resident in
   // memory, or -1 in case of error.
   static int PercentageOfResidentNativeLibraryCode();
@@ -47,38 +46,12 @@ class BASE_EXPORT NativeLibraryPrefetcher {
   static void MadviseRandomText();
 
  private:
-  // Returns true if the region matches native code or data.
-  static bool IsGoodToPrefetch(const base::debug::MappedMemoryRegion& region);
-  // Filters the regions to keep only libchrome ranges if possible.
-  static void FilterLibchromeRangesOnlyIfPossible(
-      const std::vector<base::debug::MappedMemoryRegion>& regions,
-      std::vector<AddressRange>* ranges);
-  // Finds the ranges matching the native library in /proc/self/maps.
-  // Returns true for success.
-  static bool FindRanges(std::vector<AddressRange>* ranges);
-
-  // Returns the percentage of the given address ranges currently resident in
+  // Returns the percentage of [start, end] currently resident in
   // memory, or -1 in case of error.
-  static int PercentageOfResidentCode(const std::vector<AddressRange>& ranges);
+  static int PercentageOfResidentCode(size_t start, size_t end);
 
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestIsGoodToPrefetchNoRange);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestIsGoodToPrefetchUnreadableRange);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestIsGoodToPrefetchSkipSharedRange);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestIsGoodToPrefetchLibchromeRange);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestIsGoodToPrefetchBaseApkRange);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestFilterLibchromeRangesOnlyIfPossibleNoLibchrome);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestFilterLibchromeRangesOnlyIfPossibleHasLibchrome);
   FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
                            TestPercentageOfResidentCode);
-  FRIEND_TEST_ALL_PREFIXES(NativeLibraryPrefetcherTest,
-                           TestPercentageOfResidentCodeTwoRegions);
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(NativeLibraryPrefetcher);
 };
@@ -86,4 +59,6 @@ class BASE_EXPORT NativeLibraryPrefetcher {
 }  // namespace android
 }  // namespace base
 
+#endif  // BUILDFLAG(SUPPORTS_CODE_ORDERING)
+
 #endif  // BASE_ANDROID_LIBRARY_LOADER_LIBRARY_PREFETCHER_H_

base/android/library_loader/library_prefetcher_unittest.cc

@@ -7,96 +7,15 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <sys/mman.h>
-#include <string>
-#include <vector>
-#include "base/debug/proc_maps_linux.h"
+#include "base/android/library_loader/anchor_functions_flags.h"
 #include "base/memory/shared_memory.h"
 #include "build/build_config.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if BUILDFLAG(SUPPORTS_CODE_ORDERING)
 namespace base {
 namespace android {
 
-namespace {
-const uint8_t kRead = base::debug::MappedMemoryRegion::READ;
-const uint8_t kReadPrivate = base::debug::MappedMemoryRegion::READ |
-                             base::debug::MappedMemoryRegion::PRIVATE;
-const uint8_t kExecutePrivate = base::debug::MappedMemoryRegion::EXECUTE |
-                                base::debug::MappedMemoryRegion::PRIVATE;
-}  // namespace
-
-TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchNoRange) {
-  const base::debug::MappedMemoryRegion regions[4] = {
-      base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate, ""},
-      base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
-                                      "foo"},
-      base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
-                                      "foobar.apk"},
-      base::debug::MappedMemoryRegion{0x4000, 0x5000, 10, 0, kReadPrivate,
-                                      "libchromium.so"}};
-  for (int i = 0; i < 4; ++i) {
-    ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(regions[i]));
-  }
-}
-
-TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchUnreadableRange) {
-  const base::debug::MappedMemoryRegion region = {
-      0x4000, 0x5000, 10, 0, kExecutePrivate, "base.apk"};
-  ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
-}
-
-TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchSkipSharedRange) {
-  const base::debug::MappedMemoryRegion region = {
-      0x4000, 0x5000, 10, 0, kRead, "base.apk"};
-  ASSERT_FALSE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
-}
-
-TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchLibchromeRange) {
-  const base::debug::MappedMemoryRegion region = {
-      0x4000, 0x5000, 10, 0, kReadPrivate, "libchrome.so"};
-  ASSERT_TRUE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
-}
-
-TEST(NativeLibraryPrefetcherTest, TestIsGoodToPrefetchBaseApkRange) {
-  const base::debug::MappedMemoryRegion region = {
-      0x4000, 0x5000, 10, 0, kReadPrivate, "base.apk"};
-  ASSERT_TRUE(NativeLibraryPrefetcher::IsGoodToPrefetch(region));
-}
-
-TEST(NativeLibraryPrefetcherTest,
-     TestFilterLibchromeRangesOnlyIfPossibleNoLibchrome) {
-  std::vector<base::debug::MappedMemoryRegion> regions;
-  regions.push_back(base::debug::MappedMemoryRegion{0x1, 0x2, 0, 0,
-                                                    kReadPrivate, "base.apk"});
-  regions.push_back(base::debug::MappedMemoryRegion{0x3, 0x4, 0, 0,
-                                                    kReadPrivate, "base.apk"});
-  std::vector<NativeLibraryPrefetcher::AddressRange> ranges;
-  NativeLibraryPrefetcher::FilterLibchromeRangesOnlyIfPossible(regions,
-                                                               &ranges);
-  EXPECT_EQ(ranges.size(), 2U);
-  EXPECT_EQ(ranges[0].first, 0x1U);
-  EXPECT_EQ(ranges[0].second, 0x2U);
-  EXPECT_EQ(ranges[1].first, 0x3U);
-  EXPECT_EQ(ranges[1].second, 0x4U);
-}
-
-TEST(NativeLibraryPrefetcherTest,
-     TestFilterLibchromeRangesOnlyIfPossibleHasLibchrome) {
-  std::vector<base::debug::MappedMemoryRegion> regions;
-  regions.push_back(base::debug::MappedMemoryRegion{0x1, 0x2, 0, 0,
-                                                    kReadPrivate, "base.apk"});
-  regions.push_back(base::debug::MappedMemoryRegion{
-      0x6, 0x7, 0, 0, kReadPrivate, "libchrome.so"});
-  regions.push_back(base::debug::MappedMemoryRegion{0x3, 0x4, 0, 0,
-                                                    kReadPrivate, "base.apk"});
-  std::vector<NativeLibraryPrefetcher::AddressRange> ranges;
-  NativeLibraryPrefetcher::FilterLibchromeRangesOnlyIfPossible(regions,
-                                                               &ranges);
-  EXPECT_EQ(ranges.size(), 1U);
-  EXPECT_EQ(ranges[0].first, 0x6U);
-  EXPECT_EQ(ranges[0].second, 0x7U);
-}
-
 // Fails with ASAN, crbug.com/570423.
 #if !defined(ADDRESS_SANITIZER)
 namespace {
@@ -108,55 +27,20 @@ TEST(NativeLibraryPrefetcherTest, TestPercentageOfResidentCode) {
   base::SharedMemory shared_mem;
   ASSERT_TRUE(shared_mem.CreateAndMapAnonymous(length));
   void* address = shared_mem.memory();
-
-  std::vector<NativeLibraryPrefetcher::AddressRange> ranges = {
-      {reinterpret_cast<uintptr_t>(address),
-       reinterpret_cast<uintptr_t>(address) + length}};
+  size_t start = reinterpret_cast<size_t>(address);
+  size_t end = start + length;
 
   // Remove everything.
   ASSERT_EQ(0, madvise(address, length, MADV_DONTNEED));
-  // TODO(lizeb): If flaky, mock mincore().
-  EXPECT_EQ(0, NativeLibraryPrefetcher::PercentageOfResidentCode(ranges));
+  EXPECT_EQ(0, NativeLibraryPrefetcher::PercentageOfResidentCode(start, end));
 
   // Get everything back.
   ASSERT_EQ(0, mlock(address, length));
-  EXPECT_EQ(100, NativeLibraryPrefetcher::PercentageOfResidentCode(ranges));
-  munlock(address, length);
-}
-
-TEST(NativeLibraryPrefetcherTest, TestPercentageOfResidentCodeTwoRegions) {
-  size_t length = 4 * kPageSize;
-  base::SharedMemory shared_mem;
-  ASSERT_TRUE(shared_mem.CreateAndMapAnonymous(length));
-  void* address = shared_mem.memory();
-
-  size_t length2 = 8 * kPageSize;
-  base::SharedMemory shared_mem2;
-  ASSERT_TRUE(shared_mem2.CreateAndMapAnonymous(length2));
-  void* address2 = shared_mem2.memory();
-
-  std::vector<NativeLibraryPrefetcher::AddressRange> ranges = {
-      {reinterpret_cast<uintptr_t>(address),
-       reinterpret_cast<uintptr_t>(address) + length},
-      {reinterpret_cast<uintptr_t>(address2),
-       reinterpret_cast<uintptr_t>(address2) + length2}};
-
-  // Remove everything.
-  ASSERT_EQ(0, madvise(address, length, MADV_DONTNEED));
-  ASSERT_EQ(0, madvise(address2, length, MADV_DONTNEED));
-  // TODO(lizeb): If flaky, mock mincore().
-  EXPECT_EQ(0, NativeLibraryPrefetcher::PercentageOfResidentCode(ranges));
-
-  // Get back the first range.
-  ASSERT_EQ(0, mlock(address, length));
-  EXPECT_EQ(33, NativeLibraryPrefetcher::PercentageOfResidentCode(ranges));
-  // The second one.
-  ASSERT_EQ(0, mlock(address2, length2));
-  EXPECT_EQ(100, NativeLibraryPrefetcher::PercentageOfResidentCode(ranges));
+  EXPECT_EQ(100, NativeLibraryPrefetcher::PercentageOfResidentCode(start, end));
   munlock(address, length);
-  munlock(address2, length);
 }
 #endif  // !defined(ADDRESS_SANITIZER)
 
 }  // namespace android
 }  // namespace base
+#endif  // BUILDFLAG(SUPPORTS_CODE_ORDERING)

tools/cygprofile/lightweight_cygprofile.cc

@@ -161,7 +161,7 @@ void Disable() {
 void SanityChecks() {
   CHECK_LT(base::android::kEndOfText - base::android::kStartOfText,
            kMaxTextSizeInBytes);
-  base::android::CheckOrderingSanity();
+  CHECK(base::android::IsOrderingSane());
 }
 
 bool SwitchToNextPhaseOrDump(int pid, uint64_t start_ns_since_epoch) {

tools/gn/bootstrap/bootstrap.py

@@ -227,7 +227,7 @@ def build_gn_with_ninja_manually(tempdir, options):
 
   write_buildflag_header_manually(
       root_gen_dir, 'base/android/library_loader.h',
-      {'USE_LLD': 'false'})
+      {'USE_LLD': 'false', 'SUPPORTS_CODE_ORDERING': 'false'})
 
   write_gn_ninja(os.path.join(tempdir, 'build.ninja'),
                  root_gen_dir, options)