[PATCH 1/3] Add dlsym tests

[Zhiting Zhu]

This patch includes a module called dl_tests which clones the bionic
and compile the needed library files.

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---
.gitignore | 2 +
modules/dl_tests/Makefile | 112 ++++++++++++++++++
modules/tests/Makefile | 2 +
modules/tests/module.py | 1 +
tests/tst-dlfcn.cc | 243 ++++++++++++++++++++++++++++++++++++++
5 files changed, 360 insertions(+)
create mode 100644 modules/dl_tests/Makefile

diff --git a/.gitignore b/.gitignore
index e0a76dc2..2dcb3225 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,4 +46,6 @@ modules/httpserver-jvm-plugin/obj/
modules/libtools/*.so
modules/libtools/*.o
modules/libyaml/usr.manifest
+modules/dl_tests/bionic/
+modules/dl_tests/usr.manifest
.idea
diff --git a/modules/dl_tests/Makefile b/modules/dl_tests/Makefile
new file mode 100644
index 00000000..da48e15c
--- /dev/null
+++ b/modules/dl_tests/Makefile
@@ -0,0 +1,112 @@
+module: usr.manifest build_all
+
+src := $(OSV_BASE)
+out := $(OSV_BUILD_PATH)
+arch := $(ARCH)
+fs_type := $(fs_type)
+bionic_test_libs := $(src)/modules/dl_tests/bionic/tests/libs
+
+quiet = $(if $V, $1, @echo " $2"; $1)
+very-quiet = $(if $V, $1, @$1)
+makedir = $(call very-quiet, mkdir -p $(dir $@))
+
+autodepend = -MD -MT $@ -MP
+
+INCLUDES = -I$(src)/arch/$(ARCH) -I$(src) -I$(src)/include \
+ -I$(src)/arch/common -isystem $(src)/include/glibc-compat \
+ $(shell $(CXX) -E -xc++ - -v </dev/null 2>&1 | awk '/^End/ {exit} /^ .*c\+\+/ {print "-isystem" $$0}') \
+ -isystem $(src)/include/api -isystem $(src)/include/api/$(ARCH) \
+ -isystem $(out)/gen/include
+
+COMMON = $(autodepend) $(INCLUDES) -g -O2 -fPIC -DBOOST_TEST_DYN_LINK \
+ -U _FORTIFY_SOURCE -D_KERNEL -D__OSV__ -DCONF_debug_memory=0 \
+ -Wall -Wno-pointer-arith -Wformat=0 -Wno-format-security
+
+LIBS =
+
+CXXFLAGS = -std=gnu++11 $(COMMON)
+CFLAGS = -std=gnu99 $(COMMON)
+
+tests := libtest_simple.so libtest_empty.so libtest_dlsym_from_this_grandchild.so \
+ libtest_dlsym_from_this_child.so libtest_dlsym_from_this.so libdlext_test.so \
+ libtest_with_dependency.so libtest_check_rtld_next_from_library.so
+
+.PHONY: get_file
+get_file:
+ if cd $(src)/modules/dl_tests/bionic; then \
+ git pull; \
+ else \
+ git clone --depth=1 https://android.googlesource.com/platform/bionic $(src)/modules/dl_tests/bionic; \
+ fi
+
+all_tests := $(tests:%=dl_tests/%)
+
+build_all: get_file $(all_tests:%=$(out)/%)
+.PHONY: build_all
+
+$(out)/dl_tests/libtest_simple.so: $(bionic_test_libs)/dlopen_testlib_simple.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_simple.so)
+
+$(out)/dl_tests/libtest_empty.so: $(bionic_test_libs)/empty.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_empty.so)
+
+$(out)/dl_tests/libtest_check_rtld_next_from_library.so: $(bionic_test_libs)/check_rtld_next_from_library.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_check_rtld_next_from_library.so)
+
+$(out)/dl_tests/libtest_dlsym_from_this_grandchild.so: $(bionic_test_libs)/dlsym_from_this_symbol2.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this_grandchild.so)
+
+$(out)/dl_tests/libtest_dlsym_from_this_child.so: COMMON += -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libtest_dlsym_from_this_child.so: \
+ $(bionic_test_libs)/dlsym_from_this_functions.cpp \
+ $(out)/dl_tests/libtest_dlsym_from_this_grandchild.so
+ $(makedir)
+ $(call quiet, cd $(out)/tests; $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this_child.so)
+
+$(out)/dl_tests/libtest_dlsym_from_this.so: COMMON += -Wl,--no-as-needed -ltest_dlsym_from_this_child -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libtest_dlsym_from_this.so: \
+ $(bionic_test_libs)/dlsym_from_this_symbol.cpp \
+ $(out)/dl_tests/libtest_dlsym_from_this_child.so
+ $(makedir)
+ $(call quiet, cd $(out)/tests; $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this.so)
+
+$(out)/dl_tests/libdlext_test.so: COMMON += -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libdlext_test.so: $(bionic_test_libs)/dlext_test_library.cpp
+ $(makedir)
+ $(call quiet, cd $(out)/tests; $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libdlext_test.so)
+
+$(out)/dl_tests/libtest_with_dependency.so: COMMON += -Wl,--no-as-needed -ldlext_test -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libtest_with_dependency.so: $(bionic_test_libs)/dlopen_testlib_simple.cpp
+ $(makedir)
+ $(call quiet, cd $(out)/tests; $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_with_dependency.so)
+
+usr.manifest: build_all FORCE
+ @echo "/usr/lib/libtest_simple.so: ./dl_tests/libtest_simple.so" > $@
+ @echo "/usr/lib/libtest_empty.so: ./dl_tests/libtest_empty.so" >> $@
+ @echo "/usr/lib/libtest_dlsym_from_this_grandchild.so: ./dl_tests/libtest_dlsym_from_this_grandchild.so" >> $@
+ @echo "/usr/lib/libtest_dlsym_from_this_child.so: ./dl_tests/libtest_dlsym_from_this_child.so" >> $@
+ @echo "/usr/lib/libtest_dlsym_from_this.so: ./dl_tests/libtest_dlsym_from_this.so" >> $@
+ @echo "/usr/lib/libdlext_test.so: ./dl_tests/libdlext_test.so" >> $@
+ @echo "/usr/lib/libtest_with_dependency.so: ./dl_tests/libtest_with_dependency.so" >> $@
+ @echo "/usr/lib/libtest_check_rtld_next_from_library.so: ./dl_tests/libtest_check_rtld_next_from_library.so" >> $@
+ @echo "/tests/libtest_simple.so: ./dl_tests/libtest_simple.so" >> $@
+ @echo "/tests/libtest_empty.so: ./dl_tests/libtest_empty.so" >> $@
+ @echo "/tests/libtest_dlsym_from_this_grandchild.so: ./dl_tests/libtest_dlsym_from_this_grandchild.so" >> $@
+ @echo "/tests/libtest_dlsym_from_this_child.so: ./dl_tests/libtest_dlsym_from_this_child.so" >> $@
+ @echo "/tests/libtest_dlsym_from_this.so: ./dl_tests/libtest_dlsym_from_this.so" >> $@
+ @echo "/tests/libdlext_test.so: ./dl_tests/libdlext_test.so" >> $@
+ @echo "/tests/libtest_with_dependency.so: ./dl_tests/libtest_with_dependency.so" >> $@
+ @echo "/tests/libtest_check_rtld_next_from_library.so: ./dl_tests/libtest_check_rtld_next_from_library.so" >> $@
+.PHONY: FORCE
+FORCE:
+
+clean:
+ -rm -f usr.manifest
+
+ifneq ($(MAKECMDGOALS),clean)
+include $(shell test -d $(out)/dl_tests && find $(out)/dl_tests -name '*.d')
+endif
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index aac12d3d..ce004339 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -147,6 +147,8 @@ $(out)/tests/tst-mmap.so: COMMON += -fuse-ld=bfd
$(out)/tests/tst-elf-permissions.so: COMMON += -fuse-ld=bfd
$(out)/tests/tst-tls.so: COMMON += -fuse-ld=bfd

+$(out)/tests/tst-dlfcn.so: COMMON += -rdynamic -ldl
+
$(out)/tests/tst-tls.so: \
$(src)/tests/tst-tls.cc \
$(out)/tests/libtls.so
diff --git a/modules/tests/module.py b/modules/tests/module.py
index be6315c5..f795280d 100644
--- a/modules/tests/module.py
+++ b/modules/tests/module.py
@@ -1,3 +1,4 @@
from osv.modules import api

api.require('java-tests')
+api.require('dl_tests')
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 3913c6bf..d2c9f288 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -10,6 +10,16 @@
#include <dlfcn.h>

#include <boost/test/unit_test.hpp>
+namespace utf = boost::unit_test;
+
+const bool rtld_next = false;
+const bool deep_lookup = false;
+
+static bool called = false;
+extern "C" void DlSymTestFunction()
+{
+ called = true;
+}

BOOST_AUTO_TEST_CASE(test_dlopen_with_null_as_filename)
{
@@ -53,3 +63,236 @@ BOOST_AUTO_TEST_CASE(test_dladdr)
BOOST_CHECK_EQUAL("vfprintf", info.dli_sname);
BOOST_CHECK_EQUAL(vfprintf, info.dli_saddr);
}
+
+BOOST_AUTO_TEST_CASE(test_dlsym_in_executable)
+{
+ dlerror();
+ void* self = dlopen(NULL, RTLD_NOW);
+ BOOST_REQUIRE_NE(self, nullptr);
+ BOOST_REQUIRE_EQUAL(dlerror(), nullptr);
+
+ void* sym = dlsym(self, "DlSymTestFunction");
+ BOOST_REQUIRE_NE(sym, nullptr);
+
+ void (*function)() = reinterpret_cast<void(*)()>(sym);
+
+ called = false;
+ function();
+ BOOST_REQUIRE_EQUAL(called, true);
+ BOOST_CHECK_EQUAL(0, dlclose(self));
+}
+
+BOOST_AUTO_TEST_CASE(test_dlsym_from_sofile,
+ *utf::enable_if<rtld_next>())
+{
+ dlerror();
+ void* handle = dlopen("/tests/libtest_dlsym_from_this.so", RTLD_LAZY | RTLD_LOCAL);
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE_NE(handle, nullptr);
+
+ // check that we can't find '_test_dlsym_symbol' via dlsym(RTLD_DEFAULT)
+ void* symbol = dlsym(RTLD_DEFAULT, "test_dlsym_symbol");
+ // TODO: dlopen ignores all the flags; the current implementation assumes the flag to be RTLD_GLOBAL but the default should be RTLD_LOCAL
+ // Should reenable the test after flags are implemented
+ // BOOST_REQUIRE_EQUAL(symbol, nullptr);
+ // auto err_msg = std::string(dlerror());
+ // BOOST_TEST_CONTEXT(err_msg)
+ // BOOST_REQUIRE_NE(err_msg.find("dlsym: symbol test_dlsym_symbol not found"),
+ // std::string::npos);
+ // BOOST_REQUIRE_NE(err_msg.find("undefined symbol: test_dlsym_symbol"),
+ // std::string::npos);
+
+ typedef int* (*fn_t)();
+ fn_t lookup_dlsym_symbol_using_RTLD_DEFAULT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol_using_RTLD_DEFAULT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE_NE(lookup_dlsym_symbol_using_RTLD_DEFAULT, nullptr);
+
+ int* ptr = lookup_dlsym_symbol_using_RTLD_DEFAULT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE_NE(ptr, nullptr);
+ BOOST_REQUIRE_EQUAL(42, *ptr);
+
+ fn_t lookup_dlsym_symbol2_using_RTLD_DEFAULT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol2_using_RTLD_DEFAULT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(lookup_dlsym_symbol2_using_RTLD_DEFAULT != nullptr);
+
+ ptr = lookup_dlsym_symbol2_using_RTLD_DEFAULT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(ptr != nullptr);
+ BOOST_REQUIRE_EQUAL(44, *ptr);
+
+ fn_t lookup_dlsym_symbol_using_RTLD_NEXT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol_using_RTLD_NEXT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(lookup_dlsym_symbol_using_RTLD_NEXT != nullptr);
+
+ ptr = lookup_dlsym_symbol_using_RTLD_NEXT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(ptr != nullptr);
+ BOOST_REQUIRE_EQUAL(43, *ptr);
+
+ dlclose(handle);
+}
+
+BOOST_AUTO_TEST_CASE(test_dlsym_from_sofile_with_preload,
+ *utf::enable_if<deep_lookup>())
+{
+ void* preload = dlopen("/tests/libtest_dlsym_from_this_grandchild.so", RTLD_NOW | RTLD_LOCAL);
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(preload != nullptr);
+
+ void* handle = dlopen("/tests/libtest_dlsym_from_this.so", RTLD_NOW | RTLD_LOCAL);
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(handle != nullptr);
+
+ // check that we can't find '_test_dlsym_symbol' via dlsym(RTLD_DEFAULT)
+ void* symbol = dlsym(RTLD_DEFAULT, "test_dlsym_symbol");
+ // TODO: dlopen ignores all the flags; the current implementation assumes the flag to be RTLD_GLOBAL but the default should be RTLD_LOCAL
+ // Should reenable the test after flags are implemented
+ // BOOST_REQUIRE_EQUAL(symbol, nullptr);
+ // auto err_msg = std::string(dlerror());
+ // BOOST_TEST_CONTEXT(err_msg)
+ // BOOST_REQUIRE_NE(err_msg.find("dlsym: symbol test_dlsym_symbol not found"),
+ // std::string::npos);
+ // BOOST_REQUIRE_NE(err_msg.find("undefined symbol: test_dlsym_symbol"),
+ // std::string::npos);
+
+ typedef int* (*fn_t)();
+ fn_t lookup_dlsym_symbol_using_RTLD_DEFAULT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol_using_RTLD_DEFAULT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(lookup_dlsym_symbol_using_RTLD_DEFAULT != nullptr);
+
+ int* ptr = lookup_dlsym_symbol_using_RTLD_DEFAULT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(ptr != nullptr);
+ BOOST_REQUIRE_EQUAL(42, *ptr);
+
+ fn_t lookup_dlsym_symbol2_using_RTLD_DEFAULT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol2_using_RTLD_DEFAULT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(lookup_dlsym_symbol2_using_RTLD_DEFAULT != nullptr);
+
+ ptr = lookup_dlsym_symbol2_using_RTLD_DEFAULT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE_NE(ptr, nullptr);
+ BOOST_REQUIRE_EQUAL(44, *ptr);
+
+ fn_t lookup_dlsym_symbol_using_RTLD_NEXT =
+ reinterpret_cast<fn_t>(dlsym(handle, "lookup_dlsym_symbol_using_RTLD_NEXT"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(lookup_dlsym_symbol_using_RTLD_NEXT != nullptr);
+
+ ptr = lookup_dlsym_symbol_using_RTLD_NEXT();
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(ptr != nullptr);
+ BOOST_REQUIRE_EQUAL(43, *ptr);
+
+ dlclose(handle);
+ dlclose(preload);
+}
+
+BOOST_AUTO_TEST_CASE(dlsym_handle_global_sym)
+{
+ // check that we do not look into global group
+ // when looking up symbol by handle
+ void* handle = dlopen("/tests/libtest_empty.so", RTLD_NOW);
+ dlopen("libtest_with_dependency.so", RTLD_NOW | RTLD_GLOBAL);
+ void* sym = dlsym(handle, "getRandomNumber");
+ BOOST_REQUIRE(sym == nullptr);
+ auto err_msg = std::string(dlerror());
+ BOOST_TEST_CONTEXT(err_msg)
+ BOOST_REQUIRE_NE(err_msg.find("dlsym: symbol getRandomNumber not found"),
+ std::string::npos);
+ // BOOST_REQUIRE_NE(err_msg.find("undefined symbol: getRandomNumber"),
+ // std::string::npos);
+
+ sym = dlsym(handle, "DlSymTestFunction");
+ BOOST_REQUIRE(sym == nullptr);
+ err_msg = std::string(dlerror());
+ BOOST_TEST_CONTEXT(err_msg)
+ BOOST_REQUIRE_NE(err_msg.find("dlsym: symbol DlSymTestFunction not found"),
+ std::string::npos);
+ // BOOST_REQUIRE_NE(err_msg.find("undefined symbol: DlSymTestFunction"),
+ // std::string::npos);
+ dlclose(handle);
+}
+
+BOOST_AUTO_TEST_CASE(dlsym_handle_empty_symbol)
+{
+ // check that dlsym of an empty symbol fails (see http://b/33530622)
+ void* handle = dlopen("/tests/libtest_dlsym_from_this.so", RTLD_NOW);
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(handle != nullptr);
+ void* sym = dlsym(handle, "");
+ BOOST_REQUIRE(sym == nullptr);
+ auto err_msg = std::string(dlerror());
+ BOOST_TEST_CONTEXT(err_msg)
+ BOOST_REQUIRE_NE(err_msg.find("dlsym: symbol not found"),
+ std::string::npos);
+ // BOOST_REQUIRE_NE(err_msg.find("undefined symbol: "),
+ // std::string::npos);
+ dlclose(handle);
+}
+
+BOOST_AUTO_TEST_CASE(dlsym_with_dependencies,
+ *utf::enable_if<deep_lookup>())
+{
+ void* handle = dlopen("/tests/libtest_with_dependency.so", RTLD_NOW);
+ BOOST_REQUIRE(handle != nullptr);
+ dlerror();
+ // This symbol is in DT_NEEDED library.
+ void* sym = dlsym(handle, "getRandomNumber");
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE_NE(sym, nullptr);
+ int (*fn)(void);
+ fn = reinterpret_cast<int (*)(void)>(sym);
+ BOOST_CHECK_EQUAL(4, fn());
+ dlclose(handle);
+}
+
+BOOST_AUTO_TEST_CASE(rtld_default_unknown_symbol)
+{
+ void* addr = dlsym(RTLD_DEFAULT, "UNKNOWN");
+ BOOST_REQUIRE_EQUAL(addr, nullptr);
+}
+
+BOOST_AUTO_TEST_CASE(rtld_default_fclose)
+{
+ void* addr = dlsym(RTLD_DEFAULT, "fclose");
+ BOOST_REQUIRE_NE(addr, nullptr);
+}
+
+BOOST_AUTO_TEST_CASE(rtld_next_unknown_symbol,
+ *utf::enable_if<rtld_next>())
+{
+ void* addr = dlsym(RTLD_NEXT, "UNKNOWN");
+ BOOST_REQUIRE_EQUAL(addr, nullptr);
+}
+
+BOOST_AUTO_TEST_CASE(rtld_next_fclose,
+ *utf::enable_if<rtld_next>())
+{
+ void* addr = dlsym(RTLD_NEXT, "fclose");
+ BOOST_REQUIRE_NE(addr, nullptr);
+}
+
+BOOST_AUTO_TEST_CASE(rtld_next_from_lib, *utf::enable_if<rtld_next>())
+{
+ void* library_with_fclose = dlopen("/tests/libtest_check_rtld_next_from_library.so", RTLD_NOW | RTLD_GLOBAL);
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(library_with_fclose != nullptr);
+ void* expected_addr = dlsym(RTLD_DEFAULT, "fclose");
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(expected_addr != nullptr);
+ typedef void* (*get_libc_fclose_ptr_fn_t)();
+ get_libc_fclose_ptr_fn_t get_libc_fclose_ptr =
+ reinterpret_cast<get_libc_fclose_ptr_fn_t>(dlsym(library_with_fclose, "get_libc_fclose_ptr"));
+ BOOST_TEST_CONTEXT(dlerror())
+ BOOST_REQUIRE(get_libc_fclose_ptr != nullptr);
+ BOOST_REQUIRE_EQUAL(expected_addr, get_libc_fclose_ptr());
+
+ dlclose(library_with_fclose);
+}
--
2.17.1

[Zhiting Zhu]

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---

core/elf.cc | 14 ++++++++++++++
include/osv/elf.hh | 1 +
libc/dlfcn.cc | 2 +-
tests/tst-dlfcn.cc | 2 +-
4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index 31777b92..882b4d49 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -917,6 +917,20 @@ Elf64_Sym* object::lookup_symbol(const char* name)
return sym;
}

+symbol_module object::lookup_symbol_deep(const char* name)
+{
+ symbol_module sym = { lookup_symbol(name), this };
+ if (!sym.symbol) {
+ for (auto&& _obj : _needed) {
+ sym = _obj->lookup_symbol_deep(name);
+ if (sym.symbol) {
+ break;
+ }
+ }
+ }
+ return sym;
+}
+
unsigned object::symtab_len()
{
if (dynamic_exists(DT_HASH)) {
diff --git a/include/osv/elf.hh b/include/osv/elf.hh
index 8e37aebb..ededb75a 100644
--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -341,6 +341,7 @@ public:
void* base() const;
void* end() const;
Elf64_Sym* lookup_symbol(const char* name);
+ symbol_module lookup_symbol_deep(const char* name);
void load_segments();
void unload_segments();
void fix_permissions();
diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc
index 8cfb7b2a..1cad7ffd 100644
--- a/libc/dlfcn.cc
+++ b/libc/dlfcn.cc
@@ -74,7 +74,7 @@ void* dlsym(void* handle, const char* name)
abort();
} else {
auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
- sym = { obj->lookup_symbol(name), obj.get() };
+ sym = obj->lookup_symbol_deep(name);
}
if (!sym.obj || !sym.symbol) {
dlerror_fmt("dlsym: symbol %s not found", name);
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index d2c9f288..614a3d00 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -13,7 +13,7 @@
namespace utf = boost::unit_test;

const bool rtld_next = false;
-const bool deep_lookup = false;
+const bool deep_lookup = true;

static bool called = false;
extern "C" void DlSymTestFunction()
--
2.17.1

[Zhiting Zhu]

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---

core/elf.cc | 25 +++++++++++++++++++++++++
include/osv/elf.hh | 1 +
libc/dlfcn.cc | 5 +++--
tests/tst-dlfcn.cc | 2 +-
4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index 882b4d49..11366ac2 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -41,6 +41,7 @@
TRACEPOINT(trace_elf_load, "%s", const char *);
TRACEPOINT(trace_elf_unload, "%s", const char *);
TRACEPOINT(trace_elf_lookup, "%s", const char *);
+TRACEPOINT(trace_elf_lookup_next, "%s", const char *);
TRACEPOINT(trace_elf_lookup_addr, "%p", const void *);

extern void* elf_start;
@@ -1496,6 +1497,30 @@ symbol_module program::lookup(const char* name)
return ret;
}

+symbol_module program::lookup_next(const char* name, const object* caller_object)
+{
+ trace_elf_lookup_next(name);
+ symbol_module ret(nullptr,nullptr);
+ if (caller_object == nullptr) {
+ return ret;
+ }
+ with_modules([&](const elf::program::modules_list &ml)
+ {
+ auto start = ml.objects.begin();
+ auto it = std::find(ml.objects.begin(), ml.objects.end(), caller_object);
+ assert(it != ml.objects.end());
+ start = ++it;
+ for (auto it = start, end = ml.objects.end(); it != end; ++it) {
+ auto module = *it;
+ if (auto sym = module->lookup_symbol(name)) {
+ ret = symbol_module(sym, module);
+ return;
+ }
+ }
+ });
+ return ret;
+}
+
void* program::do_lookup_function(const char* name)
{
auto sym = lookup(name);
diff --git a/include/osv/elf.hh b/include/osv/elf.hh
index ededb75a..b2c2e0d1 100644
--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -576,6 +576,7 @@ public:
void set_search_path(std::initializer_list<std::string> path);

symbol_module lookup(const char* symbol);
+ symbol_module lookup_next(const char* name, const object* caller_object);
template <typename T>
T* lookup_function(const char* symbol);

diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc
index 1cad7ffd..3a589d75 100644
--- a/libc/dlfcn.cc
+++ b/libc/dlfcn.cc
@@ -70,8 +70,9 @@ void* dlsym(void* handle, const char* name)
if ((program == handle) || (handle == RTLD_DEFAULT)) {
sym = program->lookup(name);
} else if (handle == RTLD_NEXT) {
- // FIXME: implement
- abort();
+ auto retaddr = __builtin_return_address(0);
+ auto call_obj = program->object_containing_addr(retaddr);
+ sym = program->lookup_next(name, call_obj);

} else {
auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);

sym = obj->lookup_symbol_deep(name);
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 614a3d00..78d27550 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -12,7 +12,7 @@
#include <boost/test/unit_test.hpp>

namespace utf = boost::unit_test;

-const bool rtld_next = false;
+const bool rtld_next = true;

const bool deep_lookup = true;

static bool called = false;

--
2.17.1

[Waldek Kozaczuk]

This patch is based on what I proposed as a sketch of a solution. I would like some else to see if that is correct. 

Nadav,

Would you mind looking at it?

Waldek

[Waldek Kozaczuk]

There still something off with this. When I run './scrips/build -j4 check' the very 1st time (no dl_tests module yet) it fails like that:

if cd /home/wkozaczuk/projects/osv/modules/dl_tests/bionic; then \

git pull; \

else \

git clone --depth=1 https://android.googlesource.com/platform/bionic /home/wkozaczuk/projects/osv/modules/dl_tests/bionic; \

fi

make: *** No rule to make target '/home/wkozaczuk/projects/osv/modules/dl_tests/bionic/tests/libs/dlopen_testlib_simple.cpp', needed by '/home/wkozaczuk/projects/osv/build/release.x64/dl_tests/libtest_simple.so'.  Stop.

make: *** Waiting for unfinished jobs....

/bin/sh: 1: cd: can't cd to /home/wkozaczuk/projects/osv/modules/dl_tests/bionic

Cloning into '/home/wkozaczuk/projects/osv/modules/dl_tests/bionic'...

remote: Counting objects: 3564, done

remote: Finding sources: 100% (3564/3564)

remote: Total 3564 (delta 653), reused 1927 (delta 653)

Receiving objects: 100% (3564/3564), 5.20 MiB | 2.68 MiB/s, done.

Resolving deltas: 100% (653/653), done.

Traceback (most recent call last):

  File "scripts/module.py", line 281, in <module>

    args.func(args)

  File "scripts/module.py", line 234, in build

    make_modules(modules, args)

  File "scripts/module.py", line 124, in make_modules

    raise Exception('make failed for ' + module.name)

Exception: make failed for dl_tests

./scripts/build failed: ( for i in "${args[@]}";

do

    case $i in 

        *=*)

            export "$i"

        ;;

    esac;

done; export fs_type jdkbase mode OSV_BUILD_PATH; export ARCH=$arch OSV_BASE=$SRC; scripts/module.py $j_arg build -c "$modules" $usrskel_arg $no_required_arg )

But when I run the same command after again it succeeds. Not sure why. Something needs to be tweaked in the Makefile.

Also, I see these 2 warnings:

  CXX tests/tst-dlfcn.cc

/home/wkozaczuk/projects/osv/tests/tst-dlfcn.cc: In member function ‘void test_dlsym_from_sofile::test_method()’:

/home/wkozaczuk/projects/osv/tests/tst-dlfcn.cc:94:11: warning: unused variable ‘symbol’ [-Wunused-variable]

   94 |     void* symbol = dlsym(RTLD_DEFAULT, "test_dlsym_symbol");

      |           ^~~~~~

/home/wkozaczuk/projects/osv/tests/tst-dlfcn.cc: In member function ‘void test_dlsym_from_sofile_with_preload::test_method()’:

/home/wkozaczuk/projects/osv/tests/tst-dlfcn.cc:151:11: warning: unused variable ‘symbol’ [-Wunused-variable]

  151 |     void* symbol = dlsym(RTLD_DEFAULT, "test_dlsym_symbol");

      |           ^~~~~~

  LD tests/tst-dlfcn.so

I know they are temporary until we apply other but still. We recently spent some good effort to get rid of all compiler warnings.

Waldek

[zhiting zhu]

That two warnings are related to the dlopen is not implemented correctly. I comment out the tests and add a TODO when dlopen flags are handled. I can add (void)symbol to disable the warnings.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/8bf507e8-e185-4fd0-aed0-c92cbfd8a781%40googlegroups.com.

[Waldek Kozaczuk]

Please also look at why the dl_tests make fails the very 1st time.

To unsubscribe from this group and stop receiving emails from it, send an email to osv...@googlegroups.com.

[Zhiting Zhu]

This patch includes a module called dl_tests which clones the bionic
and compile the needed library files.

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---
.gitignore | 2 +

modules/dl_tests/Makefile | 118 ++++++++++++++++++

modules/tests/Makefile | 2 +
modules/tests/module.py | 1 +

tests/tst-dlfcn.cc | 245 ++++++++++++++++++++++++++++++++++++++
5 files changed, 368 insertions(+)

create mode 100644 modules/dl_tests/Makefile

diff --git a/.gitignore b/.gitignore
index e0a76dc2..2dcb3225 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,4 +46,6 @@ modules/httpserver-jvm-plugin/obj/
modules/libtools/*.so
modules/libtools/*.o
modules/libyaml/usr.manifest
+modules/dl_tests/bionic/
+modules/dl_tests/usr.manifest
.idea
diff --git a/modules/dl_tests/Makefile b/modules/dl_tests/Makefile
new file mode 100644

index 00000000..8f3bd0da
--- /dev/null
+++ b/modules/dl_tests/Makefile
@@ -0,0 +1,118 @@

+ $(MAKE) get_file
+ $(MAKE) $(all_tests:%=$(out)/%)

+.PHONY: build_all
+
+$(out)/dl_tests/libtest_simple.so: $(bionic_test_libs)/dlopen_testlib_simple.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_simple.so)
+
+$(out)/dl_tests/libtest_empty.so: $(bionic_test_libs)/empty.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_empty.so)
+
+$(out)/dl_tests/libtest_check_rtld_next_from_library.so: $(bionic_test_libs)/check_rtld_next_from_library.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_check_rtld_next_from_library.so)
+
+$(out)/dl_tests/libtest_dlsym_from_this_grandchild.so: $(bionic_test_libs)/dlsym_from_this_symbol2.cpp
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this_grandchild.so)
+
+$(out)/dl_tests/libtest_dlsym_from_this_child.so: COMMON += -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libtest_dlsym_from_this_child.so: \
+ $(bionic_test_libs)/dlsym_from_this_functions.cpp

+ $(MAKE) $(out)/dl_tests/libtest_dlsym_from_this_grandchild.so
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this_child.so)

+
+$(out)/dl_tests/libtest_dlsym_from_this.so: COMMON += -Wl,--no-as-needed -ltest_dlsym_from_this_child -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib
+$(out)/dl_tests/libtest_dlsym_from_this.so: \
+ $(bionic_test_libs)/dlsym_from_this_symbol.cpp

+ $(MAKE) $(out)/dl_tests/libtest_dlsym_from_this_child.so
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_dlsym_from_this.so)

+
+$(out)/dl_tests/libdlext_test.so: COMMON += -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib

+$(out)/dl_tests/libdlext_test.so: \
+ $(bionic_test_libs)/dlext_test_library.cpp
+ $(MAKE) $(out)/dl_tests/libtest_simple.so
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libdlext_test.so)

+
+$(out)/dl_tests/libtest_with_dependency.so: COMMON += -Wl,--no-as-needed -ldlext_test -L=$(out)/dl_tests -L=/usr/lib -Wl,-rpath . -Wl,-rpath / -Wl,-rpath /usr/lib

+$(out)/dl_tests/libtest_with_dependency.so: \
+ $(bionic_test_libs)/dlopen_testlib_simple.cpp
+ $(MAKE) $(out)/dl_tests/libdlext_test.so
+ $(makedir)
+ $(call quiet, cd $(out); $(CXX) $(CXXFLAGS) -D__SHARED_OBJECT__=1 -shared -o $@ $<, CXX dl_tests/libtest_with_dependency.so)

index 3913c6bf..5258256a 100644

--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -10,6 +10,16 @@
#include <dlfcn.h>

#include <boost/test/unit_test.hpp>
+namespace utf = boost::unit_test;
+
+const bool rtld_next = false;
+const bool deep_lookup = false;
+
+static bool called = false;
+extern "C" void DlSymTestFunction()
+{
+ called = true;
+}

BOOST_AUTO_TEST_CASE(test_dlopen_with_null_as_filename)
{

@@ -53,3 +63,238 @@ BOOST_AUTO_TEST_CASE(test_dladdr)

+ (void)symbol;

+ (void)symbol;

[zhiting zhu]

This updated patch should fix the parallel make error.

[Commit Bot]

From: Zhiting Zhu <zhit...@cs.utexas.edu>
Committer: Waldemar Kozaczuk <jwkoz...@gmail.com>
Branch: master

Add dlsym tests

This patch includes a module called dl_tests which clones the bionic
and compile the needed library files.

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>

Message-Id: <20200110071641.1...@cs.utexas.edu>

---
diff --git a/.gitignore b/.gitignore

--- a/.gitignore
+++ b/.gitignore
@@ -46,4 +46,6 @@ modules/httpserver-jvm-plugin/obj/
modules/libtools/*.so
modules/libtools/*.o
modules/libyaml/usr.manifest
+modules/dl_tests/bionic/
+modules/dl_tests/usr.manifest
.idea
diff --git a/modules/dl_tests/Makefile b/modules/dl_tests/Makefile

--- a/modules/dl_tests/Makefile

--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -147,6 +147,8 @@ $(out)/tests/tst-mmap.so: COMMON += -fuse-ld=bfd
$(out)/tests/tst-elf-permissions.so: COMMON += -fuse-ld=bfd
$(out)/tests/tst-tls.so: COMMON += -fuse-ld=bfd

+$(out)/tests/tst-dlfcn.so: COMMON += -rdynamic -ldl
+
$(out)/tests/tst-tls.so: \
$(src)/tests/tst-tls.cc \
$(out)/tests/libtls.so
diff --git a/modules/tests/module.py b/modules/tests/module.py

--- a/modules/tests/module.py
+++ b/modules/tests/module.py
@@ -1,3 +1,4 @@
from osv.modules import api

api.require('java-tests')
+api.require('dl_tests')
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc

[Nadav Har'El]

On Fri, Jan 10, 2020 at 9:16 AM Zhiting Zhu <zhit...@cs.utexas.edu> wrote:

This patch includes a module called dl_tests which clones the bionic
and compile the needed library files.

Hi,

Unfortunately, I missed the discussion which led to this commit (I haven't been able

to put enough time into OSv recently, sorry about that...).

This patch seems to contain two unrelated things:

1. It adds our own tests for dlsym() and friends.

2. It adds a module dl_tests which clones the external project "bionic" - and makes the regular "test" require it.

I have some minor questions about #1 (how did you test that this test checks the right thing? Can you run the same test on Linux?)

But I'm more bothered by #2:

I'm not thrilled that OSv's test build - not some specific image - now needs to clone an external project we have no control over,

and moreover, it clones the latest version of that code instead of a specific version.

Does this mean our test build can now break at any time due to changes to this external project?

Does "make check" even run this dl_tests thing, or not? I didn't understand.

More fundementally,  someone please explain to me now (this *should* have been explained in this commit

message...), what is this "bionic", what does it test, and why is it even needed given that you wrote our own

tests for dlsym and friends?

I also have more comments inline, below.

Unless I'm missing something, this entire chunk of code just writes fixed text to usr.manifest. So why not just

have a text file, instead of all these commands?

What does this enable_if do?

Does it mean these tests don't run? They do run?

Again, this should have been explained somewhere - in comments, commit message, or something.

I don't know what you tried to link here, but obviously the link is wrong :-(

 

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.

To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200110071641.110196-1-zhitingz%40cs.utexas.edu.

[Nadav Har'El]

On Tue, Jan 7, 2020 at 10:15 AM Zhiting Zhu <zhit...@cs.utexas.edu> wrote:

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>

This patch needs a much longer (well, anything is much longer than 0 bytes ;-)) commit message.

The patch only adds a function, which isn't called in this patch. So it's important to know where you expect

to use this function, and why.

I am also worried that this implementation may run into an infinite loop in some bizarre cases - see below.

 

---
 core/elf.cc        | 14 ++++++++++++++
 include/osv/elf.hh |  1 +
 libc/dlfcn.cc      |  2 +-
 tests/tst-dlfcn.cc |  2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index 31777b92..882b4d49 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -917,6 +917,20 @@ Elf64_Sym* object::lookup_symbol(const char* name)
     return sym;
 }

+symbol_module object::lookup_symbol_deep(const char* name)

Would also be nice to have a comment above this function explaining what it does

differently from the usual lookup_symbol. The word "deep" doesn't have a very obvious

meaning here.

+{
+    symbol_module sym = { lookup_symbol(name), this };

Nitpick: It seems more logical to organize this code as follows:

     auto sym = lookup_symbol(name);

     if (sym) {

         return { sym, this };

     }

     // the rest of the code here, with one less indent level

     for (auto&& _obj : _needed) {

          auto ret = _obj->lookup_symbol_deep(name);          

          if (ret.symbol) {

               return ret;

          }

     }

     // Nothing was found...

     return symbol_module();

But I don't insist. I guess it's just a matter of taste.

I'm more worried whether this whole recursion is the right thing to do:

 

+    if (!sym.symbol) {
+        for (auto&& _obj : _needed) {
+            sym = _obj->lookup_symbol_deep(name);

I don't remember, are we guaranteed not to have cycles in the dependency graph?

In other words, can object A need B, and then B need C, and C needs A?

I'm afraid this *may* be possible, and will break this recursive walk implementation

which can go into an infinite loop. I'm not sure, though. Waldek, do you remember?

We have a function object::collect_dependencies() which tries to do this safely even

when there are loops.

Another potential (but minor) problem with your implementation is that it can end up checking the

same object many times - if there are many paths why it was needed. For example, imagine

the (fairly common) case where A needs B and C, C needs B and D, and D needs B.

So you'll reach B three times during the recursive walk. If checking B is expensive, it

maybe more efficient to remember we already saw B than checking it three times

.

If you use collect_dependencies() and then iterate on the result, it will solve this issue too.

But probably be slower in the "simple" cases. I don't know if we really care about the

performance of this?

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200107081540.11600-2-zhitingz%40cs.utexas.edu.

[zhiting zhu]

You can look at this thread: https://groups.google.com/forum/#!topic/osv-dev/FLsWldGFUiY

The tests are coming from bionic which is Android's libc. They have their own implementation of libdl with lots of tests for each functions. The tests that are added to tst-dlfcn.cc are derived from the bionic tests. I have run these tests on Linux. They can be run without any error. But there're some errors when running in osv. Some of the failed checks are commented out with TODO comments which is due to incorrect implementation of dlopen. 

Here's the command if you want to build them and run on linux:

g++ -std=gnu++11 -Wall -o tst-dlfcn tst-dlfcn.cc -rdynamic -ldl -Wl,-Bstatic -lboost_unit_test_framework -Wl,-Bdynamic
g++ -std=gnu++11 -Wall -o libtest_simple.so -shared -fpic dlopen_testlib_simple.cpp
g++ -std=gnu++11 -Wall -o libtest_empty.so -shared -fpic empty.cpp
g++ -std=gnu++11 -Wall -o libtest_check_rtld_next_from_library.so -shared -fpic check_rtld_next_from_library.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_grandchild.so -shared -fPIC dlsym_from_this_symbol2.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_child.so -shared -fPIC dlsym_from_this_functions.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this.so -shared -fPIC dlsym_from_this_symbol.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_child -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libdlext_test.so -shared -fPIC dlext_test_library.cpp -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_with_dependency.so -shared -fPIC dlopen_testlib_simple.cpp -Wl,--no-as-needed -ldlext_test -L. -Wl,-rpath=.

sudo mkdir -p /tests

sudo cp *.so /tests

Some of the comments in the code are coming from bionic. Those links should be removed.

dl_tests contains the library that's needed by tst-dlfcn.cc. Those library are built to help testing dlsym and dlopen. There're no tests in dl_tests. The reason for cloning the bionic repo is to avoid license issues. In my original patch, I submit those files which contains Android's license header. I can add a git checkout to a specific commit.

For enable_if, I'm following the boost unit test document: https://www.boost.org/doc/libs/1_72_0/libs/test/doc/html/boost_test/tests_organization/enabling.html When the boolean condition is false, those tests would not be run. They are failing without the other patches.

Zhiting

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjss8DhaxNzOryuLyAYR%3DkA3kdFCb1nU30y9f-hQQLkp0A%40mail.gmail.com.

[Nadav Har'El]

Thanks for the answers. I have some followup questions below.

On Sun, Jan 12, 2020 at 10:36 PM zhiting zhu <zhit...@cs.utexas.edu> wrote:

You can look at this thread: https://groups.google.com/forum/#!topic/osv-dev/FLsWldGFUiY

The tests are coming from bionic which is Android's libc. They have their own implementation of libdl with lots of tests for each functions. The tests that are added to tst-dlfcn.cc are derived from the bionic tests.

So, I thought the stuff in dl_tests comes from Bionic, but you're staying that the code in tst-tlfcn.cc was also copied from Bionic?

Is it an actual copy, or just "inspired" by their test and written by you from scratch?

If it's a copy, I have to ask:

1. What license does Bionic have? We will need to add Bionic's copyright statement. Better create a separate test file just for the stuff you copied from Bionic, so the entire source file has one copyright statement. No need to stuff this code - if it's copied - into the existing tst-dlfcn.cc file. We can create a new source file.

2. If you copied test code, why do we need to "git clone" the Bionic project for the dl_tests module? What does this module do? (oh, I see you answered this below so I'll continue below).

In the future, please try to make these issues understandable by using commit messages and/or internal documentation (e.g., a file modules/dl_tests/README would have been useful).

As I said, I missed the original discussion, but somebody may wonder about these things two years from now (e.g., when a build suddenly breaks because "dl_tests" no longer compiles), and would also like to be able to figure out what these things do.

 

I have run these tests on Linux. They can be run without any error. But there're some errors when running in osv. Some of the failed checks are commented out with TODO comments which is due to incorrect implementation of dlopen. 

Here's the command if you want to build them and run on linux:

g++ -std=gnu++11 -Wall -o tst-dlfcn tst-dlfcn.cc -rdynamic -ldl -Wl,-Bstatic -lboost_unit_test_framework -Wl,-Bdynamic
g++ -std=gnu++11 -Wall -o libtest_simple.so -shared -fpic dlopen_testlib_simple.cpp
g++ -std=gnu++11 -Wall -o libtest_empty.so -shared -fpic empty.cpp
g++ -std=gnu++11 -Wall -o libtest_check_rtld_next_from_library.so -shared -fpic check_rtld_next_from_library.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_grandchild.so -shared -fPIC dlsym_from_this_symbol2.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_child.so -shared -fPIC dlsym_from_this_functions.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this.so -shared -fPIC dlsym_from_this_symbol.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_child -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libdlext_test.so -shared -fPIC dlext_test_library.cpp -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_with_dependency.so -shared -fPIC dlopen_testlib_simple.cpp -Wl,--no-as-needed -ldlext_test -L. -Wl,-rpath=.

sudo mkdir -p /tests

sudo cp *.so /tests

Some of the comments in the code are coming from bionic. Those links should be removed.

dl_tests contains the library that's needed by tst-dlfcn.cc. Those library are built to help testing dlsym and dlopen. There're no tests in dl_tests. The reason for cloning the bionic repo is to avoid license issues. In my original patch, I submit those files which contains Android's license header. I can add a git checkout to a specific commit.

Can you give me an example of why we need a library to "help test dlsym and dlopen"?

Isn't it an overkill to download the entire bionic project (which I understood from you is an entire glibc) just to get a few utility functions?

What do these functions do and how hard would it be to just write our own?

 

For enable_if, I'm following the boost unit test document: https://www.boost.org/doc/libs/1_72_0/libs/test/doc/html/boost_test/tests_organization/enabling.html When the boolean condition is false, those tests would not be run. They are failing without the other patches.

I see. So I guess your plan is to eventually fix bugs and drop the "enable_if" thing and make these tests run unconditionally?

Thanks,

Nadav.

[Nadav Har'El]

Looks correct.

I have below a suggestion how to make it more efficient by using one iteration over the module list instead of two -

for your consideration.

Thanks!

Nadav.

This core will work, but note that you are iterating over the module list twice -

Once in program->object_containing_addr() which you called in the caller of this function,

and once in with_modules() here.

Both of these lock the module list, and iterate over it. But you can easily do everything in one iteration.

In other words, this function (lookup_next()) could get the return address instead of expecting to already

know the containing object. It will do, as it does now, a with_modules(), where loops over the modules and

finds the object it where object->contains_addr(ret) - and then already have the iterator available to

use as you did in the code here.

+        assert(it != ml.objects.end());

+        start = ++it;
+        for (auto it = start, end = ml.objects.end(); it != end; ++it) {
+            auto module = *it;
+            if (auto sym = module->lookup_symbol(name)) {
+               ret = symbol_module(sym, module);
+               return;

Nitpick: maybe "break" would be slightly less confusing (took me a few seconds to realise this "return" is returning from the lambda, not the function).

Nitpick: the documentation suggests you should call __builtin_extract_return_addr(__builtin_return_address(0))

But I guess on x86 the outer function does nothing.

+        auto call_obj = program->object_containing_addr(retaddr);

+        sym = program->lookup_next(name, call_obj);
     } else {
         auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
         sym = obj->lookup_symbol_deep(name);
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 614a3d00..78d27550 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -12,7 +12,7 @@
 #include <boost/test/unit_test.hpp>
 namespace utf = boost::unit_test;

-const bool rtld_next = false;
+const bool rtld_next = true;
 const bool deep_lookup = true;

 static bool called = false;
--
2.17.1

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200107081540.11600-3-zhitingz%40cs.utexas.edu.

[zhiting zhu]

On Sun, Jan 12, 2020 at 4:02 PM Nadav Har'El <n...@scylladb.com> wrote:

Thanks for the answers. I have some followup questions below.

On Sun, Jan 12, 2020 at 10:36 PM zhiting zhu <zhit...@cs.utexas.edu> wrote:

You can look at this thread: https://groups.google.com/forum/#!topic/osv-dev/FLsWldGFUiY

The tests are coming from bionic which is Android's libc. They have their own implementation of libdl with lots of tests for each functions. The tests that are added to tst-dlfcn.cc are derived from the bionic tests.

So, I thought the stuff in dl_tests comes from Bionic, but you're staying that the code in tst-tlfcn.cc was also copied from Bionic?

Is it an actual copy, or just "inspired" by their test and written by you from scratch?

If it's a copy, I have to ask:

1. What license does Bionic have? We will need to add Bionic's copyright statement. Better create a separate test file just for the stuff you copied from Bionic, so the entire source file has one copyright statement. No need to stuff this code - if it's copied - into the existing tst-dlfcn.cc file. We can create a new source file.

2. If you copied test code, why do we need to "git clone" the Bionic project for the dl_tests module? What does this module do? (oh, I see you answered this below so I'll continue below).

In the future, please try to make these issues understandable by using commit messages and/or internal documentation (e.g., a file modules/dl_tests/README would have been useful).

As I said, I missed the original discussion, but somebody may wonder about these things two years from now (e.g., when a build suddenly breaks because "dl_tests" no longer compiles), and would also like to be able to figure out what these things do.

I copy their code and change them from gtest to boost. The reason I need to git clone them is they contain the test library. I can add README to that directory with explanations. The license of bionic is Apache 2.0.

 

I have run these tests on Linux. They can be run without any error. But there're some errors when running in osv. Some of the failed checks are commented out with TODO comments which is due to incorrect implementation of dlopen. 

Here's the command if you want to build them and run on linux:

g++ -std=gnu++11 -Wall -o tst-dlfcn tst-dlfcn.cc -rdynamic -ldl -Wl,-Bstatic -lboost_unit_test_framework -Wl,-Bdynamic
g++ -std=gnu++11 -Wall -o libtest_simple.so -shared -fpic dlopen_testlib_simple.cpp
g++ -std=gnu++11 -Wall -o libtest_empty.so -shared -fpic empty.cpp
g++ -std=gnu++11 -Wall -o libtest_check_rtld_next_from_library.so -shared -fpic check_rtld_next_from_library.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_grandchild.so -shared -fPIC dlsym_from_this_symbol2.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_child.so -shared -fPIC dlsym_from_this_functions.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this.so -shared -fPIC dlsym_from_this_symbol.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_child -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libdlext_test.so -shared -fPIC dlext_test_library.cpp -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_with_dependency.so -shared -fPIC dlopen_testlib_simple.cpp -Wl,--no-as-needed -ldlext_test -L. -Wl,-rpath=.

sudo mkdir -p /tests

sudo cp *.so /tests

Some of the comments in the code are coming from bionic. Those links should be removed.

dl_tests contains the library that's needed by tst-dlfcn.cc. Those library are built to help testing dlsym and dlopen. There're no tests in dl_tests. The reason for cloning the bionic repo is to avoid license issues. In my original patch, I submit those files which contains Android's license header. I can add a git checkout to a specific commit.

Can you give me an example of why we need a library to "help test dlsym and dlopen"?

Isn't it an overkill to download the entire bionic project (which I understood from you is an entire glibc) just to get a few utility functions?

What do these functions do and how hard would it be to just write our own?

It's a bit overkill since I only need some of their test libraries. I'm looking at bionic just because they already have a very thorough test suite so that we don't have to come up one again. I need them as test libraries for dlopen/dlsym to load and see whether they are loaded correctly. I can implement these functions on my own.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjt%3Dd69gM5hKVhfkkFfeGf%2BOw-0nhTFpdQ0BRy448%3DyHtA%40mail.gmail.com.

[Zhiting Zhu]

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---

core/elf.cc | 31 +++++++++++++++++++++++++++++++

include/osv/elf.hh | 1 +
libc/dlfcn.cc | 5 +++--
tests/tst-dlfcn.cc | 2 +-

4 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index 882b4d49..eaf9d0c8 100644

--- a/core/elf.cc
+++ b/core/elf.cc
@@ -41,6 +41,7 @@
TRACEPOINT(trace_elf_load, "%s", const char *);
TRACEPOINT(trace_elf_unload, "%s", const char *);
TRACEPOINT(trace_elf_lookup, "%s", const char *);
+TRACEPOINT(trace_elf_lookup_next, "%s", const char *);
TRACEPOINT(trace_elf_lookup_addr, "%p", const void *);

extern void* elf_start;

@@ -1496,6 +1497,36 @@ symbol_module program::lookup(const char* name)
return ret;
}

+symbol_module program::lookup_next(const char* name, const void* retaddr)

+{
+ trace_elf_lookup_next(name);
+ symbol_module ret(nullptr,nullptr);

+ if (retaddr == nullptr) {

+ return ret;
+ }
+ with_modules([&](const elf::program::modules_list &ml)
+ {
+ auto start = ml.objects.begin();

+ for (auto it = ml.objects.begin(), end = ml.objects.end(); it != end; ++it) {

+ auto module = *it;

+ if (module->contains_addr(retaddr)) {
+ start = it;
+ break;
+ }
+ }
+ assert(start != ml.objects.end());
+ start = ++start;

+ for (auto it = start, end = ml.objects.end(); it != end; ++it) {
+ auto module = *it;
+ if (auto sym = module->lookup_symbol(name)) {
+ ret = symbol_module(sym, module);

+ break;

+ }
+ }
+ });
+ return ret;
+}
+
void* program::do_lookup_function(const char* name)
{
auto sym = lookup(name);
diff --git a/include/osv/elf.hh b/include/osv/elf.hh

index ededb75a..247d9c58 100644

--- a/include/osv/elf.hh
+++ b/include/osv/elf.hh
@@ -576,6 +576,7 @@ public:
void set_search_path(std::initializer_list<std::string> path);

symbol_module lookup(const char* symbol);

+ symbol_module lookup_next(const char* name, const void* retaddr);

template <typename T>
T* lookup_function(const char* symbol);

diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc

index 1cad7ffd..91bdfba1 100644

--- a/libc/dlfcn.cc
+++ b/libc/dlfcn.cc
@@ -70,8 +70,9 @@ void* dlsym(void* handle, const char* name)
if ((program == handle) || (handle == RTLD_DEFAULT)) {
sym = program->lookup(name);
} else if (handle == RTLD_NEXT) {
- // FIXME: implement
- abort();

+ auto retaddr = __builtin_extract_return_addr(__builtin_return_address(0));

+ auto call_obj = program->object_containing_addr(retaddr);
+ sym = program->lookup_next(name, call_obj);
} else {
auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
sym = obj->lookup_symbol_deep(name);

diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 6d91fb93..6188236f 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc

@@ -12,7 +12,7 @@
#include <boost/test/unit_test.hpp>
namespace utf = boost::unit_test;

-const bool rtld_next = false;

[zhiting zhu]

This version only contains one iteration over the module list.

[Nadav Har'El]

Please see comments below. You forgot to change one part of the patch.

Here start cannot be ml.objects.end()...

Maybe you meant to check that it != end?

Or maybe you meant to check that start != begin - since this is what you initialized start to?

(if you don't check start != begin, you also don't need to initialize start at all, by the way).

You forgot to fix this part...

The new look_next() implementation should take retaddr directly. You don't need to call program->object_containing_addr(retaddr) at all.

Unless I'm missing something, this version of the patch should not have even worked. If the tests still did pass, maybe the test isn't good enough?

 

     } else {
         auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
         sym = obj->lookup_symbol_deep(name);
diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 6d91fb93..6188236f 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc
@@ -12,7 +12,7 @@
 #include <boost/test/unit_test.hpp>
 namespace utf = boost::unit_test;

-const bool rtld_next = false;
+const bool rtld_next = true;
 const bool deep_lookup = true;

 static bool called = false;
--
2.17.1

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CA%2B3q14z2HB4%3DVRm5Z%2B01PfgjnkGs7ecHkwwFKwus1kjCSvjH8w%40mail.gmail.com.

[Nadav Har'El]

Hi, thank. Please see my answers and requests inline.

On Mon, Jan 13, 2020 at 4:39 AM zhiting zhu <zhit...@cs.utexas.edu> wrote:

On Sun, Jan 12, 2020 at 4:02 PM Nadav Har'El <n...@scylladb.com> wrote:

Thanks for the answers. I have some followup questions below.

On Sun, Jan 12, 2020 at 10:36 PM zhiting zhu <zhit...@cs.utexas.edu> wrote:

You can look at this thread: https://groups.google.com/forum/#!topic/osv-dev/FLsWldGFUiY

The tests are coming from bionic which is Android's libc. They have their own implementation of libdl with lots of tests for each functions. The tests that are added to tst-dlfcn.cc are derived from the bionic tests.

So, I thought the stuff in dl_tests comes from Bionic, but you're staying that the code in tst-tlfcn.cc was also copied from Bionic?

Is it an actual copy, or just "inspired" by their test and written by you from scratch?

If it's a copy, I have to ask:

1. What license does Bionic have? We will need to add Bionic's copyright statement. Better create a separate test file just for the stuff you copied from Bionic, so the entire source file has one copyright statement. No need to stuff this code - if it's copied - into the existing tst-dlfcn.cc file. We can create a new source file.

2. If you copied test code, why do we need to "git clone" the Bionic project for the dl_tests module? What does this module do? (oh, I see you answered this below so I'll continue below).

In the future, please try to make these issues understandable by using commit messages and/or internal documentation (e.g., a file modules/dl_tests/README would have been useful).

As I said, I missed the original discussion, but somebody may wonder about these things two years from now (e.g., when a build suddenly breaks because "dl_tests" no longer compiles), and would also like to be able to figure out what these things do.

I copy their code and change them from gtest to boost.

If you really did copy code and just applied some straightforward translation (not a complete rewrite just "inspired" by their tests), you need to include their copyright statement - and license - in the test file.

 

The reason I need to git clone them is they contain the test library.

What is "the test library"? Is it just a trivial creation of a ".so" with some function which is used by the test?

If so, I would very strongly prefer that we just create this trivial .so in modules/tests/Makefile - and not use a separate modules/dl_tests and definitely not clone an entire libc project!

But if I completely missed what "the test library" is, please explain it.

 

I can add README to that directory with explanations. The license of bionic is Apache 2.0.

At this point I'm starting to think the modules/dl_tests/ directory shouldn't even exist - not that it needs a README.

Feel free to convince me otherwise :-)

 

I have run these tests on Linux. They can be run without any error. But there're some errors when running in osv. Some of the failed checks are commented out with TODO comments which is due to incorrect implementation of dlopen. 

Here's the command if you want to build them and run on linux:

g++ -std=gnu++11 -Wall -o tst-dlfcn tst-dlfcn.cc -rdynamic -ldl -Wl,-Bstatic -lboost_unit_test_framework -Wl,-Bdynamic
g++ -std=gnu++11 -Wall -o libtest_simple.so -shared -fpic dlopen_testlib_simple.cpp
g++ -std=gnu++11 -Wall -o libtest_empty.so -shared -fpic empty.cpp
g++ -std=gnu++11 -Wall -o libtest_check_rtld_next_from_library.so -shared -fpic check_rtld_next_from_library.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_grandchild.so -shared -fPIC dlsym_from_this_symbol2.cpp
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this_child.so -shared -fPIC dlsym_from_this_functions.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_grandchild -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_dlsym_from_this.so -shared -fPIC dlsym_from_this_symbol.cpp -Wl,--no-as-needed -ltest_dlsym_from_this_child -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libdlext_test.so -shared -fPIC dlext_test_library.cpp -Wl,-z,relro -Wl,--no-as-needed -ltest_simple -L. -Wl,-rpath=.
g++ -std=gnu++11 -Wall -o libtest_with_dependency.so -shared -fPIC dlopen_testlib_simple.cpp -Wl,--no-as-needed -ldlext_test -L. -Wl,-rpath=.

sudo mkdir -p /tests

sudo cp *.so /tests

So, I don't see anything here using modules/dl_tests. If we can run this test in Linux without modules/dl_tests, isn't the same thing possible in OSv?

Some of the comments in the code are coming from bionic. Those links should be removed.

Please do.

dl_tests contains the library that's needed by tst-dlfcn.cc. Those library are built to help testing dlsym and dlopen. There're no tests in dl_tests. The reason for cloning the bionic repo is to avoid license issues. In my original patch, I submit those files which contains Android's license header. I can add a git checkout to a specific commit.

Can you give me an example of why we need a library to "help test dlsym and dlopen"?

Isn't it an overkill to download the entire bionic project (which I understood from you is an entire glibc) just to get a few utility functions?

What do these functions do and how hard would it be to just write our own?

It's a bit overkill since I only need some of their test libraries.

Yes, I would say including an entire libc project just to create an almost empty shared object (if that's indeed what you use it for), is an overkill :-)

[Waldek Kozaczuk]

I just wanted to say that it was me that got Zhiting on the road of creating the separate module. I just wanted to avoid the licensing issues and incorporating bionic code as is. But if there is a way to avoid the git clone, dl_tests altogether and no issues with licensing the better. 

Btw I will create new issue to track all the problems Zhiting and I identified in dlopen/dlsym function. 

Waldek

[Zhiting Zhu]

Signed-off-by: Zhiting Zhu <zhit...@cs.utexas.edu>
---

core/elf.cc | 33 +++++++++++++++++++++++++++++++++
include/osv/elf.hh | 1 +
libc/dlfcn.cc | 4 ++--
tests/tst-dlfcn.cc | 2 +-
4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/core/elf.cc b/core/elf.cc
index 882b4d49..badbb7b0 100644

--- a/core/elf.cc
+++ b/core/elf.cc
@@ -41,6 +41,7 @@
TRACEPOINT(trace_elf_load, "%s", const char *);
TRACEPOINT(trace_elf_unload, "%s", const char *);
TRACEPOINT(trace_elf_lookup, "%s", const char *);
+TRACEPOINT(trace_elf_lookup_next, "%s", const char *);
TRACEPOINT(trace_elf_lookup_addr, "%p", const void *);

extern void* elf_start;

@@ -1496,6 +1497,38 @@ symbol_module program::lookup(const char* name)

return ret;
}

+symbol_module program::lookup_next(const char* name, const void* retaddr)
+{
+ trace_elf_lookup_next(name);
+ symbol_module ret(nullptr,nullptr);
+ if (retaddr == nullptr) {
+ return ret;
+ }
+ with_modules([&](const elf::program::modules_list &ml)
+ {
+ auto start = ml.objects.begin();
+ for (auto it = ml.objects.begin(), end = ml.objects.end(); it != end; ++it) {
+ auto module = *it;
+ if (module->contains_addr(retaddr)) {
+ start = it;
+ break;
+ }
+ }

+ if (start == ml.objects.begin()) {
+ return;
+ }

index 1cad7ffd..3d228bbf 100644
--- a/libc/dlfcn.cc
+++ b/libc/dlfcn.cc
@@ -70,8 +70,8 @@ void* dlsym(void* handle, const char* name)

if ((program == handle) || (handle == RTLD_DEFAULT)) {
sym = program->lookup(name);
} else if (handle == RTLD_NEXT) {
- // FIXME: implement
- abort();
+ auto retaddr = __builtin_extract_return_addr(__builtin_return_address(0));

+ sym = program->lookup_next(name, retaddr);

} else {
auto obj = *reinterpret_cast<std::shared_ptr<elf::object>*>(handle);
sym = obj->lookup_symbol_deep(name);

diff --git a/tests/tst-dlfcn.cc b/tests/tst-dlfcn.cc
index 6d91fb93..6188236f 100644
--- a/tests/tst-dlfcn.cc
+++ b/tests/tst-dlfcn.cc

@@ -12,7 +12,7 @@
#include <boost/test/unit_test.hpp>
namespace utf = boost::unit_test;

-const bool rtld_next = false;

[zhiting zhu]

I meat to check start != end after ++start but that seems to not necessary. 

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjuFJRtDgNANMmS29gARrcvNJBku6d57JDy%2BfuD8iWrQXw%40mail.gmail.com.

[Waldek Kozaczuk]

Any reason we should not apply this patch?

I have opened new issue - https://github.com/cloudius-systems/osv/issues/1072 - to cover things we want to fix in dlopen/dlsym. Zhiting please verify I have not missed anything and add what I missed.

Now we can reference this issue.

[Nadav Har'El]

I think there is a logic bug here - if the match is on the *first* module, we will have start == ml.objects.begin(), but it doesn't mean it's an error....

I think the right way is not to initialize start at all, and check if we matched not by checking at the end whether it equals ml.objects.end() or not.

--
You received this message because you are subscribed to the Google Groups "OSv Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/20200113181034.11368-1-zhitingz%40cs.utexas.edu.