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

On Mon, Jan 13, 2020 at 04:59 Nadav Har'El <n...@scylladb.com> wrote:

> Hi, thank. Please see my answers and requests inline.
>
> On Mon, Jan 13, 2020 at 4:39 AM zhiting zhu <zhiti...@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 <zhiti...@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
> :-)
>
>
>> 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.
>>
>>
>>>
>>>
>>>>
>>>>
>>>> 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.
>>>
>>>
>>>>
>>>> Zhiting
>>>>
>>>>
>>>>
>>>> On Sun, Jan 12, 2020 at 4:02 AM Nadav Har'El <n...@scylladb.com> wrote:
>>>>
>>>>>
>>>>> On Fri, Jan 10, 2020 at 9:16 AM Zhiting Zhu <zhiti...@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.
>>>>>
>>>>>
>>>>>> Signed-off-by: Zhiting Zhu <zhiti...@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 @@
>>>>>> +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:
>>>>>> +       $(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)
>>>>>> +
>>>>>> +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" >> $@
>>>>>>
>>>>>
>>>>> 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?
>>>>>
>>>>> +.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..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)
>>>>>>      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>())
>>>>>>
>>>>>
>>>>> 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.
>>>>>
>>>>> +{
>>>>>> +    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");
>>>>>> +    (void)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");
>>>>>> +    (void)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)
>>>>>>
>>>>>
>>>>> I don't know what you tried to link here, but obviously the link is
>>>>> wrong :-(
>>>>>
>>>>>
>>>>>> +    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
>>>>>>
>>>>>> --
>>>>>> 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+unsubscr...@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
>>>>>> .
>>>>>>
>>>>> --
>>>>> 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+unsubscr...@googlegroups.com.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/d/msgid/osv-dev/CANEVyjss8DhaxNzOryuLyAYR%3DkA3kdFCb1nU30y9f-hQQLkp0A%40mail.gmail.com
>>>>> <https://groups.google.com/d/msgid/osv-dev/CANEVyjss8DhaxNzOryuLyAYR%3DkA3kdFCb1nU30y9f-hQQLkp0A%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>>> .
>>>>>
>>>> --
>>> 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+unsubscr...@googlegroups.com.
>>> To view this discussion on the web visit
>>> https://groups.google.com/d/msgid/osv-dev/CANEVyjt%3Dd69gM5hKVhfkkFfeGf%2BOw-0nhTFpdQ0BRy448%3DyHtA%40mail.gmail.com
>>> <https://groups.google.com/d/msgid/osv-dev/CANEVyjt%3Dd69gM5hKVhfkkFfeGf%2BOw-0nhTFpdQ0BRy448%3DyHtA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>

-- 
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+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CAL9cFfOv2B3LfdwuRpsNoMSg7NFv5xS1_RMkhx5d9vUGeZc1xA%40mail.gmail.com.

Reply via email to