Please see comments below. You forgot to change one part of the patch. On Mon, Jan 13, 2020 at 4:40 AM zhiting zhu <[email protected]> wrote:
> This version only contains one iteration over the module list. > > On Sun, Jan 12, 2020 at 8:39 PM Zhiting Zhu <[email protected]> > wrote: > >> Signed-off-by: Zhiting Zhu <[email protected]> >> --- >> 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()); >> > 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). > + 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); >> > 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 [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/osv-dev/CA%2B3q14z2HB4%3DVRm5Z%2B01PfgjnkGs7ecHkwwFKwus1kjCSvjH8w%40mail.gmail.com > <https://groups.google.com/d/msgid/osv-dev/CA%2B3q14z2HB4%3DVRm5Z%2B01PfgjnkGs7ecHkwwFKwus1kjCSvjH8w%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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/CANEVyjuFJRtDgNANMmS29gARrcvNJBku6d57JDy%2BfuD8iWrQXw%40mail.gmail.com.
