Ensure same order of global group members in all NS's

During "step 1" of find_libraries, the linker finds the transitive
closure of dependencies, in BFS order. As it finds each library, it
adds the library to its primary namespace (so that, if some other
library also depends on it, find_loaded_library_by_soname can find the
library in the process of being loaded).

LD_PRELOAD libraries are automatically marked DF_1_GLOBAL, and any
DF_1_GLOBAL library is added to every linker namespace. Previously,
this secondary namespace registration happened after step 1. The result
is that across different namespaces, the order of libraries could vary.
In general, a namespace's primary members will all appear before
secondary members. This is undesirable for libsigchain.so, which we
want to have appear before any other non-preloaded library.

Instead, when an soinfo is added to its primary namespace, immediately
add it to all the other namespaces, too. This ensures that the order of
soinfo objects is the same across namespaces.

Expand the dl.exec_with_ld_config_file_with_ld_preload and
dl.exec_with_ld_config_file tests to cover the new behavior. Mark
lib1.so DF_1_GLOBAL and use a "foo" symbol to mimic the behavior of a
signal API interposed by (e.g.) libsigchain.so and a ASAN preload.

Test: bionic unit tests
Bug: http://b/143219447
Change-Id: I9fd90f6f0d14caf1aca6d414b3e9aab77deca3ff

linker/linker.cpp

@@ -1242,7 +1242,7 @@ static bool load_library(android_namespace_t* ns,
     return false;
   }
 
-  // find and set DT_RUNPATH and dt_soname
+  // Find and set DT_RUNPATH, DT_SONAME, and DT_FLAGS_1.
   // Note that these field values are temporary and are
   // going to be overwritten on soinfo::prelink_image
   // with values from PT_LOAD segments.
@@ -1254,6 +1254,10 @@ static bool load_library(android_namespace_t* ns,
     if (d->d_tag == DT_SONAME) {
       si->set_soname(elf_reader.get_string(d->d_un.d_val));
     }
+    // We need to identify a DF_1_GLOBAL library early so we can link it to namespaces.
+    if (d->d_tag == DT_FLAGS_1) {
+      si->set_dt_flags_1(d->d_un.d_val);
+    }
   }
 
 #if !defined(__ANDROID__)
@@ -1552,6 +1556,7 @@ bool find_libraries(android_namespace_t* ns,
   });
 
   ZipArchiveCache zip_archive_cache;
+  soinfo_list_t new_global_group_members;
 
   // Step 1: expand the list of load_tasks to include
   // all DT_NEEDED libraries (do not load them just yet)
@@ -1586,13 +1591,31 @@ bool find_libraries(android_namespace_t* ns,
 
     // When ld_preloads is not null, the first
     // ld_preloads_count libs are in fact ld_preloads.
+    bool is_ld_preload = false;
     if (ld_preloads != nullptr && soinfos_count < ld_preloads_count) {
       ld_preloads->push_back(si);
+      is_ld_preload = true;
     }
 
     if (soinfos_count < library_names_count) {
       soinfos[soinfos_count++] = si;
     }
+
+    // Add the new global group members to all initial namespaces. Do this secondary namespace setup
+    // at the same time that libraries are added to their primary namespace so that the order of
+    // global group members is the same in the every namespace. Only add a library to a namespace
+    // once, even if it appears multiple times in the dependency graph.
+    if (is_ld_preload || (si->get_dt_flags_1() & DF_1_GLOBAL) != 0) {
+      if (!si->is_linked() && namespaces != nullptr && !new_global_group_members.contains(si)) {
+        new_global_group_members.push_back(si);
+        for (auto linked_ns : *namespaces) {
+          if (si->get_primary_namespace() != linked_ns) {
+            linked_ns->add_soinfo(si);
+            si->add_secondary_namespace(linked_ns);
+          }
+        }
+      }
+    }
   }
 
   // Step 2: Load libraries in random order (see b/24047022)
@@ -1649,39 +1672,15 @@ bool find_libraries(android_namespace_t* ns,
     register_soinfo_tls(si);
   }
 
