Is there anything wrong/deficient with this patch that prevents us from applying it?
On Thursday, December 26, 2019 at 10:22:46 PM UTC-5, Waldek Kozaczuk wrote: > > The commit > https://github.com/cloudius-systems/osv/commit/ed1eed7a567ec17138c65f0a5628c2311603c712 > > enhanced dynamic linker to skip old symbols per versions table. > Unfortunately > the relevant code did not take into account whether the old symbol is > being > looked up by the object itself during relocation. > > This sentence from https://www.akkadia.org/drepper/symbol-versioning - > "If the highest bit (bit 15) is set this is a hidden symbol which cannot > be referenced from outside the object." - seems to indicate the old > symbols should be visible to the object itself only. > > This patch enhances lookup method to help track "who" is looking > and if it is the object itself, then the versions table is ignored. > > Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com> > --- > core/elf.cc | 20 ++++++++++---------- > include/osv/elf.hh | 6 +++--- > libc/dlfcn.cc | 4 ++-- > 3 files changed, 15 insertions(+), 15 deletions(-) > > diff --git a/core/elf.cc b/core/elf.cc > index 24dfe914..fa63c9aa 100644 > --- a/core/elf.cc > +++ b/core/elf.cc > @@ -154,7 +154,7 @@ void object::setprivate(bool priv) > template <> > void* object::lookup(const char* symbol) > { > - symbol_module sm{lookup_symbol(symbol), this}; > + symbol_module sm{lookup_symbol(symbol, false), this}; > if (!sm.symbol || sm.symbol->st_shndx == SHN_UNDEF) { > return nullptr; > } > @@ -648,7 +648,7 @@ symbol_module object::symbol(unsigned idx, bool > ignore_missing) > } > auto nameidx = sym->st_name; > auto name = dynamic_ptr<const char>(DT_STRTAB) + nameidx; > - auto ret = _prog.lookup(name); > + auto ret = _prog.lookup(name, this); > if (!ret.symbol && binding == STB_WEAK) { > return symbol_module(sym, this); > } > @@ -676,7 +676,7 @@ symbol_module object::symbol_other(unsigned idx) > for (auto module : ml.objects) { > if (module == this) > continue; // do not match this module > - if (auto sym = module->lookup_symbol(name)) { > + if (auto sym = module->lookup_symbol(name, false)) { > ret = symbol_module(sym, module); > break; > } > @@ -852,7 +852,7 @@ dl_new_hash(const char *s) > return h & 0xffffffff; > } > > -Elf64_Sym* object::lookup_symbol_gnu(const char* name) > +Elf64_Sym* object::lookup_symbol_gnu(const char* name, bool self_lookup) > { > auto symtab = dynamic_ptr<Elf64_Sym>(DT_SYMTAB); > auto strtab = dynamic_ptr<char>(DT_STRTAB); > @@ -876,7 +876,7 @@ Elf64_Sym* object::lookup_symbol_gnu(const char* name) > if (idx == 0) { > return nullptr; > } > - auto version_symtab = dynamic_exists(DT_VERSYM) ? > dynamic_ptr<Elf64_Versym>(DT_VERSYM) : nullptr; > + auto version_symtab = (!self_lookup && dynamic_exists(DT_VERSYM)) ? > dynamic_ptr<Elf64_Versym>(DT_VERSYM) : nullptr; > do { > if ((chains[idx] & ~1) != (hashval & ~1)) { > continue; > @@ -891,14 +891,14 @@ Elf64_Sym* object::lookup_symbol_gnu(const char* > name) > return nullptr; > } > > -Elf64_Sym* object::lookup_symbol(const char* name) > +Elf64_Sym* object::lookup_symbol(const char* name, bool self_lookup) > { > if (!visible()) { > return nullptr; > } > Elf64_Sym* sym; > if (dynamic_exists(DT_GNU_HASH)) { > - sym = lookup_symbol_gnu(name); > + sym = lookup_symbol_gnu(name, self_lookup); > } else { > sym = lookup_symbol_old(name); > } > @@ -1457,14 +1457,14 @@ void program::del_debugger_obj(object* obj) > } > } > > -symbol_module program::lookup(const char* name) > +symbol_module program::lookup(const char* name, object* seeker) > { > trace_elf_lookup(name); > symbol_module ret(nullptr,nullptr); > with_modules([&](const elf::program::modules_list &ml) > { > for (auto module : ml.objects) { > - if (auto sym = module->lookup_symbol(name)) { > + if (auto sym = module->lookup_symbol(name, seeker == module)) > { > ret = symbol_module(sym, module); > return; > } > @@ -1475,7 +1475,7 @@ symbol_module program::lookup(const char* name) > > void* program::do_lookup_function(const char* name) > { > - auto sym = lookup(name); > + auto sym = lookup(name, nullptr); > if (!sym.symbol) { > throw std::runtime_error("symbol not found " + demangle(name)); > } > diff --git a/include/osv/elf.hh b/include/osv/elf.hh > index 4466b2ab..fce71ba5 100644 > --- a/include/osv/elf.hh > +++ b/include/osv/elf.hh > @@ -340,7 +340,7 @@ public: > void set_dynamic_table(Elf64_Dyn* dynamic_table); > void* base() const; > void* end() const; > - Elf64_Sym* lookup_symbol(const char* name); > + Elf64_Sym* lookup_symbol(const char* name, bool self_lookup); > void load_segments(); > void unload_segments(); > void fix_permissions(); > @@ -382,7 +382,7 @@ protected: > unsigned get_segment_mmap_permissions(const Elf64_Phdr& phdr); > private: > Elf64_Sym* lookup_symbol_old(const char* name); > - Elf64_Sym* lookup_symbol_gnu(const char* name); > + Elf64_Sym* lookup_symbol_gnu(const char* name, bool self_lookup); > template <typename T> > T* dynamic_ptr(unsigned tag); > Elf64_Xword dynamic_val(unsigned tag); > @@ -574,7 +574,7 @@ public: > */ > void set_search_path(std::initializer_list<std::string> path); > > - symbol_module lookup(const char* symbol); > + symbol_module lookup(const char* symbol, object* seeker); > template <typename T> > T* lookup_function(const char* symbol); > > diff --git a/libc/dlfcn.cc b/libc/dlfcn.cc > index 8cfb7b2a..346cf195 100644 > --- a/libc/dlfcn.cc > +++ b/libc/dlfcn.cc > @@ -68,13 +68,13 @@ void* dlsym(void* handle, const char* name) > elf::symbol_module sym; > auto program = elf::get_program(); > if ((program == handle) || (handle == RTLD_DEFAULT)) { > - sym = program->lookup(name); > + sym = program->lookup(name, nullptr); > } else if (handle == RTLD_NEXT) { > // FIXME: implement > abort(); > } else { > auto obj = > *reinterpret_cast<std::shared_ptr<elf::object>*>(handle); > - sym = { obj->lookup_symbol(name), obj.get() }; > + sym = { obj->lookup_symbol(name, false), obj.get() }; > } > if (!sym.obj || !sym.symbol) { > dlerror_fmt("dlsym: symbol %s not found", name); > -- > 2.20.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/4ba179b2-44e6-4015-b4ef-0db912d8bbef%40googlegroups.com.
