Sorry, I might miss your reply. Thanks for the tip on format-patch. I will
do it in the future. This patch is written by Waldemar for a fix on the
test errors. I'm not familiar with the elf spec. I don't know whether
DT_NEEDED would have a cycle.
I feel the order of the objects matter, especially when the man page said
about it.
On Wed, Jan 15, 2020 at 5:37 PM Nadav Har'El <n...@scylladb.com> wrote:
> When you send a second version of a patch, please label it with "v2" and
> so on (if you use git format-patch, you can do "git format-patch -v2" to do
> this automatically) and write below the fold (the "--" line) some comments
> about what changed. Because it's hard for me to keep track what are new
> patches and what are modified versions and what was modified.
>
> In the present case, this appears to be a second version of your earlier
> patch "[PATCH 2/3] look up symbol deeper".
>
> I had some review comments in that version, that I think you forgot to
> address. In that version, I was worried that the "DT_NEEDED" graph may have
> cycles, and cause your code to go into an infinite loop. I asked if you
> believe (and why) it can't have cycles. If it can, I suggested you can use
> object::collect_dependencies(), which builds a list of all modules you need
> to look at (dependencies and dependencies of dependencies and ...) -
> without a risk of cycles because the DFS walk uses a set to avoid walking
> the same node more than once.
>
> (by the way, the dlsym() manual says that it does a BFS walk over the
> dependencies. object::collect_dependencies() which I suggested above does
> DFS, not BFS. We need to consider if this makes any difference and if we
> need to do a BFS version of collect_dependencies()... I hope not, but I
> don't know).
>
> Thanks,
> Nadav.
>
> --
> Nadav Har'El
> n...@scylladb.com
>
>
> On Wed, Jan 15, 2020 at 7:39 PM Zhiting Zhu <zhiti...@cs.utexas.edu>
> wrote:
>
>> dlsym function doesn't seem to lookup symbol deep enough. This
>> patch fixes test_dlsym_from_sofile_with_preload and
>> dlsym_with_dependencies
>>
>> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
>> Signed-off-by: Zhiting Zhu <zhiti...@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 73834b19..26a25c43 100644
>> --- a/core/elf.cc
>> +++ b/core/elf.cc
>> @@ -950,6 +950,20 @@ 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) {
>> + 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 1dac6c08..8cd7b37a 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();
>> 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 osv-dev+unsubscr...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/osv-dev/20200115173946.76109-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/CANEVyjuF-6X%2BOi1pdkNzJupkVQunaH7-FuEZhr85JNUgjKFvrA%40mail.gmail.com
> <https://groups.google.com/d/msgid/osv-dev/CANEVyjuF-6X%2BOi1pdkNzJupkVQunaH7-FuEZhr85JNUgjKFvrA%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/CA%2B3q14yW3X6zvzcXr-C5xUQnYZ8iLWxadfQnS%2BouPtASGqqjnA%40mail.gmail.com.