-  // Step 4: Construct the global group. Note: DF_1_GLOBAL bit of a library is
-  // determined at step 3.
-
-  // Step 4-1: DF_1_GLOBAL bit is force set for LD_PRELOADed libs because they
-  // must be added to the global group
+  // Step 4: Construct the global group. DF_1_GLOBAL bit is force set for LD_PRELOADed libs because
+  // they must be added to the global group. Note: The DF_1_GLOBAL bit for a library is normally set
+  // in step 3.
   if (ld_preloads != nullptr) {
     for (auto&& si : *ld_preloads) {
       si->set_dt_flags_1(si->get_dt_flags_1() | DF_1_GLOBAL);
     }
   }
 
-  // Step 4-2: Gather all DF_1_GLOBAL libs which were newly loaded during this
-  // run. These will be the new member of the global group
-  soinfo_list_t new_global_group_members;
-  for (auto&& task : load_tasks) {
-    soinfo* si = task->get_soinfo();
-    if (!si->is_linked() && (si->get_dt_flags_1() & DF_1_GLOBAL) != 0) {
-      new_global_group_members.push_back(si);
-    }
-  }
-
-  // Step 4-3: Add the new global group members to all the linked namespaces
-  if (namespaces != nullptr) {
-    for (auto linked_ns : *namespaces) {
-      for (auto si : new_global_group_members) {
-        if (si->get_primary_namespace() != linked_ns) {
-          linked_ns->add_soinfo(si);
-          si->add_secondary_namespace(linked_ns);
-        }
-      }
-    }
-  }
-
   // Step 5: Collect roots of local_groups.
   // Whenever needed_by->si link crosses a namespace boundary it forms its own local_group.
   // Here we collect new roots to link them separately later on. Note that we need to avoid

tests/dl_test.cpp

@@ -268,8 +268,16 @@ static bool is_debuggable_build() {
 }
 #endif
 
-// _lib1.so and _lib2.so are now searchable by having another namespace 'ns2'
+// lib1.so and lib2.so are now searchable by having another namespace 'ns2'
 // whose search paths include the 'ns2/' subdir.
+//
+// lib1.so is linked with DF_1_GLOBAL, so both it and the executable are added
+// to every namespace.
+//
+// namespace configuration ('*' indicates primary ns)
+//  - default: exe[*], lib1.so
+//  - ns2: exe, lib1.so[*], lib2.so[*]
+//
 TEST(dl, exec_with_ld_config_file) {
 #if defined(__BIONIC__)
   SKIP_WITH_HWASAN << "libclang_rt.hwasan is not found with custom ld config";
@@ -285,13 +293,28 @@ TEST(dl, exec_with_ld_config_file) {
   ExecTestHelper eth;
   eth.SetArgs({ helper.c_str(), nullptr });
   eth.SetEnv({ env.c_str(), nullptr });
-  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, "12345");
+  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0,
+          "foo lib1\n"
+          "lib1_call_funcs\n"
+          "foo lib1\n"
+          "bar lib2\n");
 #endif
 }
 
-// _lib3.so has same symbol as lib2.so but returns 54321. _lib3.so is
-// LD_PRELOADed. This test is to ensure LD_PRELOADed libs are available to
-// additional namespaces other than the default namespace.
+// lib3.so has same foo and bar symbols as lib2.so. lib3.so is LD_PRELOADed.
+// This test ensures that LD_PRELOADed libs are available to all namespaces.
+//
+// namespace configuration ('*' indicates primary ns)
+//  - default: exe[*], lib3.so[*], lib1.so
+//  - ns2: exe, lib3.so, lib1.so[*], lib2.so[*]
+//
+// Ensure that, in both namespaces, a call to foo calls the lib3.so symbol,
+// which then calls the lib1.so symbol using RTLD_NEXT. Ensure that RTLD_NEXT
+// finds nothing when called from lib1.so.
+//
+// For the bar symbol, lib3.so's primary namespace is the default namespace, but
+// lib2.so is not in the default namespace, so using RTLD_NEXT from lib3.so
+// doesn't find the symbol in lib2.so.
 TEST(dl, exec_with_ld_config_file_with_ld_preload) {
 #if defined(__BIONIC__)
   SKIP_WITH_HWASAN << "libclang_rt.hwasan is not found with custom ld config";
@@ -308,7 +331,17 @@ TEST(dl, exec_with_ld_config_file_with_ld_preload) {
   ExecTestHelper eth;
   eth.SetArgs({ helper.c_str(), nullptr });
   eth.SetEnv({ env.c_str(), env2.c_str(), nullptr });
-  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0, "54321");
+  eth.Run([&]() { execve(helper.c_str(), eth.GetArgs(), eth.GetEnv()); }, 0,
+          "foo lib3\n"
+          "foo lib1\n"
+          "lib1_call_funcs\n"
+          "foo lib3\n"
+          "foo lib1\n"
+          "bar lib3\n"
+          "lib3_call_funcs\n"
+          "foo lib3\n"
+          "foo lib1\n"
+          "bar lib3\n");
 #endif
 }
 

