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());
/////////////////////////////////////////////////////////////////