In run tests, do not rely on loading native libs in the ART namespace
with an allow_all_shared_libs fallback to system.

The system and com_android_art namespaces need to be properly
separated, but run tests have relied on loading test libraries through
either LD_LIBRARY_PATH or java.library.path without a clear separation.
That has worked through a combination of
ANDROID_ADDITIONAL_PUBLIC_LIBRARIES and fallback dlopen() calls that
used the ART namespace.

This change introduces a new directory
/data/nativetest(64)/com.android.art for test libraries that depend on
internal ART libraries. It's added with LD_LIBRARY_PATH to the default
namespace, which in the APEX linker config has full access to
com_android_art.

Normal JNI libraries that don't depend on ART internals stay in
/data/nativetest(64)/art/<arch>. There should be no overlap between the
two locations.

A new environment variable NATIVELOADER_DEFAULT_NAMESPACE_LIBS is
introduced to list the libraries added through LD_LIBRARY_PATH, so
libnativeloader can link to them from classloader namespaces and in the
fallback namespace when no classloader is specified.

Like ANDROID_ADDITIONAL_PUBLIC_LIBRARIES,
NATIVELOADER_DEFAULT_NAMESPACE_LIBS is only effective when
ro.debuggable is true.

A new cc_defaults "art_test_internal_library_defaults" is added to
Android.bp, to be used in libraries that should be installed in the new
com.android.art directory.

Some run tests that are special and need more treatment are disabled
for now, to be addressed in b/186654484.

Test: art/test/testrunner/testrunner.py --target --64 --optimizing
Test: art/test/testrunner/testrunner.py --target --32 --jit
Test: art/test/testrunner/testrunner.py --host --64 --optimizing
Bug: 130340935
Change-Id: Iec640d5e22b46af2c1a4d375ce3f06c57b1d224e
diff --git a/libnativeloader/native_loader.cpp b/libnativeloader/native_loader.cpp
index 64d60ea..b34692a 100644
--- a/libnativeloader/native_loader.cpp
+++ b/libnativeloader/native_loader.cpp
@@ -29,7 +29,9 @@
 
 #include <android-base/file.h>
 #include <android-base/macros.h>
+#include <android-base/properties.h>
 #include <android-base/strings.h>
+#include <android-base/thread_annotations.h>
 #include <nativebridge/native_bridge.h>
 #include <nativehelper/scoped_utf_chars.h>
 
