Revert "Avoid loading external libraries from ARTs internal linker namespace."

This reverts commit 582448f29f2f2529202bf868d00ba5d3d3776bb6.

Reason for revert: breaks tests

Change-Id: I2e0b2a28d4644b314887673d4aef4f1094aea289
diff --git a/libnativebridge/Android.bp b/libnativebridge/Android.bp
index 8e87997..cb07d35 100644
--- a/libnativebridge/Android.bp
+++ b/libnativebridge/Android.bp
@@ -10,6 +10,9 @@
 cc_defaults {
     name: "libnativebridge-defaults",
     defaults: ["art_defaults"],
+    cppflags: [
+        "-fvisibility=protected",
+    ],
     header_libs: [
         "jni_headers",
         "libnativebridge-headers",
@@ -61,10 +64,10 @@
 
     target: {
         android: {
-            header_libs: [
-                "libnativeloader-headers", // For dlext_namespaces.h
-            ],
-            shared_libs: ["libdl_android"],
+            version_script: "libnativebridge.map.txt",
+        },
+        linux: {
+            version_script: "libnativebridge.map.txt",
         },
     },
 
diff --git a/libnativebridge/include/nativebridge/native_bridge.h b/libnativebridge/include/nativebridge/native_bridge.h
index 2199bab..e20b627 100644
--- a/libnativebridge/include/nativebridge/native_bridge.h
+++ b/libnativebridge/include/nativebridge/native_bridge.h
@@ -29,11 +29,6 @@
 extern "C" {
 #endif  // __cplusplus
 
-// Loads a shared library from the system linker namespace, suitable for
-// platform libraries in /system/lib(64). If linker namespaces don't exist (i.e.
-// on host), this simply calls dlopen().
-void* OpenSystemLibrary(const char* path, int flags);
-
 struct NativeBridgeRuntimeCallbacks;
 struct NativeBridgeRuntimeValues;
 
diff --git a/libnativebridge/native_bridge.cc b/libnativebridge/native_bridge.cc
index fb13d62..46a05a2 100644
--- a/libnativebridge/native_bridge.cc
+++ b/libnativebridge/native_bridge.cc
@@ -31,10 +31,6 @@
 #include <android-base/macros.h>
 #include <log/log.h>
 
-#ifdef ART_TARGET_ANDROID
-#include "nativeloader/dlext_namespaces.h"
-#endif
-
 namespace android {
 
 #ifdef __APPLE__
@@ -44,28 +40,6 @@
 
 extern "C" {
 
-void* OpenSystemLibrary(const char* path, int flags) {
-#ifdef ART_TARGET_ANDROID
-  // The system namespace is called "default" for binaries in /system and
-  // "system" for those in the Runtime APEX. Try "system" first since
-  // "default" always exists.
-  // TODO(b/185587109): Get rid of this error prone logic.
-  android_namespace_t* system_ns = android_get_exported_namespace("system");
-  if (system_ns == nullptr) {
-    system_ns = android_get_exported_namespace("default");
-    LOG_ALWAYS_FATAL_IF(system_ns == nullptr,
-                        "Failed to get system namespace for loading %s", path);
-  }
-  const android_dlextinfo dlextinfo = {
-      .flags = ANDROID_DLEXT_USE_NAMESPACE,
-      .library_namespace = system_ns,
-  };
-  return android_dlopen_ext(path, flags, &dlextinfo);
-#else
-  return dlopen(path, flags);
-#endif
-}
-
 // Environment values required by the apps running with native bridge.
 struct NativeBridgeRuntimeValues {
     const char* os_arch;
@@ -249,12 +223,8 @@
     if (!NativeBridgeNameAcceptable(nb_library_filename)) {
       CloseNativeBridge(true);
     } else {
-      // Try to open the library. We assume this library is provided by the
-      // platform rather than the ART APEX itself, so use the system namespace
-      // to avoid requiring a static linker config link to it from the
-      // com_android_art namespace.
-      void* handle = OpenSystemLibrary(nb_library_filename, RTLD_LAZY);
-
+      // Try to open the library.
+      void* handle = dlopen(nb_library_filename, RTLD_LAZY);
       if (handle != nullptr) {
         callbacks = reinterpret_cast<NativeBridgeCallbacks*>(dlsym(handle,
                                                                    kNativeBridgeInterfaceSymbol));
diff --git a/libnativeloader/Android.bp b/libnativeloader/Android.bp
index 2cc6b80..316c087 100644
--- a/libnativeloader/Android.bp
+++ b/libnativeloader/Android.bp
@@ -12,6 +12,9 @@
 cc_defaults {
     name: "libnativeloader-defaults",
     defaults: ["art_defaults"],
+    cppflags: [
+        "-fvisibility=hidden",
+    ],
     header_libs: ["libnativeloader-headers"],
     export_header_lib_headers: ["libnativeloader-headers"],
 }
@@ -118,32 +121,30 @@
 
 art_cc_test {
     name: "libnativeloader_test",
-    defaults: ["art_test_defaults"],
-    host_supported: false,
-
     srcs: [
         "native_loader_test.cpp",
+        "native_loader.cpp",
+        "library_namespaces.cpp",
+        "native_loader_namespace.cpp",
+        "public_libraries.cpp",
     ],
     cflags: ["-DANDROID"],
-
-    // The test mocks libdl_android and libnativebridge symbols, so export them
-    // to override the ones loaded from their libs.
-    ldflags: [
-        "-Wl,--export-dynamic-symbol=android_*",
-        "-Wl,--export-dynamic-symbol=NativeBridge*",
+    static_libs: [
+        "libbase",
+        "liblog",
+        "libgmock",
+        "PlatformProperties",
     ],
-
     header_libs: [
         "libnativebridge-headers",
         "libnativehelper_header_only",
+        "libnativeloader-headers",
     ],
-    static_libs: [
-        "libgmock",
+    // native_loader_test.cpp mocks libdl APIs so system_shared_libs
+    // are used to include C libraries without libdl.
+    system_shared_libs: [
+        "libc",
+        "libm",
     ],
-    shared_libs: [
-        "libbase",
-        "libnativeloader",
-    ],
-
     test_suites: ["device-tests"],
 }
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index 59369ee..79fee06 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -122,18 +122,16 @@
     return;
   }
 
-  // Load the preloadable public libraries. Since libnativeloader is in the
-  // com_android_art namespace, use OpenSystemLibrary rather than dlopen to
-  // ensure the libraries are loaded in the system namespace.
+  // android_init_namespaces() expects all the public libraries
+  // to be loaded so that they can be found by soname alone.
   //
   // TODO(dimitry): this is a bit misleading since we do not know
   // if the vendor public library is going to be opened from /vendor/lib
   // we might as well end up loading them from /system/lib or /product/lib
   // For now we rely on CTS test to catch things like this but
   // it should probably be addressed in the future.
-  for (const std::string& soname : android::base::Split(preloadable_public_libraries(), ":")) {
-    void* handle = OpenSystemLibrary(soname.c_str(), RTLD_NOW | RTLD_NODELETE);
-    LOG_ALWAYS_FATAL_IF(handle == nullptr,
+  for (const auto& soname : android::base::Split(preloadable_public_libraries(), ":")) {
+    LOG_ALWAYS_FATAL_IF(dlopen(soname.c_str(), RTLD_NOW | RTLD_NODELETE) == nullptr,
                         "Error preloading public library %s: %s", soname.c_str(), dlerror());
   }
 }
diff --git a/libnativeloader/native_loader.cpp b/libnativeloader/native_loader.cpp
index 30c7b5a..b34692a 100644
--- a/libnativeloader/native_loader.cpp
+++ b/libnativeloader/native_loader.cpp
@@ -254,11 +254,7 @@
       }
     }
 
-    // Fall back to the system namespace. This happens for preloaded JNI
-    // libraries in the zygote.
-    // TODO(b/185833744): Investigate if this should fall back to the app main
-    // namespace (aka anonymous namespace) instead.
-    void* handle = OpenSystemLibrary(path, RTLD_NOW);
+    void* handle = dlopen(path, RTLD_NOW);
     if (handle == nullptr) {
       *error_msg = strdup(dlerror());
     }
diff --git a/libnativeloader/native_loader_test.cpp b/libnativeloader/native_loader_test.cpp
index e754414..43c3c15 100644
--- a/libnativeloader/native_loader_test.cpp
+++ b/libnativeloader/native_loader_test.cpp
@@ -36,13 +36,12 @@
 namespace nativeloader {
 
 using ::testing::Eq;
-using ::testing::NotNull;
 using ::testing::Return;
 using ::testing::StrEq;
 using ::testing::_;
 using internal::ConfigEntry;
-using internal::ParseApexLibrariesConfig;
 using internal::ParseConfig;
+using internal::ParseApexLibrariesConfig;
 
 #if defined(__LP64__)
 #define LIB_DIR "lib64"
@@ -50,15 +49,19 @@
 #define LIB_DIR "lib"
 #endif
 
-// gmock interface that represents interested platform APIs on libdl_android and libnativebridge
+// gmock interface that represents interested platform APIs on libdl and libnativebridge
 class Platform {
  public:
   virtual ~Platform() {}
 
-  // These mock_* are the APIs semantically the same across libdl_android and libnativebridge.
+  // libdl APIs
+  virtual void* dlopen(const char* filename, int flags) = 0;
+  virtual int dlclose(void* handle) = 0;
+  virtual char* dlerror(void) = 0;
+
+  // These mock_* are the APIs semantically the same across libdl and libnativebridge.
   // Instead of having two set of mock APIs for the two, define only one set with an additional
-  // argument 'bool bridged' to identify the context (i.e., called for libdl_android or
-  // libnativebridge).
+  // argument 'bool bridged' to identify the context (i.e., called for libdl or libnativebridge).
   typedef char* mock_namespace_handle;
   virtual bool mock_init_anonymous_namespace(bool bridged, const char* sonames,
                                              const char* search_paths) = 0;
@@ -71,7 +74,7 @@
   virtual void* mock_dlopen_ext(bool bridged, const char* filename, int flags,
                                 mock_namespace_handle ns) = 0;
 
-  // libnativebridge APIs for which libdl_android has no corresponding APIs
+  // libnativebridge APIs for which libdl has no corresponding APIs
   virtual bool NativeBridgeInitialized() = 0;
   virtual const char* NativeBridgeGetError() = 0;
   virtual bool NativeBridgeIsPathSupported(const char*) = 0;
@@ -122,6 +125,11 @@
         }));
   }
 
+  // Mocking libdl APIs
+  MOCK_METHOD2(dlopen, void*(const char*, int));
+  MOCK_METHOD1(dlclose, int(void*));
+  MOCK_METHOD0(dlerror, char*());
+
   // Mocking the common APIs
   MOCK_METHOD3(mock_init_anonymous_namespace, bool(bool, const char*, const char*));
   MOCK_METHOD7(mock_create_namespace,
@@ -147,11 +155,19 @@
 
 static std::unique_ptr<MockPlatform> mock;
 
-// Provide C wrappers for the mock object. These symbols must be exported by ld
-// to be able to override the real symbols in the shared libs.
+// Provide C wrappers for the mock object.
 extern "C" {
+void* dlopen(const char* file, int flag) {
+  return mock->dlopen(file, flag);
+}
 
-// libdl_android APIs
+int dlclose(void* handle) {
+  return mock->dlclose(handle);
+}
+
+char* dlerror(void) {
+  return mock->dlerror();
+}
 
 bool android_init_anonymous_namespace(const char* sonames, const char* search_path) {
   return mock->mock_init_anonymous_namespace(false, sonames, search_path);
@@ -181,7 +197,6 @@
 }
 
 // libnativebridge APIs
-
 bool NativeBridgeIsSupported(const char* libpath) {
   return mock->NativeBridgeIsSupported(libpath);
 }
@@ -298,8 +313,7 @@
     std::vector<std::string> default_public_libs =
         android::base::Split(preloadable_public_libraries(), ":");
     for (auto l : default_public_libs) {
-      EXPECT_CALL(*mock,
-                  mock_dlopen_ext(false, StrEq(l.c_str()), RTLD_NOW | RTLD_NODELETE, NotNull()))
+      EXPECT_CALL(*mock, dlopen(StrEq(l.c_str()), RTLD_NOW | RTLD_NODELETE))
           .WillOnce(Return(any_nonnull));
     }
   }
@@ -322,76 +336,6 @@
   RunTest();
 }
 
-TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderInApex) {
-  const char* test_lib_path = "libfoo.so";
-  void* fake_handle = &fake_handle;  // Arbitrary non-null value
-  EXPECT_CALL(*mock,
-              mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("com_android_art")))
-      .WillOnce(Return(fake_handle));
-
-  bool needs_native_bridge = false;
-  char* errmsg = nullptr;
-  EXPECT_EQ(fake_handle,
-            OpenNativeLibrary(env.get(),
-                              /*target_sdk_version=*/17,
-                              test_lib_path,
-                              /*class_loader=*/nullptr,
-                              /*caller_location=*/"/apex/com.android.art/javalib/myloadinglib.jar",
-                              /*library_path=*/nullptr,
-                              &needs_native_bridge,
-                              &errmsg));
-  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
-  // should maybe change.
-  EXPECT_EQ(needs_native_bridge, false);
-  EXPECT_EQ(errmsg, nullptr);
-}
-
-TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderInFramework) {
-  const char* test_lib_path = "libfoo.so";
-  void* fake_handle = &fake_handle;  // Arbitrary non-null value
-  EXPECT_CALL(*mock, mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("system")))
-      .WillOnce(Return(fake_handle));
-
-  bool needs_native_bridge = false;
-  char* errmsg = nullptr;
-  EXPECT_EQ(fake_handle,
-            OpenNativeLibrary(env.get(),
-                              /*target_sdk_version=*/17,
-                              test_lib_path,
-                              /*class_loader=*/nullptr,
-                              /*caller_location=*/"/system/framework/framework.jar!classes1.dex",
-                              /*library_path=*/nullptr,
-                              &needs_native_bridge,
-                              &errmsg));
-  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
-  // should maybe change.
-  EXPECT_EQ(needs_native_bridge, false);
-  EXPECT_EQ(errmsg, nullptr);
-}
-
-TEST_P(NativeLoaderTest, OpenNativeLibraryWithoutClassloaderAndCallerLocation) {
-  const char* test_lib_path = "libfoo.so";
-  void* fake_handle = &fake_handle;  // Arbitrary non-null value
-  EXPECT_CALL(*mock, mock_dlopen_ext(false, StrEq(test_lib_path), RTLD_NOW, NsEq("system")))
-      .WillOnce(Return(fake_handle));
-
-  bool needs_native_bridge = false;
-  char* errmsg = nullptr;
-  EXPECT_EQ(fake_handle,
-            OpenNativeLibrary(env.get(),
-                              /*target_sdk_version=*/17,
-                              test_lib_path,
-                              /*class_loader=*/nullptr,
-                              /*caller_location=*/nullptr,
-                              /*library_path=*/nullptr,
-                              &needs_native_bridge,
-                              &errmsg));
-  // OpenNativeLibrary never uses nativebridge when there's no classloader. That
-  // should maybe change.
-  EXPECT_EQ(needs_native_bridge, false);
-  EXPECT_EQ(errmsg, nullptr);
-}
-
 INSTANTIATE_TEST_SUITE_P(NativeLoaderTests, NativeLoaderTest, testing::Bool());
 
 /////////////////////////////////////////////////////////////////