On Wed, Aug 29, 2018 at 4:02 AM, Waldemar Kozaczuk <jwkozac...@gmail.com>
wrote:

> Sometimes object files are compiled with "-z now" flag and resulting
> ELF files end up with BIND_NOW setting which requires dynamic linker
> to eagerly resolve dependent symbols. OSv satisfies this requirement
> and aborts when it finds first unresolvable symbol. This harsh behavior
> is undesired as in many cases those missing symbols may never be used.
>
> So this patch changes OSv dynamic linker by relaxing its behavior
> when handling missing symbols in BIND_NOW scenario. Instead of aborting
> it simply warns about missing symbol and leaves it unresolved to trigger
> abort when potentially referenced later. So in essence we are postponing
> abort until absolutely necessary.
>
> Fixes #993
>
> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
> ---
>  arch/aarch64/arch-elf.cc | 11 ++++++++---
>  arch/x64/arch-elf.cc     | 11 ++++++++---
>  core/elf.cc              | 22 +++++++++++++---------
>  include/osv/elf.hh       |  4 ++--
>  4 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/arch/aarch64/arch-elf.cc b/arch/aarch64/arch-elf.cc
> index fe63c59b..21324568 100644
> --- a/arch/aarch64/arch-elf.cc
> +++ b/arch/aarch64/arch-elf.cc
> @@ -58,10 +58,15 @@ bool object::arch_relocate_rela(u32 type, u32 sym,
> void *addr,
>      return true;
>  }
>
> -bool object::arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend)
> +bool object::arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend, bool ignore_missing)
>  {
> -    *static_cast<void**>(addr) = symbol(sym).relocated_addr() + addend;
> -    return true;
> +    auto _sym = symbol(sym, ignore_missing);
> +    if (_sym.symbol) {
> +        *static_cast<void**>(addr) = _sym.relocated_addr() + addend;
> +         return true;
> +    } else {
> +         return false;
> +    }
>  }
>
>  void object::prepare_initial_tls(void* buffer, size_t size,
> diff --git a/arch/x64/arch-elf.cc b/arch/x64/arch-elf.cc
> index 5182703a..82c60eb1 100644
> --- a/arch/x64/arch-elf.cc
> +++ b/arch/x64/arch-elf.cc
> @@ -103,10 +103,15 @@ bool object::arch_relocate_rela(u32 type, u32 sym,
> void *addr,
>      return true;
>  }
>
> -bool object::arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend)
> +bool object::arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend, bool ignore_missing)
>  {
> -    *static_cast<void**>(addr) = symbol(sym).relocated_addr();
> -    return true;
> +    auto _sym = symbol(sym, ignore_missing);
> +    if (_sym.symbol) {
> +        *static_cast<void**>(addr) = _sym.relocated_addr();
> +         return true;
> +    } else {
> +         return false;
> +    }
>  }
>
>  void object::prepare_initial_tls(void* buffer, size_t size,
> diff --git a/core/elf.cc b/core/elf.cc
> index 4f064cb6..071b1394 100644
> --- a/core/elf.cc
> +++ b/core/elf.cc
> @@ -549,7 +549,7 @@ static std::string demangle(const char *name) {
>      return ret;
>  }
>
> -symbol_module object::symbol(unsigned idx)
> +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));
> @@ -562,8 +562,12 @@ symbol_module object::symbol(unsigned idx)
>          return symbol_module(sym, this);
>      }
>      if (!ret.symbol) {
> -        abort("%s: failed looking up symbol %s\n",
> -                pathname().c_str(), demangle(name).c_str());
> +        if (ignore_missing) {
> +            debug("%s: failed looking up symbol %s\n",
> pathname().c_str(), demangle(name).c_str());
> +        } else {
> +            abort("%s: failed looking up symbol %s\n",
> pathname().c_str(), demangle(name).c_str());
> +        }
> +        return symbol_module(nullptr, this);
>

Is this return necessary? If we don't do it, we fallback to "return ret"
below, which is if I understand correctly, symbol_module(nullptr, _prog).
Is that not good enough?

     }
>      return ret;
>  }
> @@ -643,13 +647,13 @@ void object::relocate_pltgot()
>          void *addr = _base + p->r_offset;
>          if (bind_now) {
>              // If on-load binding is requested (instead of the default
> lazy
> -            // binding), resolve all the PLT entries now.
> +            // binding), try to resolve all the PLT entries now.
> +            // If symbol cannot be resolved warn about it instead of
> aborting
>              u32 sym = info >> 32;
> -            if (!arch_relocate_jump_slot(sym, addr, p->r_addend)) {
> -                debug_early("relocate_pltgot(): failed jump slot
> relocation\n");
> -                abort();
> -            }
> -        } else if (original_plt) {
> +            if (arch_relocate_jump_slot(sym, addr, p->r_addend, true))
> +                  continue;
> +        }
> +        if (original_plt) {
>              // Restore the link to the original plt.
>              // We know the JUMP_SLOT entries are in plt order, and that
>              // each plt entry is 16 bytes.
> diff --git a/include/osv/elf.hh b/include/osv/elf.hh
> index d9e2acd3..d7dfb31d 100644
> --- a/include/osv/elf.hh
> +++ b/include/osv/elf.hh
> @@ -378,7 +378,7 @@ private:
>      std::vector<const char*> dynamic_str_array(unsigned tag);
>      Elf64_Dyn& dynamic_tag(unsigned tag);
>      Elf64_Dyn* _dynamic_tag(unsigned tag);
> -    symbol_module symbol(unsigned idx);
> +    symbol_module symbol(unsigned idx, bool ignore_missing = false);
>      symbol_module symbol_other(unsigned idx);
>      Elf64_Xword symbol_tls_module(unsigned idx);
>      void relocate_rela();
> @@ -428,7 +428,7 @@ protected:
>      // The return value is true on success, false on failure.
>      bool arch_relocate_rela(u32 type, u32 sym, void *addr,
>                              Elf64_Sxword addend);
> -    bool arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend);
> +    bool arch_relocate_jump_slot(u32 sym, void *addr, Elf64_Sxword
> addend, bool bind_now = false);
>

You forgot to update the "bind_now" name here.

     size_t static_tls_end() {
>          if (is_core()) {
>              return 0;
> --
> 2.17.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.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
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.
For more options, visit https://groups.google.com/d/optout.

Reply via email to