@@ -42,11 +44,36 @@
 namespace android {
 
 namespace {
+
 #if defined(ART_TARGET_ANDROID)
+
+// NATIVELOADER_DEFAULT_NAMESPACE_LIBS is an environment variable that can be
+// used when ro.debuggable is true to list extra libraries (separated by ":")
+// that libnativeloader will load from the default namespace. The libraries must
+// be listed without paths, and then LD_LIBRARY_PATH is typically set to the
+// directories to load them from. The libraries will be available in all
+// classloader namespaces, and also in the fallback namespace used when no
+// classloader is given.
+//
+// kNativeloaderExtraLibs is the name of that fallback namespace.
+//
+// NATIVELOADER_DEFAULT_NAMESPACE_LIBS is intended to be used for testing only,
+// and in particular in the ART run tests that are executed through dalvikvm in
+// the APEX. In that case the default namespace links to the ART namespace
+// (com_android_art) for all libraries, which means this can be used to load
+// test libraries that depend on ART internal libraries.
+constexpr const char* kNativeloaderExtraLibs = "nativeloader-extra-libs";
+
+bool Debuggable() {
+  static bool debuggable = android::base::GetBoolProperty("ro.debuggable", false);
+  return debuggable;
+}
+
 using android::nativeloader::LibraryNamespaces;
 
 std::mutex g_namespaces_mutex;
 LibraryNamespaces* g_namespaces = new LibraryNamespaces;
+NativeLoaderNamespace* g_nativeloader_extra_libs_namespace = nullptr;
 
 android_namespace_t* FindExportedNamespace(const char* caller_location) {
   auto name = nativeloader::FindApexNamespaceName(caller_location);
@@ -58,7 +85,98 @@
   }
   return nullptr;
 }
+
+Result<void> CreateNativeloaderDefaultNamespaceLibsLink(NativeLoaderNamespace& ns)
+    REQUIRES(g_namespaces_mutex) {
+  const char* links = getenv("NATIVELOADER_DEFAULT_NAMESPACE_LIBS");
+  if (links == nullptr || *links == 0) {
+    return {};
+  }
+  // Pass nullptr to Link() to create a link to the default namespace without
+  // requiring it to be visible.
+  return ns.Link(nullptr, links);
+}
+
+Result<NativeLoaderNamespace*> GetNativeloaderExtraLibsNamespace() REQUIRES(g_namespaces_mutex) {
+  if (g_nativeloader_extra_libs_namespace != nullptr) {
+    return g_nativeloader_extra_libs_namespace;
+  }
+
+  Result<NativeLoaderNamespace> ns =
+      NativeLoaderNamespace::Create(kNativeloaderExtraLibs,
+                                    /*search_paths=*/"",
+                                    /*permitted_paths=*/"",
+                                    /*parent=*/nullptr,
+                                    /*is_shared=*/false,
+                                    /*is_exempt_list_enabled=*/false,
+                                    /*also_used_as_anonymous=*/false);
+  if (!ns.ok()) {
+    return ns.error();
+  }
+  g_nativeloader_extra_libs_namespace = new NativeLoaderNamespace(std::move(ns.value()));
+  Result<void> linked =
+      CreateNativeloaderDefaultNamespaceLibsLink(*g_nativeloader_extra_libs_namespace);
+  if (!linked.ok()) {
+    return linked.error();
+  }
+  return g_nativeloader_extra_libs_namespace;
+}
+
+// If the given path matches a library in NATIVELOADER_DEFAULT_NAMESPACE_LIBS
+// then load it in the nativeloader-extra-libs namespace, otherwise return
+// nullptr without error. This is only enabled if the ro.debuggable is true.
+Result<void*> TryLoadNativeloaderExtraLib(const char* path) {
+  if (!Debuggable()) {
+    return nullptr;
+  }
+  const char* links = getenv("NATIVELOADER_DEFAULT_NAMESPACE_LIBS");
+  if (links == nullptr || *links == 0) {
+    return nullptr;
+  }
+  std::vector<std::string> lib_list = base::Split(links, ":");
+  if (std::find(lib_list.begin(), lib_list.end(), path) == lib_list.end()) {
+    return nullptr;
+  }
+
+  std::lock_guard<std::mutex> guard(g_namespaces_mutex);
+  Result<NativeLoaderNamespace*> ns = GetNativeloaderExtraLibsNamespace();
+  if (!ns.ok()) {
+    return ns.error();
+  }
+  return ns.value()->Load(path);
+}
+
+Result<NativeLoaderNamespace*> CreateClassLoaderNamespaceLocked(JNIEnv* env,
+                                                                int32_t target_sdk_version,
+                                                                jobject class_loader,
+                                                                bool is_shared,
+                                                                jstring dex_path,
+                                                                jstring library_path,
+                                                                jstring permitted_path,
+                                                                jstring uses_library_list)
+    REQUIRES(g_namespaces_mutex) {
+  Result<NativeLoaderNamespace*> ns = g_namespaces->Create(env,
+                                                           target_sdk_version,
+                                                           class_loader,
+                                                           is_shared,
+                                                           dex_path,
+                                                           library_path,
+                                                           permitted_path,
+                                                           uses_library_list);
+  if (!ns.ok()) {
+    return ns;
+  }
+  if (Debuggable()) {
+    Result<void> linked = CreateNativeloaderDefaultNamespaceLibsLink(*ns.value());
+    if (!linked.ok()) {
+      return linked.error();
+    }
+  }
+  return ns;
+}
+
 #endif  // #if defined(ART_TARGET_ANDROID)
+
 }  // namespace
 
 void InitializeNativeLoader() {
@@ -72,6 +190,8 @@
 #if defined(ART_TARGET_ANDROID)
   std::lock_guard<std::mutex> guard(g_namespaces_mutex);
   g_namespaces->Reset();
+  delete(g_nativeloader_extra_libs_namespace);
+  g_nativeloader_extra_libs_namespace = nullptr;
 #endif
 }
 
