Merge "Ensure same order of global group members in all NS's"
diff --git a/linker/linker.cpp b/linker/linker.cpp
index c240c56..3488f5c 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -1242,7 +1242,7 @@
     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 @@
     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 @@
   });
 
   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 @@
 
     // 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 @@
     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
diff --git a/tests/dl_test.cpp b/tests/dl_test.cpp
index 6adba19..6c9bf3f 100644
--- a/tests/dl_test.cpp
+++ b/tests/dl_test.cpp
@@ -268,8 +268,16 @@
 }
 #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 @@
   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 @@
   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
 }
 
diff --git a/tests/libs/Android.bp b/tests/libs/Android.bp
index 385d120..ba1c198 100644
--- a/tests/libs/Android.bp
+++ b/tests/libs/Android.bp
@@ -1478,6 +1478,8 @@
     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 {
diff --git a/tests/libs/ld_config_test_helper.cpp b/tests/libs/ld_config_test_helper.cpp
index 87e512e..1249121 100644
--- a/tests/libs/ld_config_test_helper.cpp
+++ b/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 @@
     dlclose(handle);
   }
 
-  printf("%d", get_value_from_lib());
+  foo();
+  lib1_call_funcs();
+  if (lib3_call_funcs) lib3_call_funcs();
+
   return 0;
 }
diff --git a/tests/libs/ld_config_test_helper_lib1.cpp b/tests/libs/ld_config_test_helper_lib1.cpp
index fc5401a..ffa9a45 100644
--- a/tests/libs/ld_config_test_helper_lib1.cpp
+++ b/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();
 }
diff --git a/tests/libs/ld_config_test_helper_lib2.cpp b/tests/libs/ld_config_test_helper_lib2.cpp
index a620a6c..d5bca2c 100644
--- a/tests/libs/ld_config_test_helper_lib2.cpp
+++ b/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();
 }
diff --git a/tests/libs/ld_config_test_helper_lib3.cpp b/tests/libs/ld_config_test_helper_lib3.cpp
index 93d1cd8..94e1570 100644
--- a/tests/libs/ld_config_test_helper_lib3.cpp
+++ b/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();
 }