LibELF: Perform weak relocations in much more straight-forward manner

There is no need to defer unresolved weak relocations until stage 3 --
it is not like we will magically find new symbols in the same objects.

Userland/Libraries/LibELF/DynamicLoader.cpp

@@ -196,13 +196,10 @@ void DynamicLoader::do_main_relocations()
 
     Optional<DynamicLoader::CachedLookupResult> cached_result;
     m_dynamic_object->relocation_section().for_each_relocation([&](DynamicObject::Relocation const& relocation) {
-        switch (do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::No, ShouldCallIfuncResolver::No)) {
+        switch (do_direct_relocation(relocation, cached_result, ShouldCallIfuncResolver::No)) {
         case RelocationResult::Failed:
             dbgln("Loader.so: {} unresolved symbol '{}'", m_filepath, relocation.symbol().name());
             VERIFY_NOT_REACHED();
-        case RelocationResult::ResolveLater:
-            m_unresolved_relocations.append(relocation);
-            break;
         case RelocationResult::CallIfuncResolver:
             m_direct_ifunc_relocations.append(relocation);
             break;
@@ -226,8 +223,8 @@ void DynamicLoader::do_main_relocations()
         if (static_cast<GenericDynamicRelocationType>(relocation.type()) == GenericDynamicRelocationType::TLSDESC) {
             // GNU ld for some reason puts TLSDESC relocations into .rela.plt
             // https://sourceware.org/bugzilla/show_bug.cgi?id=28387
-
+            auto result = do_direct_relocation(relocation, cached_result, ShouldCallIfuncResolver::No);
-            VERIFY(do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::No, ShouldCallIfuncResolver::No) == RelocationResult::Success);
+            VERIFY(result == RelocationResult::Success);
             return;
         }
 
@@ -237,8 +234,6 @@ void DynamicLoader::do_main_relocations()
             case RelocationResult::Failed:
                 dbgln("Loader.so: {} unresolved symbol '{}'", m_filepath, relocation.symbol().name());
                 VERIFY_NOT_REACHED();
-            case RelocationResult::ResolveLater:
-                VERIFY_NOT_REACHED();
             case RelocationResult::CallIfuncResolver:
                 m_plt_ifunc_relocations.append(relocation);
                 // Set up lazy binding, in case an IFUNC resolver calls another IFUNC that hasn't been resolved yet.
@@ -255,7 +250,6 @@ void DynamicLoader::do_main_relocations()
 
 Result<NonnullRefPtr<DynamicObject>, DlErrorMessage> DynamicLoader::load_stage_3(unsigned flags)
 {
-    do_lazy_relocations();
     if (flags & RTLD_LAZY) {
         if (m_dynamic_object->has_plt())
             setup_plt_trampoline();
@@ -270,7 +264,7 @@ Result<NonnullRefPtr<DynamicObject>, DlErrorMessage> DynamicLoader::load_stage_3
 
     Optional<DynamicLoader::CachedLookupResult> cached_result;
     for (auto const& relocation : m_direct_ifunc_relocations) {
-        auto result = do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::No, ShouldCallIfuncResolver::Yes);
+        auto result = do_direct_relocation(relocation, cached_result, ShouldCallIfuncResolver::Yes);
         VERIFY(result == RelocationResult::Success);
     }
 
@@ -307,17 +301,6 @@ void DynamicLoader::load_stage_4()
     m_fully_initialized = true;
 }
 
-void DynamicLoader::do_lazy_relocations()
-{
-    Optional<DynamicLoader::CachedLookupResult> cached_result;
-    for (auto const& relocation : m_unresolved_relocations) {
-        if (auto res = do_direct_relocation(relocation, cached_result, ShouldInitializeWeak::Yes, ShouldCallIfuncResolver::Yes); res != RelocationResult::Success) {
-            dbgln("Loader.so: {} unresolved symbol '{}'", m_filepath, relocation.symbol().name());
-            VERIFY_NOT_REACHED();
-        }
-    }
-}
-
 void DynamicLoader::load_program_headers()
 {
     FlatPtr ph_load_start = SIZE_MAX;
@@ -520,7 +503,6 @@ void DynamicLoader::load_program_headers()
 
 DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObject::Relocation const& relocation,
     Optional<DynamicLoader::CachedLookupResult>& cached_result,
-    ShouldInitializeWeak should_initialize_weak,
     ShouldCallIfuncResolver should_call_ifunc_resolver)
 {
     FlatPtr* patch_ptr = nullptr;
@@ -571,10 +553,7 @@ DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObjec
         auto res = lookup_symbol(symbol);
         VirtualAddress symbol_address;
         if (!res.has_value()) {
-            if (symbol.bind() == STB_WEAK) {
+            if (symbol.bind() != STB_WEAK) {
-                if (should_initialize_weak == ShouldInitializeWeak::No)
-                    return RelocationResult::ResolveLater;
-            } else {
                 dbgln("ERROR: symbol not found: {}.", symbol.name());
                 return RelocationResult::Failed;
             }
@@ -598,10 +577,7 @@ DynamicLoader::RelocationResult DynamicLoader::do_direct_relocation(DynamicObjec
         auto res = lookup_symbol(symbol);
         VirtualAddress symbol_location;
         if (!res.has_value()) {
-            if (symbol.bind() == STB_WEAK) {
+            if (symbol.bind() != STB_WEAK) {
-                if (should_initialize_weak == ShouldInitializeWeak::No)
-                    return RelocationResult::ResolveLater;
-            } else {
                 // Symbol not found
                 return RelocationResult::Failed;
             }

Userland/Libraries/LibELF/DynamicLoader.h

@@ -35,11 +35,6 @@ private:
     size_t m_size;
 };
 
-enum class ShouldInitializeWeak {
-    Yes,
-    No
-};
-
 enum class ShouldCallIfuncResolver {
     Yes,
     No
@@ -127,7 +122,6 @@ private:
     void do_main_relocations();
 
     // Stage 3
-    void do_lazy_relocations();
     void setup_plt_trampoline();
 
     // Stage 4
@@ -138,16 +132,15 @@ private:
     friend FlatPtr _fixup_plt_entry(DynamicObject*, u32);
 
     enum class RelocationResult : uint8_t {
-        Failed = 0,
+        Failed,
-        Success = 1,
+        Success,
-        ResolveLater = 2,
+        CallIfuncResolver,
-        CallIfuncResolver = 3,
     };
     struct CachedLookupResult {
         DynamicObject::Symbol symbol;
         Optional<DynamicObject::SymbolLookupResult> result;
     };
-    RelocationResult do_direct_relocation(DynamicObject::Relocation const&, Optional<CachedLookupResult>&, ShouldInitializeWeak, ShouldCallIfuncResolver);
+    RelocationResult do_direct_relocation(DynamicObject::Relocation const&, Optional<CachedLookupResult>&, ShouldCallIfuncResolver);
     // Will be called from _fixup_plt_entry, as part of the PLT trampoline
     static RelocationResult do_plt_relocation(DynamicObject::Relocation const&, ShouldCallIfuncResolver);
     void do_relr_relocations();
@@ -174,7 +167,6 @@ private:
     size_t m_tls_size_of_current_object { 0 };
     size_t m_tls_alignment_of_current_object { 0 };
 
-    Vector<DynamicObject::Relocation> m_unresolved_relocations;
     Vector<DynamicObject::Relocation> m_direct_ifunc_relocations;
     Vector<DynamicObject::Relocation> m_plt_ifunc_relocations;