@@ -80,8 +200,14 @@
                                    jstring permitted_path, jstring uses_library_list) {
 #if defined(ART_TARGET_ANDROID)
   std::lock_guard<std::mutex> guard(g_namespaces_mutex);
-  auto ns = g_namespaces->Create(env, target_sdk_version, class_loader, is_shared, dex_path,
-                                 library_path, permitted_path, uses_library_list);
+  Result<NativeLoaderNamespace*> ns = CreateClassLoaderNamespaceLocked(env,
+                                                                       target_sdk_version,
+                                                                       class_loader,
+                                                                       is_shared,
+                                                                       dex_path,
+                                                                       library_path,
+                                                                       permitted_path,
+                                                                       uses_library_list);
   if (!ns.ok()) {
     return env->NewStringUTF(ns.error().message().c_str());
   }
@@ -97,6 +223,7 @@
                         bool* needs_native_bridge, char** error_msg) {
 #if defined(ART_TARGET_ANDROID)
   UNUSED(target_sdk_version);
+
   if (class_loader == nullptr) {
     *needs_native_bridge = false;
     if (caller_location != nullptr) {
@@ -113,6 +240,20 @@
         return handle;
       }
     }
+
+    // Check if the library is in NATIVELOADER_DEFAULT_NAMESPACE_LIBS and should
+    // be loaded from the kNativeloaderExtraLibs namespace.
+    {
+      Result<void*> handle = TryLoadNativeloaderExtraLib(path);
+      if (!handle.ok()) {
+        *error_msg = strdup(handle.error().message().c_str());
+        return nullptr;
+      }
+      if (handle.value() != nullptr) {
+        return handle.value();
+      }
+    }
+
     void* handle = dlopen(path, RTLD_NOW);
     if (handle == nullptr) {
       *error_msg = strdup(dlerror());
@@ -127,8 +268,14 @@
     // This is the case where the classloader was not created by ApplicationLoaders
     // In this case we create an isolated not-shared namespace for it.
     Result<NativeLoaderNamespace*> isolated_ns =
-        g_namespaces->Create(env, target_sdk_version, class_loader, false /* is_shared */, nullptr,
-                             library_path, nullptr, nullptr);
+        CreateClassLoaderNamespaceLocked(env,
+                                         target_sdk_version,
+                                         class_loader,
+                                         /*is_shared=*/false,
+                                         /*dex_path=*/nullptr,
+                                         library_path,
+                                         /*permitted_path=*/nullptr,
+                                         /*uses_library_list=*/nullptr);
     if (!isolated_ns.ok()) {
       *error_msg = strdup(isolated_ns.error().message().c_str());
       return nullptr;
@@ -238,6 +385,24 @@
   std::lock_guard<std::mutex> guard(g_namespaces_mutex);
   return g_namespaces->FindNamespaceByClassLoader(env, class_loader);
 }
-#endif
+
+void LinkNativeLoaderNamespaceToExportedNamespaceLibrary(struct NativeLoaderNamespace* ns,
+                                                         const char* exported_ns_name,
+                                                         const char* library_name,
+                                                         char** error_msg) {
+  Result<NativeLoaderNamespace> exported_ns =
+      NativeLoaderNamespace::GetExportedNamespace(exported_ns_name, ns->IsBridged());
+  if (!exported_ns.ok()) {
+    *error_msg = strdup(exported_ns.error().message().c_str());
+    return;
+  }
+
+  Result<void> linked = ns->Link(&exported_ns.value(), std::string(library_name));
+  if (!linked.ok()) {
+    *error_msg = strdup(linked.error().message().c_str());
+  }
+}
+
+#endif  // ART_TARGET_ANDROID
 
 };  // namespace android
diff --git a/test/Android.bp b/test/Android.bp
index bc340bd..d71bd06 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -35,6 +35,8 @@
                 "com.android.art.debug",
             ],
         },
+        // The following set of relative_install_paths installs the library in a
+        // location that will be available as java.library.path in run tests.
         android_arm: {
             relative_install_path: "art/arm",
         },
