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 <[email protected]> > --- > 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/f5da871a-9028-430d-81d0-fb8c49377902%40googlegroups.com.
