LibELF: Use Optional<SymbolLookupResult> as a return type

Instead of storing a "found" state inside the result object.

Userland/Libraries/LibELF/DynamicLinker.cpp

@@ -66,19 +66,18 @@ bool g_allowed_to_check_environment_variables { false };
 bool g_do_breakpoint_trap_before_entry { false };
 }
 
-DynamicObject::SymbolLookupResult DynamicLinker::lookup_global_symbol(const char* symbol_name)
+Optional<DynamicObject::SymbolLookupResult> DynamicLinker::lookup_global_symbol(const char* symbol_name)
 {
-    DynamicObject::SymbolLookupResult weak_result = {};
+    Optional<DynamicObject::SymbolLookupResult> weak_result;
     for (auto& lib : g_global_objects) {
         auto res = lib->lookup_symbol(symbol_name);
-        if (res.found) {
-            if (res.bind == STB_GLOBAL) {
-                return res;
-            } else if (res.bind == STB_WEAK && !weak_result.found) {
-                weak_result = res;
-            }
-            // We don't want to allow local symbols to be pulled in to other modules
-        }
+        if (!res.has_value())
+            continue;
+        if (res.value().bind == STB_GLOBAL)
+            return res;
+        if (res.value().bind == STB_WEAK && !weak_result.has_value())
+            weak_result = res;
+        // We don't want to allow local symbols to be pulled in to other modules
     }
     return weak_result;
 }
@@ -163,21 +162,21 @@ static void initialize_libc(DynamicObject& libc)
     // Also, we can't just mark `__libc_init` with "__attribute__((constructor))"
     // because it uses getenv() internally, so `environ` has to be initialized before we call `__libc_init`.
     auto res = libc.lookup_symbol("environ");
-    ASSERT(res.found);
-    *((char***)res.address) = g_envp;
+    ASSERT(res.has_value());
+    *((char***)res.value().address) = g_envp;
 
     res = libc.lookup_symbol("__environ_is_malloced");
-    ASSERT(res.found);
-    *((bool*)res.address) = false;
+    ASSERT(res.has_value());
+    *((bool*)res.value().address) = false;
 
     res = libc.lookup_symbol("exit");
-    ASSERT(res.found);
-    g_libc_exit = (LibCExitFunction)res.address;
+    ASSERT(res.has_value());
+    g_libc_exit = (LibCExitFunction)res.value().address;
 
     res = libc.lookup_symbol("__libc_init");
-    ASSERT(res.found);
+    ASSERT(res.has_value());
     typedef void libc_init_func();
-    ((libc_init_func*)res.address)();
+    ((libc_init_func*)res.value().address)();
 }
 
 static void load_elf(const String& name)

Userland/Libraries/LibELF/DynamicLinker.h

