Skip to content

Commit

Permalink
elf: ignore versioning table if symbol is looked up by object itself
Browse files Browse the repository at this point in the history
The commit ed1eed7
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.

Here is some background information:

The main (and so far the only one) motivation for this patch is to allow
upgrading libgcc_s.so to the newest version from host instead of relying
on the "antique" version from externals (see usr.manifest.skel). It turns
out that at some point (starting with version 6) GCC team moved the __cpu_model
and __cpu_indicator_init (constructor function) symbols from the shared library
libgcc_s.so to the static one - libgcc.a (please see this commit for some details -
 gcc-mirror/gcc@4b5fb32#diff-3592c95126806aad11bb0f09e182f2f4)
and marked them as "compat" in libgcc_s.so (shown with single @).

This makes those symbols invisible to OSv linker because per symbols version
table they are effectively "old" and should be ignored during lookup
but not to the object itself when it tries to relocate them.
This patch actually makes the change to the version symbol handling
logic to achieve exactly that.

The version symbol handling logic in OSv is based on musl, but musl does not try to expose
such "old" symbols to the object itself like this patch does. It turns out that Musl team complained
about original change to libgcc_s.so and GCC team changed how libgcc_s.so for musl is built.
This commit - gcc-mirror/gcc@6e6c7fc
and this discussion - https://patchwork.ozlabs.org/patch/693782/ - sheds some light.

Signed-off-by: Waldemar Kozaczuk <[email protected]>
  • Loading branch information
wkozaczuk committed Jan 13, 2020
1 parent ee38389 commit b6654d0
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 15 deletions.
20 changes: 10 additions & 10 deletions core/elf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ void object::set_visibility(elf::VisibilityLevel visibilityLevel)
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;
}
Expand Down Expand Up @@ -680,7 +680,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);
}
Expand Down Expand Up @@ -708,7 +708,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;
}
Expand Down Expand Up @@ -885,7 +885,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);
Expand All @@ -909,7 +909,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;
Expand All @@ -924,14 +924,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);
}
Expand Down Expand Up @@ -1491,14 +1491,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;
}
Expand All @@ -1509,7 +1509,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));
}
Expand Down
6 changes: 3 additions & 3 deletions include/osv/elf.hh
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,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();
Expand Down Expand Up @@ -388,7 +388,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);
Expand Down Expand Up @@ -581,7 +581,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);

Expand Down
4 changes: 2 additions & 2 deletions libc/dlfcn.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit b6654d0

Please sign in to comment.