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/20190816033500.17180-1-jwkozaczuk%40gmail.com.

Reply via email to