@@ -34,7 +34,7 @@ namespace ELF {
 
 class DynamicLinker {
 public:
-    static DynamicObject::SymbolLookupResult lookup_global_symbol(const char* symbol);
+    static Optional<DynamicObject::SymbolLookupResult> lookup_global_symbol(const char* symbol);
     [[noreturn]] static void linker_main(String&& main_program_name, int fd, bool is_secure, int argc, char** argv, char** envp);
 
 private:

Userland/Libraries/LibELF/DynamicLoader.cpp

@@ -333,14 +333,13 @@ DynamicLoader::RelocationResult DynamicLoader::do_relocation(size_t total_tls_si
         auto symbol = relocation.symbol();
         dbgln<DYNAMIC_LOAD_DEBUG>("Absolute relocation: name: '{}', value: {}", symbol.name(), symbol.value());
         auto res = lookup_symbol(symbol);
-        if (!res.found) {
-            if (symbol.bind() == STB_WEAK) {
+        if (!res.has_value()) {
+            if (symbol.bind() == STB_WEAK)
                 return RelocationResult::ResolveLater;
-            }
             dbgln("ERROR: symbol not found: {}.", symbol.name());
             ASSERT_NOT_REACHED();
         }
-        u32 symbol_address = res.address;
+        u32 symbol_address = res.value().address;
         *patch_ptr += symbol_address;
         dbgln<DYNAMIC_LOAD_DEBUG>("   Symbol address: {:p}", *patch_ptr);
         break;
@@ -349,8 +348,8 @@ DynamicLoader::RelocationResult DynamicLoader::do_relocation(size_t total_tls_si
         auto symbol = relocation.symbol();
         dbgln<DYNAMIC_LOAD_DEBUG>("PC-relative relocation: '{}', value: {:p}", symbol.name(), symbol.value());
         auto res = lookup_symbol(symbol);
-        ASSERT(res.found);
-        u32 relative_offset = (res.address - (FlatPtr)(m_dynamic_object->base_address().as_ptr() + relocation.offset()));
+        ASSERT(res.has_value());
+        u32 relative_offset = (res.value().address - (FlatPtr)(m_dynamic_object->base_address().as_ptr() + relocation.offset()));
         *patch_ptr += relative_offset;
         dbgln<DYNAMIC_LOAD_DEBUG>("   Symbol address: {:p}", *patch_ptr);
         break;
@@ -359,7 +358,7 @@ DynamicLoader::RelocationResult DynamicLoader::do_relocation(size_t total_tls_si
         auto symbol = relocation.symbol();
         dbgln<DYNAMIC_LOAD_DEBUG>("Global data relocation: '{}', value: {:p}", symbol.name(), symbol.value());
         auto res = lookup_symbol(symbol);
-        if (!res.found) {
+        if (!res.has_value()) {
             // We do not support these
             // TODO: Can we tell gcc not to generate the piece of code that uses these?
             // (--disable-tm-clone-registry flag in gcc conifugraion?)
@@ -368,17 +367,16 @@ DynamicLoader::RelocationResult DynamicLoader::do_relocation(size_t total_tls_si
                 break;
             }
 
-            if (symbol.bind() == STB_WEAK) {
+            if (symbol.bind() == STB_WEAK)
                 return RelocationResult::ResolveLater;
-            }
 
             // Symbol not found
             return RelocationResult::Failed;
         }
-        dbgln<DYNAMIC_LOAD_DEBUG>("was symbol found? {}, address: {:#08x}", res.found, res.address);
+        dbgln<DYNAMIC_LOAD_DEBUG>("symbol found, location: {:#08x}", res.value().address);
         dbgln<DYNAMIC_LOAD_DEBUG>("object: {}", m_filename);
 
-        u32 symbol_location = res.address;
+        u32 symbol_location = res.value().address;
         ASSERT(symbol_location != (FlatPtr)m_dynamic_object->base_address().as_ptr());
         *patch_ptr = symbol_location;
         dbgln<DYNAMIC_LOAD_DEBUG>("   Symbol address: {:p}", *patch_ptr);
@@ -404,12 +402,11 @@ DynamicLoader::RelocationResult DynamicLoader::do_relocation(size_t total_tls_si
         dbgln<DYNAMIC_LOAD_DEBUG>("Symbol is_undefined?: {}", symbol.is_undefined());
         dbgln<DYNAMIC_LOAD_DEBUG>("TLS relocation: '{}', value: {:p}", symbol.name(), symbol.value());
         auto res = lookup_symbol(symbol);
-        if (!res.found)
+        if (!res.has_value())
             break;
-        ASSERT(res.found);
-        u32 symbol_value = res.value;
+        u32 symbol_value = res.value().value;
         dbgln<DYNAMIC_LOAD_DEBUG>("symbol value: {}", symbol_value);
-        const auto dynamic_object_of_symbol = res.dynamic_object;
+        auto* dynamic_object_of_symbol = res.value().dynamic_object;
         ASSERT(dynamic_object_of_symbol);
         size_t offset_of_tls_end = dynamic_object_of_symbol->tls_offset().value() + dynamic_object_of_symbol->tls_size().value();
         dbgln<DYNAMIC_LOAD_DEBUG>("patch ptr: {:p}", patch_ptr);
@@ -503,7 +500,7 @@ u32 DynamicLoader::ProgramHeaderRegion::mmap_prot() const
     return prot;
 }
 
-DynamicObject::SymbolLookupResult DynamicLoader::lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const
+Optional<DynamicObject::SymbolLookupResult> DynamicLoader::lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const
 {
     return m_dynamic_object->lookup_symbol(symbol);
 }

Userland/Libraries/LibELF/DynamicLoader.h

@@ -131,7 +131,7 @@ private:
     RelocationResult do_relocation(size_t total_tls_size, DynamicObject::Relocation relocation);
     size_t calculate_tls_size() const;
 
-    DynamicObject::SymbolLookupResult lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const;
+    Optional<DynamicObject::SymbolLookupResult> lookup_symbol(const ELF::DynamicObject::Symbol&) const;
 
     String m_filename;
     String m_program_interpreter;

Userland/Libraries/LibELF/DynamicObject.cpp

@@ -462,12 +462,12 @@ static const char* name_for_dtag(Elf32_Sword d_tag)
     }
 }
 
-DynamicObject::SymbolLookupResult DynamicObject::lookup_symbol(const char* name) const
+Optional<DynamicObject::SymbolLookupResult> DynamicObject::lookup_symbol(const char* name) const
 {
     auto res = hash_section().lookup_symbol(name);
     if (res.is_undefined())
         return {};
-    return SymbolLookupResult { true, res.value(), (FlatPtr)res.address().as_ptr(), res.bind(), this };
+    return SymbolLookupResult { res.value(), (FlatPtr)res.address().as_ptr(), res.bind(), this };
 }
 
 NonnullRefPtr<DynamicObject> DynamicObject::construct(VirtualAddress base_address, VirtualAddress dynamic_section_address)
@@ -485,14 +485,14 @@ Elf32_Addr DynamicObject::patch_plt_entry(u32 relocation_offset)
     auto sym = relocation.symbol();
 
     u8* relocation_address = relocation.address().as_ptr();
-    auto res = lookup_symbol(sym);
+    auto result = lookup_symbol(sym);
 
-    if (!res.found) {
-        dbgln("did not find symbol: {} ", sym.name());
+    if (!result.has_value()) {
+        dbgln("did not find symbol: {}", sym.name());
         ASSERT_NOT_REACHED();
     }
 
-    u32 symbol_location = res.address;
+    u32 symbol_location = result.value().address;
 
     dbgln<DYNAMIC_LOAD_DEBUG>("DynamicLoader: Jump slot relocation: putting {} ({:p}) into PLT at {}", sym.name(), symbol_location, (void*)relocation_address);
 
@@ -501,7 +501,7 @@ Elf32_Addr DynamicObject::patch_plt_entry(u32 relocation_offset)
     return symbol_location;
 }
 
-DynamicObject::SymbolLookupResult DynamicObject::lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const
+Optional<DynamicObject::SymbolLookupResult> DynamicObject::lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const
 {
     dbgln<DYNAMIC_LOAD_DEBUG>("looking up symbol: {}", symbol.name());
     if (symbol.is_undefined() || symbol.bind() == STB_WEAK)
@@ -509,7 +509,7 @@ DynamicObject::SymbolLookupResult DynamicObject::lookup_symbol(const ELF::Dynami
 
     if (!symbol.is_undefined()) {
         dbgln<DYNAMIC_LOAD_DEBUG>("symbol is defined in its object");
-        return { true, symbol.value(), (FlatPtr)symbol.address().as_ptr(), symbol.bind(), &symbol.object() };
+        return SymbolLookupResult { symbol.value(), (FlatPtr)symbol.address().as_ptr(), symbol.bind(), &symbol.object() };
     }
     return DynamicLinker::lookup_global_symbol(symbol.name());
 }

Userland/Libraries/LibELF/DynamicObject.h

@@ -276,18 +276,17 @@ public:
     void for_each_initialization_array_function(F f) const;
 
     struct SymbolLookupResult {
-        bool found { false };
         FlatPtr value { 0 };
         FlatPtr address { 0 };
         unsigned bind { STB_LOCAL };
         const ELF::DynamicObject* dynamic_object { nullptr }; // The object in which the symbol is defined
     };
-    SymbolLookupResult lookup_symbol(const char* name) const;
+    Optional<SymbolLookupResult> lookup_symbol(const char* name) const;
 
     // Will be called from _fixup_plt_entry, as part of the PLT trampoline
     Elf32_Addr patch_plt_entry(u32 relocation_offset);
 
-    SymbolLookupResult lookup_symbol(const ELF::DynamicObject::Symbol& symbol) const;
+    Optional<SymbolLookupResult> lookup_symbol(const ELF::DynamicObject::Symbol&) const;
 
     bool elf_is_dynamic() const { return m_is_elf_dynamic; }