Count references for groups instead of instances
Count references on the group level to avoid
partially unloading function that might be
referenced by other libraries in the local_group
Bonus: with this change we can correctly unload recursively
linked libraries. is_recursive check is removed.
Also dynamic executables (not .so) with 0 DT_NEEDED libraries
are now correctly linked.
Change-Id: Idfa83baef402840599b93a875f2881d9f020dbcd
diff --git a/tests/dlfcn_test.cpp b/tests/dlfcn_test.cpp
index ea20869..c988d29 100644
--- a/tests/dlfcn_test.cpp
+++ b/tests/dlfcn_test.cpp
@@ -22,6 +22,7 @@
#include <stdio.h>
#include <stdint.h>
+#include "gtest_ex.h"
#include "private/ScopeGuard.h"
#include <string>
@@ -362,6 +363,48 @@
ASSERT_EQ(0, dlclose(handle));
}
+TEST(dlfcn, check_unload_after_reloc) {
+ // This is how this one works:
+ // libtest_two_parents_parent1 <- answer_impl() used by libtest_two_parents_child
+ // |
+ // +-> libtest_two_parents_child
+ //
+ // libtest_two_parents_parent2 <- answer_impl() not used by libtest_two_parents_child
+ // |
+ // +-> libtest_two_parents_child
+ //
+ // Test dlopens parent1 which loads and relocates libtest_two_parents_child.so
+ // as a second step it dlopens parent2 and dlcloses parent1...
+
+ test_isolated([] {
+ void* handle = dlopen("libtest_two_parents_parent1.so", RTLD_NOW | RTLD_LOCAL);
+ ASSERT_TRUE(handle != nullptr) << dlerror();
+
+ void* handle2 = dlopen("libtest_two_parents_parent2.so", RTLD_NOW | RTLD_LOCAL);
+ ASSERT_TRUE(handle2 != nullptr) << dlerror();
+
+ typedef int (*fn_t) (void);
+ fn_t fn = reinterpret_cast<fn_t>(dlsym(handle2, "check_order_reloc_get_answer"));
+ ASSERT_TRUE(fn != nullptr) << dlerror();
+ ASSERT_EQ(42, fn());
+
+ ASSERT_EQ(0, dlclose(handle));
+
+ handle = dlopen("libtest_two_parents_parent1.so", RTLD_NOW | RTLD_LOCAL | RTLD_NOLOAD);
+ ASSERT_TRUE(handle != nullptr);
+ ASSERT_EQ(0, dlclose(handle));
+
+ fn = reinterpret_cast<fn_t>(dlsym(handle2, "check_order_reloc_get_answer"));
+ ASSERT_TRUE(fn != nullptr) << dlerror();
+ ASSERT_EQ(42, fn());
+
+ ASSERT_EQ(0, dlclose(handle2));
+
+ handle = dlopen("libtest_two_parents_parent1.so", RTLD_NOW | RTLD_LOCAL | RTLD_NOLOAD);
+ ASSERT_TRUE(handle == nullptr);
+ });
+}
+
extern "C" int check_order_reloc_root_get_answer_impl() {
return 42;
}
@@ -442,25 +485,25 @@
// libtest_with_dependency_loop_b.so -> libtest_with_dependency_loop_c.so ->
// libtest_with_dependency_loop_a.so
TEST(dlfcn, dlopen_check_loop) {
- void* handle = dlopen("libtest_with_dependency_loop.so", RTLD_NOW);
-#if defined(__BIONIC__)
- ASSERT_TRUE(handle == nullptr);
- ASSERT_STREQ("dlopen failed: recursive link to \"libtest_with_dependency_loop_a.so\"", dlerror());
- // This symbol should never be exposed
- void* f = dlsym(RTLD_DEFAULT, "dlopen_test_invalid_function");
- ASSERT_TRUE(f == nullptr);
- ASSERT_SUBSTR("undefined symbol: dlopen_test_invalid_function", dlerror());
+ test_isolated([] {
+ void* handle = dlopen("libtest_with_dependency_loop.so", RTLD_NOW);
+ ASSERT_TRUE(handle != nullptr) << dlerror();
+ void* f = dlsym(handle, "dlopen_test_loopy_function");
+ ASSERT_TRUE(f != nullptr) << dlerror();
+ EXPECT_TRUE(reinterpret_cast<bool (*)(void)>(f)());
+ ASSERT_EQ(0, dlclose(handle));
- // dlopen second time to make sure that the library wasn't loaded even though dlopen returned null.
- // This may happen if during cleanup the root library or one of the depended libs were not removed
- // from soinfo list.
- handle = dlopen("libtest_with_dependency_loop.so", RTLD_NOW | RTLD_NOLOAD);
- ASSERT_TRUE(handle == nullptr);
- ASSERT_STREQ("dlopen failed: library \"libtest_with_dependency_loop.so\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror());
-#else // glibc allows recursive links
- ASSERT_TRUE(handle != nullptr);
- dlclose(handle);
+ // dlopen second time to make sure that the library was unloaded correctly
+ handle = dlopen("libtest_with_dependency_loop.so", RTLD_NOW | RTLD_NOLOAD);
+ ASSERT_TRUE(handle == nullptr);
+#ifdef __BIONIC__
+ // TODO: glibc returns nullptr on dlerror() here. Is it bug?
+ ASSERT_STREQ("dlopen failed: library \"libtest_with_dependency_loop.so\" wasn't loaded and RTLD_NOLOAD prevented it", dlerror());
#endif
+
+ handle = dlopen("libtest_with_dependency_a.so", RTLD_NOW | RTLD_NOLOAD);
+ ASSERT_TRUE(handle == nullptr);
+ });
}
TEST(dlfcn, dlopen_nodelete) {
diff --git a/tests/libs/Android.build.dlopen_2_parents_reloc.mk b/tests/libs/Android.build.dlopen_2_parents_reloc.mk
new file mode 100644
index 0000000..29ae10d
--- /dev/null
+++ b/tests/libs/Android.build.dlopen_2_parents_reloc.mk
@@ -0,0 +1,52 @@
+#
+# Copyright (C) 2014 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# -----------------------------------------------------------------------------
+# Libraries used by dlfcn tests to verify local group ref_counting
+# libtest_two_parents*.so
+# -----------------------------------------------------------------------------
+
+# -----------------------------------------------------------------------------
+# ..._child.so - correct answer
+# -----------------------------------------------------------------------------
+libtest_two_parents_child_src_files := \
+ dlopen_2_parents_reloc_answer.cpp
+
+module := libtest_two_parents_child
+include $(LOCAL_PATH)/Android.build.testlib.mk
+
+# -----------------------------------------------------------------------------
+# ..._parent1.so - correct answer
+# -----------------------------------------------------------------------------
+libtest_two_parents_parent1_src_files := \
+ dlopen_check_order_reloc_answer_impl.cpp
+
+libtest_two_parents_parent1_shared_libraries := libtest_two_parents_child
+libtest_two_parents_parent1_cflags := -D__ANSWER=42
+module := libtest_two_parents_parent1
+include $(LOCAL_PATH)/Android.build.testlib.mk
+
+# -----------------------------------------------------------------------------
+# ..._parent2.so - incorrect answer
+# -----------------------------------------------------------------------------
+libtest_two_parents_parent2_src_files := \
+ dlopen_check_order_reloc_answer_impl.cpp
+
+libtest_two_parents_parent2_shared_libraries := libtest_two_parents_child
+libtest_two_parents_parent2_cflags := -D__ANSWER=1
+module := libtest_two_parents_parent2
+include $(LOCAL_PATH)/Android.build.testlib.mk
+
diff --git a/tests/libs/Android.mk b/tests/libs/Android.mk
index fafb9e0..e4620f5 100644
--- a/tests/libs/Android.mk
+++ b/tests/libs/Android.mk
@@ -21,6 +21,7 @@
common_additional_dependencies := \
$(LOCAL_PATH)/Android.mk \
$(LOCAL_PATH)/Android.build.dlext_testzip.mk \
+ $(LOCAL_PATH)/Android.build.dlopen_2_parents_reloc.mk \
$(LOCAL_PATH)/Android.build.dlopen_check_order_dlsym.mk \
$(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_siblings.mk \
$(LOCAL_PATH)/Android.build.dlopen_check_order_reloc_main_executable.mk \
@@ -166,6 +167,11 @@
include $(LOCAL_PATH)/Android.build.testlib.mk
# -----------------------------------------------------------------------------
+# Build library with two parents
+# -----------------------------------------------------------------------------
+include $(LOCAL_PATH)/Android.build.dlopen_2_parents_reloc.mk
+
+# -----------------------------------------------------------------------------
# Build libtest_check_order_dlsym.so with its dependencies.
# -----------------------------------------------------------------------------
include $(LOCAL_PATH)/Android.build.dlopen_check_order_dlsym.mk
@@ -185,7 +191,7 @@
#
# libtest_with_dependency_loop -> a -> b -> c -> a
# -----------------------------------------------------------------------------
-libtest_with_dependency_loop_src_files := dlopen_testlib_invalid.cpp
+libtest_with_dependency_loop_src_files := dlopen_testlib_loopy_root.cpp
libtest_with_dependency_loop_shared_libraries := \
libtest_with_dependency_loop_a
@@ -196,7 +202,7 @@
# -----------------------------------------------------------------------------
# libtest_with_dependency_loop_a.so
# -----------------------------------------------------------------------------
-libtest_with_dependency_loop_a_src_files := dlopen_testlib_invalid.cpp
+libtest_with_dependency_loop_a_src_files := dlopen_testlib_loopy_a.cpp
libtest_with_dependency_loop_a_shared_libraries := \
libtest_with_dependency_loop_b_tmp
@@ -209,7 +215,7 @@
#
# this is temporary placeholder - will be removed
# -----------------------------------------------------------------------------
-libtest_with_dependency_loop_b_tmp_src_files := dlopen_testlib_invalid.cpp
+libtest_with_dependency_loop_b_tmp_src_files := dlopen_testlib_loopy_invalid.cpp
libtest_with_dependency_loop_b_tmp_ldflags := -Wl,-soname=libtest_with_dependency_loop_b.so
module := libtest_with_dependency_loop_b_tmp
@@ -218,7 +224,7 @@
# -----------------------------------------------------------------------------
# libtest_with_dependency_loop_b.so
# -----------------------------------------------------------------------------
-libtest_with_dependency_loop_b_src_files := dlopen_testlib_invalid.cpp
+libtest_with_dependency_loop_b_src_files := dlopen_testlib_loopy_b.cpp
libtest_with_dependency_loop_b_shared_libraries := libtest_with_dependency_loop_c
module := libtest_with_dependency_loop_b
@@ -227,7 +233,7 @@
# -----------------------------------------------------------------------------
# libtest_with_dependency_loop_c.so
# -----------------------------------------------------------------------------
-libtest_with_dependency_loop_c_src_files := dlopen_testlib_invalid.cpp
+libtest_with_dependency_loop_c_src_files := dlopen_testlib_loopy_c.cpp
libtest_with_dependency_loop_c_shared_libraries := \
libtest_with_dependency_loop_a
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_2_parents_reloc_answer.cpp
similarity index 69%
copy from tests/libs/dlopen_testlib_invalid.cpp
copy to tests/libs/dlopen_2_parents_reloc_answer.cpp
index f2039c6..036670b 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_2_parents_reloc_answer.cpp
@@ -14,11 +14,10 @@
* limitations under the License.
*/
-#include <stdlib.h>
+extern "C" int __attribute__((weak)) check_order_reloc_get_answer_impl() {
+ return 0;
+}
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
- abort();
+extern "C" int check_order_reloc_get_answer() {
+ return check_order_reloc_get_answer_impl();
}
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_testlib_loopy_a.cpp
similarity index 71%
copy from tests/libs/dlopen_testlib_invalid.cpp
copy to tests/libs/dlopen_testlib_loopy_a.cpp
index f2039c6..4c08764 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_testlib_loopy_a.cpp
@@ -16,9 +16,10 @@
#include <stdlib.h>
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
- abort();
+extern "C" bool __attribute__((weak)) dlopen_test_loopy_function_impl() {
+ return false;
+}
+
+extern "C" bool dlopen_test_loopy_function() {
+ return dlopen_test_loopy_function_impl();
}
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_testlib_loopy_b.cpp
similarity index 71%
copy from tests/libs/dlopen_testlib_invalid.cpp
copy to tests/libs/dlopen_testlib_loopy_b.cpp
index f2039c6..01dcda9 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_testlib_loopy_b.cpp
@@ -16,9 +16,6 @@
#include <stdlib.h>
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
- abort();
+extern "C" bool dlopen_test_loopy_function_impl() {
+ return false;
}
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_testlib_loopy_c.cpp
similarity index 71%
copy from tests/libs/dlopen_testlib_invalid.cpp
copy to tests/libs/dlopen_testlib_loopy_c.cpp
index f2039c6..01dcda9 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_testlib_loopy_c.cpp
@@ -16,9 +16,6 @@
#include <stdlib.h>
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
- abort();
+extern "C" bool dlopen_test_loopy_function_impl() {
+ return false;
}
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_testlib_loopy_invalid.cpp
similarity index 72%
rename from tests/libs/dlopen_testlib_invalid.cpp
rename to tests/libs/dlopen_testlib_loopy_invalid.cpp
index f2039c6..5aa11f8 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_testlib_loopy_invalid.cpp
@@ -16,9 +16,8 @@
#include <stdlib.h>
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
+// This library should never be loaded
+static void __attribute__((constructor)) panic() {
abort();
}
+
diff --git a/tests/libs/dlopen_testlib_invalid.cpp b/tests/libs/dlopen_testlib_loopy_root.cpp
similarity index 71%
copy from tests/libs/dlopen_testlib_invalid.cpp
copy to tests/libs/dlopen_testlib_loopy_root.cpp
index f2039c6..c9459f0 100644
--- a/tests/libs/dlopen_testlib_invalid.cpp
+++ b/tests/libs/dlopen_testlib_loopy_root.cpp
@@ -16,9 +16,6 @@
#include <stdlib.h>
-// This file is used for libraries that are not supposed to
-// be successfully loaded/linked - therefore, this function should
-// not be visible via dlsym - (we are going to use this fact in tests)
-extern "C" int dlopen_test_invalid_function() {
- abort();
+extern "C" bool dlopen_test_loopy_function_impl() {
+ return true;
}