Another question: with the patch as is, would missing symbol be handled 
when invoked same way as with relocate_pltgot() by elf_resolv_pltgot() or 
would we have to do something special for that?

Waldek 

On Thursday, August 15, 2019 at 11:35:22 PM UTC-4, Waldek Kozaczuk wrote:
>
> The commit 
> https://github.com/cloudius-systems/osv/commit/f5cc12d56dd986d2c7982c5738b1f859702b07fb
>  
> addressed the issue #993 to relax handling of missing symbols in 
> relocate_pltgot() 
> when loading ELF objects with BIND_NOW. 
>
> This patch on other hand attempts to close the gap of handling 
> missing symbols this time in relocate_rela() during loading phase of ELF 
> objects 
> by dynamic linker. The solution is similar to what we do in 
> relocate_pltgot() 
> - let missing symbol get ignored. 
>
> Given it is actually an RFC here are some questions: 
>
> 1) Currently, unlike relocate_pltgot(), relocate_rela() does not take 
>    into account presence of BIND_NOW tag at all and this patch 
>    assumes that BIND_NOW is on. Should we take BIND_NOW into account? 
>    Would it change anything? 
>
> 2) Should we be letting missing symbol be ignored for all relocation 
>    types below? Maybe not all? What about symbol_other() case? 
>
> Fixes #1023 
>
> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com> 
> --- 
>  arch/x64/arch-elf.cc | 41 +++++++++++++++++++++++++++++------------ 
>  1 file changed, 29 insertions(+), 12 deletions(-) 
>
> diff --git a/arch/x64/arch-elf.cc b/arch/x64/arch-elf.cc 
> index ebe40996..1d03fa4c 100644 
> --- a/arch/x64/arch-elf.cc 
> +++ b/arch/x64/arch-elf.cc 
> @@ -66,20 +66,28 @@ bool object::arch_relocate_rela(u32 type, u32 sym, 
> void *addr, 
>      case R_X86_64_NONE: 
>          break; 
>      case R_X86_64_COPY: { 
> -        symbol_module sm = symbol_other(sym); 
> +        symbol_module sm = symbol_other(sym); //TODO 
>          memcpy(addr, sm.relocated_addr(), sm.size()); 
>          break; 
>      } 
> -    case R_X86_64_64: 
> -        *static_cast<void**>(addr) = symbol(sym).relocated_addr() + 
> addend; 
> +    case R_X86_64_64: { 
> +        auto _sym = symbol(sym, true); 
> +        if (_sym.symbol) { 
> +            *static_cast<void**>(addr) = _sym.relocated_addr() + addend; 
> +        } 
>          break; 
> +    } 
>      case R_X86_64_RELATIVE: 
>          *static_cast<void**>(addr) = _base + addend; 
>          break; 
>      case R_X86_64_JUMP_SLOT: 
> -    case R_X86_64_GLOB_DAT: 
> -        *static_cast<void**>(addr) = symbol(sym).relocated_addr(); 
> +    case R_X86_64_GLOB_DAT: { 
> +        auto _sym = symbol(sym, true); 
> +        if (_sym.symbol) { 
> +            *static_cast<void**>(addr) = _sym.relocated_addr(); 
> +        } 
>          break; 
> +    } 
>      // The next 3 types are intended to relocate symbols of thread local 
> variables 
>      // defined with __thread modifier 
>      // 
> @@ -100,23 +108,32 @@ bool object::arch_relocate_rela(u32 type, u32 sym, 
> void *addr, 
>          } else { 
>              // The thread-local variable being accessed is located 
>              // in DIFFERENT shared object that the caller 
> -            *static_cast<u64*>(addr) = symbol(sym).obj->_module_index; 
> +            auto _sym = symbol(sym, true); 
> +            if (_sym.symbol) { 
> +                *static_cast<u64*>(addr) = _sym.obj->_module_index; 
> +            } 
>          } 
>          break; 
> -    case R_X86_64_DTPOFF64: 
> +    case R_X86_64_DTPOFF64: { 
>          // The thread-local variable being accessed is located 
>          // in DIFFERENT shared object that the caller 
> -        *static_cast<u64*>(addr) = symbol(sym).symbol->st_value; 
> +        auto _sym = symbol(sym, true); 
> +        if (_sym.symbol) { 
> +            *static_cast<u64*>(addr) = _sym.symbol->st_value; 
> +        } 
>          break; 
> +    } 
>      case R_X86_64_TPOFF64: 
>          // This type is intended to resolve symbols of thread-local 
> variables in static TLS 
>          // accessed in initial-exec mode and is handled to calculate the 
> virtual address of 
>          // target thread-local variable 
>          if (sym) { 
> -            auto sm = symbol(sym); 
> -            sm.obj->alloc_static_tls(); 
> -            auto tls_offset = sm.obj->static_tls_end() + 
> sched::kernel_tls_size(); 
> -            *static_cast<u64*>(addr) = sm.symbol->st_value + addend - 
> tls_offset; 
> +            auto sm = symbol(sym, true); 
> +            if (sm.symbol) { 
> +                sm.obj->alloc_static_tls(); 
> +                auto tls_offset = sm.obj->static_tls_end() + 
> sched::kernel_tls_size(); 
> +                *static_cast<u64*>(addr) = sm.symbol->st_value + addend - 
> tls_offset; 
> +            } 
>          } else { 
>              // TODO: Which case does this handle? 
>              alloc_static_tls(); 
> -- 
> 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/f5da871a-9028-430d-81d0-fb8c49377902%40googlegroups.com.

Reply via email to