Sorry, I forgot to review this. I'll do it now.

--
Nadav Har'El
[email protected]


On Wed, Jan 22, 2020 at 2:34 PM Waldek Kozaczuk <[email protected]>
wrote:

> Shall we commit this and another V4 version of these patches?
>
> On Sunday, January 19, 2020 at 8:17:39 PM UTC-5, Zhiting Zhu wrote:
>>
>> dlsym function doesn't seem to lookup symbol deep enough. This
>> patch includes a bfs version of collect dependencies. It
>> fixes test_dlsym_from_sofile_with_preload and
>> dlsym_with_dependencies.
>>
>> Signed-off-by: Waldemar Kozaczuk <[email protected]>
>> Signed-off-by: Zhiting Zhu <[email protected]>
>> ---
>>  core/elf.cc        | 39 +++++++++++++++++++++++++++++++++++++++
>>  include/osv/elf.hh |  2 ++
>>  libc/dlfcn.cc      |  2 +-
>>  tests/tst-dlfcn.cc |  2 +-
>>  4 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/core/elf.cc b/core/elf.cc
>> index 73834b19..a419ea6a 100644
>> --- a/core/elf.cc
>> +++ b/core/elf.cc
>> @@ -27,6 +27,7 @@
>>  #include <sys/utsname.h>
>>  #include <osv/demangle.hh>
>>  #include <boost/version.hpp>
>> +#include <deque>
>>
>>  #include "arch.hh"
>>
>> @@ -950,6 +951,22 @@ Elf64_Sym* object::lookup_symbol(const char* name,
>> bool self_lookup)
>>      return sym;
>>  }
>>
>> +symbol_module object::lookup_symbol_deep(const char* name)
>> +{
>> +    symbol_module sym = { lookup_symbol(name, false), this };
>> +    if (!sym.symbol) {
>> +        auto deps = this->collect_dependencies_bfs();
>> +        for (auto&& _obj : deps) {
>> +            auto symbol = _obj->lookup_symbol(name, false);
>> +            if (symbol) {
>> +                sym = { symbol, _obj };
>> +                break;
>> +            }
>> +        }
>> +    }
>> +    return sym;
>> +}
>> +
>>  unsigned object::symtab_len()
>>  {
>>      if (dynamic_exists(DT_HASH)) {
>> @@ -1089,6 +1106,28 @@ void
>> object::collect_dependencies(std::unordered_set<elf::object*>& ds)
>>      }
>>  }
>>
>> +std::deque<elf::object*> object::collect_dependencies_bfs()
>> +{
>> +    std::unordered_set<object*> deps_set;
>> +    std::deque<object*> operate_queue;
>> +    std::deque<object*> deps;
>> +    operate_queue.push_back(this);
>> +    deps_set.insert(this);
>> +
>> +    while (!operate_queue.empty()) {
>> +        object* obj = operate_queue.front();
>> +        operate_queue.pop_front();
>> +        deps.push_back(obj);
>> +        for (auto&& d : obj->_needed) {
>> +            if (!deps_set.count(d.get())) {
>> +                deps_set.insert(d.get());
>> +                operate_queue.push_back(d.get());
>> +            }
>> +        }
>> +    }
>> +    return deps;
>> +}
>> +
>>  std::string object::soname()
>>  {
>>      return dynamic_exists(DT_SONAME) ? dynamic_str(DT_SONAME) :
>> std::string();
>> diff --git a/include/osv/elf.hh b/include/osv/elf.hh
>> index 1dac6c08..c942ac61 100644
>> --- a/include/osv/elf.hh
>> +++ b/include/osv/elf.hh
>> @@ -347,6 +347,7 @@ public:
>>      void* base() const;
>>      void* end() const;
>>      Elf64_Sym* lookup_symbol(const char* name, bool self_lookup);
>> +    symbol_module lookup_symbol_deep(const char* name);
>>      void load_segments();
>>      void process_headers();
>>      void unload_segments();
>> @@ -405,6 +406,7 @@ private:
>>      void relocate_pltgot();
>>      unsigned symtab_len();
>>      void collect_dependencies(std::unordered_set<elf::object*>& ds);
>> +    std::deque<elf::object*> collect_dependencies_bfs();
>>      void prepare_initial_tls(void* buffer, size_t size,
>> std::vector<ptrdiff_t>& offsets);
>>      void prepare_local_tls(std::vector<ptrdiff_t>& offsets);
>>      void alloc_static_tls();
>> diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc
>> index 346cf195..e3a4a577 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, false), 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 5258256a..6d91fb93 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
>>
>> --
> 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 [email protected].
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/e4fd1c02-1111-49a1-9128-fe12e36a42ba%40googlegroups.com
> <https://groups.google.com/d/msgid/osv-dev/e4fd1c02-1111-49a1-9128-fe12e36a42ba%40googlegroups.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 [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyjt8TOSXMWOhQ4sz6WA8kTm0sPVNGqXwX9enaNToXygA%3Dg%40mail.gmail.com.

Reply via email to