@@ -61,6 +63,32 @@
     ],
 }
 
+// Variant of art_test_defaults that installs the library in a location which
+// will be added to the default namespace, and hence also the com_android_art
+// namespace. That allows the library to have shared_libs dependencies on all
+// ART internal libraries.
+//
+// Currently this only works for run tests where run-test-jar sets
+// LD_LIBRARY_PATH and NATIVELOADER_DEFAULT_NAMESPACE_LIBS.
+art_cc_defaults {
+    name: "art_test_internal_library_defaults",
+    defaults: ["art_test_defaults"],
+    target: {
+        android_arm: {
+            relative_install_path: "com.android.art/lib",
+        },
+        android_arm64: {
+            relative_install_path: "com.android.art/lib64",
+        },
+        android_x86: {
+            relative_install_path: "com.android.art/lib",
+        },
+        android_x86_64: {
+            relative_install_path: "com.android.art/lib64",
+        },
+    },
+}
+
 art_cc_defaults {
     name: "art_gtest_defaults",
     // These really are gtests, but the gtest library comes from libart-gtest.so
@@ -314,7 +342,7 @@
 cc_defaults {
     name: "libartagent-defaults",
     defaults: [
-        "art_test_defaults",
+        "art_test_internal_library_defaults",
         "art_defaults",
     ],
     shared_libs: [
@@ -354,7 +382,7 @@
 art_cc_defaults {
     name: "libtiagent-base-defaults",
     defaults: [
-        "art_test_defaults",
+        "art_test_internal_library_defaults",
         "art_defaults",
         // Not derived from libartagent-defaults for NDK.
     ],
@@ -619,7 +647,7 @@
 cc_defaults {
     name: "libarttest-defaults",
     defaults: [
-        "art_test_defaults",
+        "art_test_internal_library_defaults",
         "art_defaults",
     ],
     srcs: [
diff --git a/test/etc/run-test-jar b/test/etc/run-test-jar
index fb88c3a..86eb722 100755
--- a/test/etc/run-test-jar
+++ b/test/etc/run-test-jar
@@ -53,6 +53,7 @@
 INVOKE_WITH=""
 IS_JVMTI_TEST="n"
 ADD_LIBDIR_ARGUMENTS="n"
+SUFFIX64=""
 ISA=x86
 LIBRARY_DIRECTORY="lib"
 TEST_DIRECTORY="nativetest"
@@ -421,6 +422,7 @@
         shift
         break
     elif [ "x$1" = "x--64" ]; then
+        SUFFIX64="64"
         ISA="x86_64"
         GDBSERVER_DEVICE="gdbserver64"
         DALVIKVM="dalvikvm64"
@@ -631,14 +633,7 @@
   if [[ "$USE_JVM" = "y" ]]; then
     FLAGS="${FLAGS} -agentpath:${ANDROID_HOST_OUT}/nativetest64/${agent}=${TEST_NAME},jvm"
   else
-    if [[ "$HOST" = "y" ]]; then
-      FLAGS="${FLAGS} -agentpath:${agent}=${TEST_NAME},art"
-    else
-      # The linker configuration used for dalvikvm(64) in the ART APEX
-      # requires us to pass the full path to the agent to the runtime when
-      # running on device.
-      FLAGS="${FLAGS} -agentpath:/data/${TEST_DIRECTORY}/art/${ISA}/${agent}=${TEST_NAME},art"
-    fi
+    FLAGS="${FLAGS} -agentpath:${agent}=${TEST_NAME},art"
   fi
 fi
 
@@ -669,14 +664,7 @@
   if [[ "$USE_JVM" = "y" ]]; then
     FLAGS="${FLAGS} -agentpath:${ANDROID_HOST_OUT}/nativetest64/${agent}=${agent_args}"
   else
-    if [[ "$HOST" = "y" ]]; then
-      FLAGS="${FLAGS} -agentpath:${agent}=${agent_args}"
-    else
-      # The linker configuration used for dalvikvm(64) in the ART APEX
-      # requires us to pass the full path to the agent to the runtime when
-      # running on device.
-      FLAGS="${FLAGS} -agentpath:/data/${TEST_DIRECTORY}/art/${ISA}/${agent}=${agent_args}"
-    fi
+    FLAGS="${FLAGS} -agentpath:${agent}=${agent_args}"
   fi
 fi
 
@@ -1160,10 +1148,24 @@
       # installation.
       LD_LIBRARY_PATH="$ANDROID_ROOT/$LIBRARY_DIRECTORY"
     fi
-    # Needed to access libarttest(d).so and JVMTI agent libraries.
-    LD_LIBRARY_PATH="/data/$TEST_DIRECTORY/art/$ISA:$LD_LIBRARY_PATH"
-    # Needed to access the boot (core) image files.
-    LD_LIBRARY_PATH="/apex/com.android.art/javalib/$ISA:$LD_LIBRARY_PATH"
+
+    # This adds libarttest(d).so to the default linker namespace when dalvikvm
+    # is run from /apex/com.android.art/bin. Since that namespace is essentially
+    # an alias for the com_android_art namespace, that gives libarttest(d).so
+    # full access to the internal ART libraries.
+    LD_LIBRARY_PATH="/data/$TEST_DIRECTORY/com.android.art/lib${SUFFIX64}:$LD_LIBRARY_PATH"
+    if [ "$TEST_IS_NDEBUG" = "y" ]; then dlib=""; else dlib="d"; fi
+    art_test_internal_libraries=(
+      libartagent${dlib}.so
+      libarttest${dlib}.so
+      libtiagent${dlib}.so
+      libtistress${dlib}.so
+    )
+    art_test_internal_libraries="${art_test_internal_libraries[*]}"
+    NATIVELOADER_DEFAULT_NAMESPACE_LIBS="${art_test_internal_libraries// /:}"
+    dlib=
+    art_test_internal_libraries=
+
     # Needed to access the test's Odex files.
     LD_LIBRARY_PATH="$DEX_LOCATION/oat/$ISA:$LD_LIBRARY_PATH"
     # Needed to access the test's native libraries (see e.g. 674-hiddenapi,
@@ -1242,6 +1244,7 @@
              rm -rf ${DEX_LOCATION}/dalvik-cache/ && \
              mkdir -p ${mkdir_locations} && \
              export LD_LIBRARY_PATH=$LD_LIBRARY_PATH && \
+             export NATIVELOADER_DEFAULT_NAMESPACE_LIBS=$NATIVELOADER_DEFAULT_NAMESPACE_LIBS && \
              export PATH=$PREPEND_TARGET_PATH:\$PATH && \
              $profman_cmdline && \
              $dex2oat_cmdline && \
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 9b21cf8..6034b94 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1394,5 +1394,14 @@
         "tests": ["692-vdex-secondary-loader"],
         "env_vars": {"ART_USE_READ_BARRIER": "false"},
         "description": ["Uses the low-ram flag which does not work with CMS"]
+    },
+    {
+        "tests": ["150-loadlibrary",
+                  "656-annotation-lookup-generic-jni",
+                  "674-hiddenapi",
+                  "817-hiddenapi",
+                  "900-hello-plugin"],
+        "variant": "target",
+        "description": ["TODO(b/186654484): Disabled after the switch to avoid allow_all_shared_libs from the ART namespace to system."]
     }
 ]
diff --git a/test/run-test b/test/run-test
index f2e2a81..9352314 100755
--- a/test/run-test
+++ b/test/run-test
@@ -686,6 +686,10 @@
         run_args+=(--runtime-option "-Djava.library.path=${host_lib_root}/lib${suffix64}:${host_lib_root}/nativetest${suffix64}")
     else
         guess_target_arch_name
+        # Note that libarttest(d).so and other test libraries that depend on ART
+        # internal libraries must not be in this path for JNI libraries - they
+        # need to be loaded through LD_LIBRARY_PATH and
+        # NATIVELOADER_DEFAULT_NAMESPACE_LIBS instead.
         run_args+=(--runtime-option "-Djava.library.path=/data/nativetest${suffix64}/art/${target_arch_name}")
         run_args+=(--boot "/apex/com.android.art/javalib/boot.art")
     fi