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