tests/libs/Android.bp

@@ -1478,6 +1478,8 @@ cc_test_library {
     srcs: ["ld_config_test_helper_lib1.cpp"],
     shared_libs: ["ld_config_test_helper_lib2"],
     relative_install_path: "bionic-loader-test-libs/ns2",
+    // Mark the library DF_1_GLOBAL so it is added to every linker namespace.
+    ldflags: ["-Wl,-z,global"]
 }
 
 cc_test_library {

tests/libs/ld_config_test_helper.cpp

@@ -22,7 +22,9 @@
 #endif
 #include <unistd.h>
 
-extern int get_value_from_lib();
+extern "C" void foo();
+void lib1_call_funcs();
+__attribute__((weak)) void lib3_call_funcs();
 
 int main() {
   bool skip_vdso_check = false;
@@ -45,6 +47,9 @@ int main() {
     dlclose(handle);
   }
 
-  printf("%d", get_value_from_lib());
+  foo();
+  lib1_call_funcs();
+  if (lib3_call_funcs) lib3_call_funcs();
+
   return 0;
 }

tests/libs/ld_config_test_helper_lib1.cpp

@@ -1,4 +1,19 @@
-extern int get_value_from_another_lib();
-int get_value_from_lib() {
-  return get_value_from_another_lib();
+#include <dlfcn.h>
+#include <stdio.h>
+
+// Mark foo and bar weak so that Clang allows the run-time linker to decide which DSO's symbol to
+// use.
+
+__attribute__((weak)) extern "C" void foo() {
+  printf("foo lib1\n");
+  void (*next)(void) = reinterpret_cast<void (*)()>(dlsym(RTLD_NEXT, "foo"));
+  if (next != nullptr) next();
+}
+
+__attribute__((weak)) extern "C" void bar();
+
+void lib1_call_funcs() {
+  printf("lib1_call_funcs\n");
+  foo();
+  bar();
 }

tests/libs/ld_config_test_helper_lib2.cpp

@@ -1,3 +1,11 @@
-int get_value_from_another_lib() {
-  return 12345;
+#include <dlfcn.h>
+#include <stdio.h>
+
+// Mark foo and bar weak so that Clang allows the run-time linker to decide which DSO's symbol to
+// use.
+
+__attribute__((weak)) extern "C" void bar() {
+  printf("bar lib2\n");
+  void (*next)(void) = reinterpret_cast<void (*)()>(dlsym(RTLD_NEXT, "bar"));
+  if (next != nullptr) next();
 }

tests/libs/ld_config_test_helper_lib3.cpp

@@ -1,3 +1,23 @@
-int get_value_from_another_lib() {
-  return 54321;
+#include <dlfcn.h>
+#include <stdio.h>
+
+// Mark foo and bar weak so that Clang allows the run-time linker to decide which DSO's symbol to
+// use.
+
+__attribute__((weak)) extern "C" void foo() {
+  printf("foo lib3\n");
+  void (*next)(void) = reinterpret_cast<void (*)()>(dlsym(RTLD_NEXT, "foo"));
+  if (next != nullptr) next();
+}
+
+__attribute__((weak)) extern "C" void bar() {
+  printf("bar lib3\n");
+  void (*next)(void) = reinterpret_cast<void (*)()>(dlsym(RTLD_NEXT, "bar"));
+  if (next != nullptr) next();
+}
+
+void lib3_call_funcs() {
+  printf("lib3_call_funcs\n");
+  foo();
+  bar();
 }