When relocating do not lookup STB_LOCAL symbols by name

[wkozaczuk]

Some linkers (like gold) in certain scenarios generate ELF objects that end up with symbols with local binding (STB_LOCAL) that need to be relocated. The known concrete scenarios involve symbols of type STT_SECTION and STT_TLS which fail to get relocated by OSv linker as it attempts to look them up by name and they are not present in the relevant lookup tables or worse have an empty name. For more details please look these two discussions on the mailing list - STT_SECTION and STT_TLS and STT_TLS - look for gCurrentThreadInfo.

In such scenarios, we ought to use the symbol definition as is instead of looking up by name. Couple of months ago musl made a similar change to address the exact same type of problems as described by this commit.

[wkozaczuk]

It might be tempting to solve it by this tiny patch:

diff --git a/core/elf.cc b/core/elf.cc
index ec141ff1..24dfe914 100644
--- a/core/elf.cc
+++ b/core/elf.cc
@@ -642,10 +642,13 @@ symbol_module object::symbol(unsigned idx, bool ignore_missing)
     auto symtab = dynamic_ptr<Elf64_Sym>(DT_SYMTAB);
     assert(dynamic_val(DT_SYMENT) == sizeof(Elf64_Sym));
     auto sym = &symtab[idx];
+    auto binding = symbol_binding(*sym);
+    if (binding == STB_LOCAL) {
+        return symbol_module(sym, this);
+    }
     auto nameidx = sym->st_name;
     auto name = dynamic_ptr<const char>(DT_STRTAB) + nameidx;
     auto ret = _prog.lookup(name);
-    auto binding = symbol_binding(*sym);
     if (!ret.symbol && binding == STB_WEAK) {
         return symbol_module(sym, this);
     }

but this may be too broad. I think it is more correct to apply this logic in bool object::arch_relocate_rela(u32 type, u32 sym, void *addr, Elf64_Sxword addend).

We might also consider this comment from glibc and apply similar criteria to define what 'local' symbol is.

There are at least 3 cases affected by this linker limitation (bug) - see this.

Simple reproducer:

cat vers
{
global:
  get_tls_var;
local:
  *;
};

cat dso.c 
__thread int tlsvar = 11;
int get_tls_var(void) {
  return tlsvar;
}

cat main.c 
#include <stdio.h>
int get_tls_var(void);
int main() {
  printf("get_tls_var(): %d\n", get_tls_var());
  return 0;
}

So now If I build those two like this:

gcc dso.c -fpic -shared -Wl,-version-script=vers -o libdso.so
gcc main.c libdso.so -Wl,-rpath,'$ORIGIN' -o main

the main program will work with or without my latest patch.

But if we build it like that with gold linker:
gcc dso.c -fpic -shared -fuse-ld=gold -Wl,-version-script=vers -o libdso.so
gcc main.c libdso.so -Wl,-rpath,'$ORIGIN' -o main

the program crashes like so:
/usr/lib/libdso.so: failed looking up symbol tlsvar

[backtrace]
0x0000000040364155 <elf::object::symbol(unsigned int, bool)+1637>
0x00000000403b28b9 <elf::object::arch_relocate_rela(unsigned int, unsigned int, void*, long)+649>
0x000000004035fcb3 <elf::object::relocate_rela()+211>
0x000000004036129a <elf::object::relocate()+250>
0x0000000040365a20 <elf::program::load_object(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<std::shared_ptr<elf::object>, std::allocator<std::shared_ptr<elf::object> > >&)+2416>
0x0000000040364a69 <elf::object::load_needed(std::vector<std::shared_ptr<elf::object>, std::allocator<std::shared_ptr<elf::object> > >&)+1209>
0x0000000040365a18 <elf::program::load_object(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::vector<std::shared_ptr<elf::object>, std::allocator<std::shared_ptr<elf::object> > >&)+2408>
0x0000000040366a79 <elf::program::get_library(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, bool)+345>
0x0000000040443882 <osv::application::application(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, bool, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<0x0000000040443bd2 <osv::application::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, bool, std::unordered_map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void ()>0x0000000040443e20 <osv::application::run(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&)+80>
0x000000004022d6ee <do_main_thread(void*)+3230>
0x00000000404782b9 <???+1078428345>
0x000000004040f29b <thread_main_c+43>
0x00000000403ae8b2 <???+1077602482>

and works the latest patch correctly.