Merge changes I578d36a1,Id17508ab,I385f312b

* changes:
  Create linker_log[_va_list] functions
  Validate defined versions in prelink_image
  Prelink each library only once

linker/Android.bp

@@ -156,6 +156,7 @@ filegroup {
         "linker_dlwarning.cpp",
         "linker_cfi.cpp",
         "linker_config.cpp",
+        "linker_debug.cpp",
         "linker_gdb_support.cpp",
         "linker_globals.cpp",
         "linker_libc_support.c",
@@ -460,6 +461,7 @@ cc_test {
         // Parts of the linker that we're testing.
         "linker_block_allocator.cpp",
         "linker_config.cpp",
+        "linker_debug.cpp",
         "linker_test_globals.cpp",
         "linker_utils.cpp",
     ],

linker/linker.cpp

@@ -508,42 +508,29 @@ bool soinfo_do_lookup(soinfo* si_from, const char* name, const version_info* vi,
    */
   if (si_from->has_DT_SYMBOLIC) {
     DEBUG("%s: looking up %s in local scope (DT_SYMBOLIC)", si_from->get_realpath(), name);
-    if (!si_from->find_symbol_by_name(symbol_name, vi, &s)) {
-      return false;
-    }
-
+    s = si_from->find_symbol_by_name(symbol_name, vi);
     if (s != nullptr) {
       *si_found_in = si_from;
     }
   }
 
   // 1. Look for it in global_group
   if (s == nullptr) {
-    bool error = false;
     global_group.visit([&](soinfo* global_si) {
       DEBUG("%s: looking up %s in %s (from global group)",
           si_from->get_realpath(), name, global_si->get_realpath());
-      if (!global_si->find_symbol_by_name(symbol_name, vi, &s)) {
-        error = true;
-        return false;
-      }
-
+      s = global_si->find_symbol_by_name(symbol_name, vi);
       if (s != nullptr) {
         *si_found_in = global_si;
         return false;
       }
 
       return true;
     });
-
-    if (error) {
-      return false;
-    }
   }
 
   // 2. Look for it in the local group
   if (s == nullptr) {
-    bool error = false;
     local_group.visit([&](soinfo* local_si) {
       if (local_si == si_from && si_from->has_DT_SYMBOLIC) {
         // we already did this - skip
@@ -552,22 +539,14 @@ bool soinfo_do_lookup(soinfo* si_from, const char* name, const version_info* vi,
 
       DEBUG("%s: looking up %s in %s (from local group)",
           si_from->get_realpath(), name, local_si->get_realpath());
-      if (!local_si->find_symbol_by_name(symbol_name, vi, &s)) {
-        error = true;
-        return false;
-      }
-
+      s = local_si->find_symbol_by_name(symbol_name, vi);
       if (s != nullptr) {
         *si_found_in = local_si;
         return false;
       }
 
       return true;
     });
-
-    if (error) {
-      return false;
-    }
   }
 
   if (s != nullptr) {
@@ -863,11 +842,7 @@ static const ElfW(Sym)* dlsym_handle_lookup(android_namespace_t* ns,
       return kWalkSkip;
     }
 
-    if (!current_soinfo->find_symbol_by_name(symbol_name, vi, &result)) {
-      result = nullptr;
-      return kWalkStop;
-    }
-
+    result = current_soinfo->find_symbol_by_name(symbol_name, vi);
     if (result != nullptr) {
       *found = current_soinfo;
       return kWalkStop;
@@ -947,10 +922,7 @@ static const ElfW(Sym)* dlsym_linear_lookup(android_namespace_t* ns,
       continue;
     }
 
-    if (!si->find_symbol_by_name(symbol_name, vi, &s)) {
-      return nullptr;
-    }
-
+    s = si->find_symbol_by_name(symbol_name, vi);
     if (s != nullptr) {
       *found = si;
       break;
@@ -2819,25 +2791,38 @@ static bool for_each_verdef(const soinfo* si, F functor) {
   return true;
 }
 
-bool find_verdef_version_index(const soinfo* si, const version_info* vi, ElfW(Versym)* versym) {
+ElfW(Versym) find_verdef_version_index(const soinfo* si, const version_info* vi) {
   if (vi == nullptr) {
-    *versym = kVersymNotNeeded;
-    return true;
+    return kVersymNotNeeded;
   }
 
-  *versym = kVersymGlobal;
+  ElfW(Versym) result = kVersymGlobal;
 
-  return for_each_verdef(si,
+  if (!for_each_verdef(si,
     [&](size_t, const ElfW(Verdef)* verdef, const ElfW(Verdaux)* verdaux) {
       if (verdef->vd_hash == vi->elf_hash &&
           strcmp(vi->name, si->get_string(verdaux->vda_name)) == 0) {
-        *versym = verdef->vd_ndx;
+        result = verdef->vd_ndx;
         return true;
       }
 
       return false;
     }
-  );
+  )) {
+    // verdef should have already been validated in prelink_image.
+    async_safe_fatal("invalid verdef after prelinking: %s, %s",
+                     si->get_realpath(), linker_get_error_buffer());
+  }
+
+  return result;
+}
+
+// Validate the library's verdef section. On error, returns false and invokes DL_ERR.
+bool validate_verdef_section(const soinfo* si) {
+  return for_each_verdef(si,
+    [&](size_t, const ElfW(Verdef)*, const ElfW(Verdaux)*) {
+      return false;
+    });
 }
 
 bool VersionTracker::init_verdef(const soinfo* si_from) {
@@ -3346,6 +3331,7 @@ bool soinfo::relocate(const VersionTracker& version_tracker, ElfRelIteratorT&& r
 static soinfo_list_t g_empty_list;
 
 bool soinfo::prelink_image() {
+  if (flags_ & FLAG_PRELINKED) return true;
   /* Extract dynamic section */
   ElfW(Word) dynamic_flags = 0;
   phdr_table_get_dynamic_section(phdr, phnum, load_bias, &dynamic, &dynamic_flags);
@@ -3840,6 +3826,12 @@ bool soinfo::prelink_image() {
 
     // Don't call add_dlwarning because a missing DT_SONAME isn't important enough to show in the UI
   }
+
+  // Validate each library's verdef section once, so we don't have to validate
+  // it each time we look up a symbol with a version.
+  if (!validate_verdef_section(this)) return false;
+
+  flags_ |= FLAG_PRELINKED;
   return true;
 }
 

linker/linker.h

@@ -204,3 +204,5 @@ struct address_space_params {
   size_t reserved_size = 0;
   bool must_use_address = false;
 };
+
+ElfW(Versym) find_verdef_version_index(const soinfo* si, const version_info* vi);

linker/linker_cfi.cpp

@@ -142,8 +142,7 @@ static soinfo* find_libdl(soinfo* solist) {
 
 static uintptr_t soinfo_find_symbol(soinfo* si, const char* s) {
   SymbolName name(s);
-  const ElfW(Sym) * sym;
-  if (si->find_symbol_by_name(name, nullptr, &sym) && sym) {
+  if (const ElfW(Sym)* sym = si->find_symbol_by_name(name, nullptr)) {
     return si->resolve_symbol_address(sym);
   }
   return 0;

linker/linker_debug.cpp

@@ -0,0 +1,47 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *  * Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ *  * Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "linker_debug.h"
+
+#include <unistd.h>
+
+void linker_log_va_list(int prio __unused, const char* fmt, va_list ap) {
+#if LINKER_DEBUG_TO_LOG
+  async_safe_format_log_va_list(5 - prio, "linker", fmt, ap);
+#else
+  async_safe_format_fd_va_list(STDOUT_FILENO, fmt, ap);
+  write(STDOUT_FILENO, "\n", 1);
+#endif
+}
+
+void linker_log(int prio, const char* fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  linker_log_va_list(prio, fmt, ap);
+  va_end(ap);
+}

linker/linker_debug.h

@@ -52,31 +52,33 @@
  * To enable/disable specific debug options, change the defines above
  *********************************************************************/
 
+#include <stdarg.h>
 #include <unistd.h>
 
 #include <async_safe/log.h>
 #include <async_safe/CHECK.h>
 
+#define LINKER_VERBOSITY_PRINT  -1
+#define LINKER_VERBOSITY_INFO   0
+#define LINKER_VERBOSITY_TRACE  1
+#define LINKER_VERBOSITY_DEBUG  2
+
 __LIBC_HIDDEN__ extern int g_ld_debug_verbosity;
 
-#if LINKER_DEBUG_TO_LOG
-#define _PRINTVF(v, x...) \
-    do { \
-      if (g_ld_debug_verbosity > (v)) async_safe_format_log(5-(v), "linker", x); \
-    } while (0)
-#else /* !LINKER_DEBUG_TO_LOG */
+__LIBC_HIDDEN__ void linker_log_va_list(int prio, const char* fmt, va_list ap);
+__LIBC_HIDDEN__ void linker_log(int prio, const char* fmt, ...) __printflike(2, 3);
+
 #define _PRINTVF(v, x...) \
     do { \
-      if (g_ld_debug_verbosity > (v)) { async_safe_format_fd(1, x); write(1, "\n", 1); } \
+      if (g_ld_debug_verbosity > (v)) linker_log((v), x); \
     } while (0)
-#endif /* !LINKER_DEBUG_TO_LOG */
 
-#define PRINT(x...)          _PRINTVF(-1, x)
-#define INFO(x...)           _PRINTVF(0, x)
-#define TRACE(x...)          _PRINTVF(1, x)
+#define PRINT(x...)          _PRINTVF(LINKER_VERBOSITY_PRINT, x)
+#define INFO(x...)           _PRINTVF(LINKER_VERBOSITY_INFO, x)
+#define TRACE(x...)          _PRINTVF(LINKER_VERBOSITY_TRACE, x)
 
 #if TRACE_DEBUG
-#define DEBUG(x...)          _PRINTVF(2, "DEBUG: " x)
+#define DEBUG(x...)          _PRINTVF(LINKER_VERBOSITY_DEBUG, "DEBUG: " x)
 #else /* !TRACE_DEBUG */
 #define DEBUG(x...)          do {} while (0)
 #endif /* TRACE_DEBUG */