Refactor code to return Optional<ExecutorSymbolDef>

SahilPatidar · SahilPatidar · commit 72ddefa61a72 · 2025-09-15T16:23:42.000+05:30
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h b/llvm/include/llvm/ExecutionEngine/Orc/EPCGenericDylibManager.h
@@ -52,25 +52,25 @@ class EPCGenericDylibManager {
   LLVM_ABI Expected<tpctypes::DylibHandle> open(StringRef Path, uint64_t Mode);
 
   /// Looks up symbols within the given dylib.
-  Expected<std::vector<ExecutorSymbolDef>>
+  Expected<tpctypes::LookupResult>
   lookup(tpctypes::DylibHandle H, const SymbolLookupSet &Lookup) {
-    std::promise<MSVCPExpected<std::vector<ExecutorSymbolDef>>> RP;
+    std::promise<MSVCPExpected<tpctypes::LookupResult>> RP;
     auto RF = RP.get_future();
     lookupAsync(H, Lookup, [&RP](auto R) { RP.set_value(std::move(R)); });
     return RF.get();
   }
 
   /// Looks up symbols within the given dylib.
-  Expected<std::vector<ExecutorSymbolDef>>
+  Expected<tpctypes::LookupResult>
   lookup(tpctypes::DylibHandle H, const RemoteSymbolLookupSet &Lookup) {
-    std::promise<MSVCPExpected<std::vector<ExecutorSymbolDef>>> RP;
+    std::promise<MSVCPExpected<tpctypes::LookupResult>> RP;
     auto RF = RP.get_future();
     lookupAsync(H, Lookup, [&RP](auto R) { RP.set_value(std::move(R)); });
     return RF.get();
   }
 
   using SymbolLookupCompleteFn =
-      unique_function<void(Expected<std::vector<ExecutorSymbolDef>>)>;
+      unique_function<void(Expected<tpctypes::LookupResult>)>;
 
   /// Looks up symbols within the given dylib.
   LLVM_ABI void lookupAsync(tpctypes::DylibHandle H,
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Shared/OrcRTBridge.h b/llvm/include/llvm/ExecutionEngine/Orc/Shared/OrcRTBridge.h
@@ -72,9 +72,9 @@ using SPSSimpleExecutorDylibManagerLookupSignature =
         shared::SPSExecutorAddr, shared::SPSExecutorAddr,
         shared::SPSRemoteSymbolLookupSet);
 
-using SPSSimpleExecutorDylibManagerResolveSignature =
-    shared::SPSExpected<shared::SPSSequence<shared::SPSExecutorSymbolDef>>(
-        shared::SPSExecutorAddr, shared::SPSRemoteSymbolLookupSet);
+using SPSSimpleExecutorDylibManagerResolveSignature = shared::SPSExpected<
+    shared::SPSSequence<shared::SPSOptional<shared::SPSExecutorSymbolDef>>>(
+    shared::SPSExecutorAddr, shared::SPSRemoteSymbolLookupSet);
 
 using SPSSimpleExecutorMemoryManagerReserveSignature =
     shared::SPSExpected<shared::SPSExecutorAddr>(shared::SPSExecutorAddr,
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/Shared/TargetProcessControlTypes.h b/llvm/include/llvm/ExecutionEngine/Orc/Shared/TargetProcessControlTypes.h
@@ -118,7 +118,7 @@ using DylibHandle = ExecutorAddr;
 ///  dylib in the target process.
 using ResolverHandle = ExecutorAddr;
 
-using LookupResult = std::vector<ExecutorSymbolDef>;
+using LookupResult = std::vector<std::optional<ExecutorSymbolDef>>;
 
 } // end namespace tpctypes
 
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/TargetProcess/ExecutorResolver.h b/llvm/include/llvm/ExecutionEngine/Orc/TargetProcess/ExecutorResolver.h
@@ -23,7 +23,7 @@ namespace llvm::orc {
 
 class ExecutorResolver {
 public:
-  using ResolveResult = Expected<std::vector<ExecutorSymbolDef>>;
+  using ResolveResult = Expected<std::vector<std::optional<ExecutorSymbolDef>>>;
   using YieldResolveResultFn = unique_function<void(ResolveResult)>;
 
   virtual ~ExecutorResolver() = default;
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp b/llvm/lib/ExecutionEngine/Orc/EPCDebugObjectRegistrar.cpp
@@ -41,8 +41,10 @@ Expected<std::unique_ptr<EPCDebugObjectRegistrar>> createJITLoaderGDBRegistrar(
   assert(Result->size() == 1 && "Unexpected number of dylibs in result");
   assert((*Result)[0].size() == 1 &&
          "Unexpected number of addresses in result");
+  assert((*Result)[0][0].has_value() &&
+         "Expected a valid address in the lookup result");
 
-  ExecutorAddr RegisterAddr = (*Result)[0][0].getAddress();
+  ExecutorAddr RegisterAddr = (*Result)[0][0]->getAddress();
   return std::make_unique<EPCDebugObjectRegistrar>(ES, RegisterAddr);
 }
 
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/EPCDynamicLibrarySearchGenerator.cpp
@@ -79,12 +79,15 @@ Error EPCDynamicLibrarySearchGenerator::tryToGenerate(
     assert(Result->front().size() == LookupSymbols.size() &&
            "Result has incorrect number of elements");
 
+    auto SymsIt = Result->front().begin();
+    SymbolNameSet MissingSymbols;
     SymbolMap NewSymbols;
-    auto ResultI = Result->front().begin();
-    for (auto &KV : LookupSymbols) {
-      if (ResultI->getAddress())
-        NewSymbols[KV.first] = *ResultI;
-      ++ResultI;
+    for (auto &[Name, Flags] : LookupSymbols) {
+      const auto &Sym = *SymsIt++;
+      if (Sym && Sym->getAddress())
+        NewSymbols[Name] = *Sym;
+      else if (LLVM_UNLIKELY(!Sym && Flags == SymbolLookupFlags::RequiredSymbol))
+        MissingSymbols.insert(Name);
     }
 
     LLVM_DEBUG({
@@ -96,6 +99,10 @@ Error EPCDynamicLibrarySearchGenerator::tryToGenerate(
     if (NewSymbols.empty())
       return LS.continueLookup(Error::success());
 
+    if (LLVM_UNLIKELY(!MissingSymbols.empty()))
+      return LS.continueLookup(make_error<SymbolsNotFound>(
+          this->EPC.getSymbolStringPool(), std::move(MissingSymbols)));
+
     // Define resolved symbols.
     Error Err = addAbsolutes(JD, std::move(NewSymbols));
 
diff --git a/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp b/llvm/lib/ExecutionEngine/Orc/EPCGenericDylibManager.cpp
@@ -89,7 +89,8 @@ void EPCGenericDylibManager::lookupAsync(tpctypes::DylibHandle H,
       SAs.Resolve,
       [Complete = std::move(Complete)](
           Error SerializationErr,
-          Expected<std::vector<ExecutorSymbolDef>> Result) mutable {
+          Expected<std::vector<std::optional<ExecutorSymbolDef>>>
+              Result) mutable {
         if (SerializationErr) {
           cantFail(Result.takeError());
           Complete(std::move(SerializationErr));
@@ -107,7 +108,8 @@ void EPCGenericDylibManager::lookupAsync(tpctypes::DylibHandle H,
       SAs.Resolve,
       [Complete = std::move(Complete)](
           Error SerializationErr,
-          Expected<std::vector<ExecutorSymbolDef>> Result) mutable {
+          Expected<std::vector<std::optional<ExecutorSymbolDef>>>
+              Result) mutable {
         if (SerializationErr) {
           cantFail(Result.takeError());
           Complete(std::move(SerializationErr));
diff --git a/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp b/llvm/lib/ExecutionEngine/Orc/ExecutorResolutionGenerator.cpp
@@ -62,15 +62,16 @@ Error ExecutorResolutionGenerator::tryToGenerate(
         assert(Result->front().size() == LookupSymbols.size() &&
                "Result has incorrect number of elements");
 
-        const std::vector<ExecutorSymbolDef> &Syms = Result->front();
-        size_t SymIdx = 0;
+        // const tpctypes::LookupResult &Syms = Result->front();
+        // size_t SymIdx = 0;
+        auto Syms = Result->front().begin();
         SymbolNameSet MissingSymbols;
         SymbolMap NewSyms;
         for (auto &[Name, Flags] : LookupSymbols) {
-          auto Sym = Syms[SymIdx++];
-          if (Sym.getAddress())
-            NewSyms[Name] = Sym;
-          else if (LLVM_UNLIKELY(Flags == SymbolLookupFlags::RequiredSymbol))
+          const auto &Sym = *Syms++;
+          if (Sym && Sym->getAddress())
+            NewSyms[Name] = *Sym;
+          else if (LLVM_UNLIKELY(!Sym && Flags == SymbolLookupFlags::RequiredSymbol))
             MissingSymbols.insert(Name);
         }
 
diff --git a/llvm/lib/ExecutionEngine/Orc/LookupAndRecordAddrs.cpp b/llvm/lib/ExecutionEngine/Orc/LookupAndRecordAddrs.cpp
@@ -72,9 +72,10 @@ Error lookupAndRecordAddrs(
     return make_error<StringError>("Error in lookup result elements",
                                    inconvertibleErrorCode());
 
-  for (unsigned I = 0; I != Pairs.size(); ++I)
-    *Pairs[I].second = Result->front()[I].getAddress();
-
+  for (unsigned I = 0; I != Pairs.size(); ++I) {
+    if (Result->front()[I])
+      *Pairs[I].second = Result->front()[I]->getAddress();
+  }
   return Error::success();
 }
 
diff --git a/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp b/llvm/lib/ExecutionEngine/Orc/SelfExecutorProcessControl.cpp
@@ -91,22 +91,18 @@ void SelfExecutorProcessControl::lookupSymbolsAsync(
 
   for (auto &Elem : Request) {
     sys::DynamicLibrary Dylib(Elem.Handle.toPtr<void *>());
-    R.push_back(std::vector<ExecutorSymbolDef>());
+    R.push_back(tpctypes::LookupResult());
     for (auto &KV : Elem.Symbols) {
       auto &Sym = KV.first;
       std::string Tmp((*Sym).data() + !!GlobalManglingPrefix,
                       (*Sym).size() - !!GlobalManglingPrefix);
       void *Addr = Dylib.getAddressOfSymbol(Tmp.c_str());
-      if (!Addr && KV.second == SymbolLookupFlags::RequiredSymbol) {
-        // FIXME: Collect all failing symbols before erroring out.
-        SymbolNameVector MissingSymbols;
-        MissingSymbols.push_back(Sym);
-        return Complete(
-            make_error<SymbolsNotFound>(SSP, std::move(MissingSymbols)));
-      }
-      // FIXME: determine accurate JITSymbolFlags.
-      R.back().push_back(
-          {ExecutorAddr::fromPtr(Addr), JITSymbolFlags::Exported});
+      if (!Addr && KV.second == SymbolLookupFlags::RequiredSymbol)
+        R.back().emplace_back();
+      else
+        // FIXME: determine accurate JITSymbolFlags.
+        R.back().emplace_back(ExecutorSymbolDef(ExecutorAddr::fromPtr(Addr),
+                                                JITSymbolFlags::Exported));
     }
   }
 
diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcess/ExecutorResolver.cpp b/llvm/lib/ExecutionEngine/Orc/TargetProcess/ExecutorResolver.cpp
@@ -9,7 +9,7 @@ namespace llvm::orc {
 void DylibSymbolResolver::resolveAsync(
     const RemoteSymbolLookupSet &L,
     ExecutorResolver::YieldResolveResultFn &&OnResolve) {
-  std::vector<ExecutorSymbolDef> Result;
+  std::vector<std::optional<ExecutorSymbolDef>> Result;
   auto DL = sys::DynamicLibrary(Handle.toPtr<void *>());
 
   for (const auto &E : L) {
@@ -33,12 +33,11 @@ void DylibSymbolResolver::resolveAsync(
 
       void *Addr = DL.getAddressOfSymbol(DemangledSymName);
       if (!Addr && E.Required)
-        OnResolve(make_error<StringError>(Twine("Missing definition for ") +
-                                              DemangledSymName,
-                                          inconvertibleErrorCode()));
-
-      // FIXME: determine accurate JITSymbolFlags.
-      Result.push_back({ExecutorAddr::fromPtr(Addr), JITSymbolFlags::Exported});
+        Result.emplace_back();
+      else
+        // FIXME: determine accurate JITSymbolFlags.
+        Result.emplace_back(ExecutorSymbolDef(ExecutorAddr::fromPtr(Addr),
+                                              JITSymbolFlags::Exported));
     }
   }
 
diff --git a/llvm/lib/ExecutionEngine/Orc/TargetProcess/SimpleExecutorDylibManager.cpp b/llvm/lib/ExecutionEngine/Orc/TargetProcess/SimpleExecutorDylibManager.cpp
@@ -133,7 +133,8 @@ SimpleExecutorDylibManager::resolveWrapper(const char *ArgData,
              rt::SPSSimpleExecutorDylibManagerResolveSignature>::
       handle(ArgData, ArgSize,
              [](ExecutorAddr Obj, RemoteSymbolLookupSet L) -> ResolveResult {
-               using TmpResult = MSVCPExpected<std::vector<ExecutorSymbolDef>>;
+               using TmpResult =
+                   MSVCPExpected<std::vector<std::optional<ExecutorSymbolDef>>>;
                std::promise<TmpResult> P;
                auto F = P.get_future();
                Obj.toPtr<ExecutorResolver *>()->resolveAsync(
diff --git a/llvm/tools/lli/ForwardingMemoryManager.h b/llvm/tools/lli/ForwardingMemoryManager.h
@@ -109,8 +109,11 @@ class RemoteResolver : public LegacyJITSymbolResolver {
       if (Syms->size() != 1)
         return make_error<StringError>("Unexpected remote lookup result",
                                        inconvertibleErrorCode());
-      return JITSymbol(Syms->front().getAddress().getValue(),
-                       Syms->front().getFlags());
+      if (!Syms->front())
+        return make_error<StringError>("Expected valid address",
+                                       inconvertibleErrorCode());
+      return JITSymbol(Syms->front()->getAddress().getValue(),
+                       Syms->front()->getFlags());
     } else
       return Syms.takeError();
   }
diff --git a/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp b/llvm/unittests/ExecutionEngine/Orc/ObjectLinkingLayerTest.cpp
@@ -301,15 +301,15 @@ TEST(ObjectLinkingLayerSearchGeneratorTest, AbsoluteSymbolsObjectLayer) {
 
     void lookupSymbolsAsync(ArrayRef<LookupRequest> Request,
                             SymbolLookupCompleteFn Complete) override {
-      std::vector<ExecutorSymbolDef> Result;
+      std::vector<std::optional<ExecutorSymbolDef>> Result;
       EXPECT_EQ(Request.size(), 1u);
       for (auto &LR : Request) {
         EXPECT_EQ(LR.Symbols.size(), 1u);
         for (auto &Sym : LR.Symbols) {
           if (*Sym.first == "_testFunc") {
             ExecutorSymbolDef Def{ExecutorAddr::fromPtr((void *)0x1000),
                                   JITSymbolFlags::Exported};
-            Result.push_back(Def);
+            Result.emplace_back(Def);
           } else {
             ADD_FAILURE() << "unexpected symbol request " << *Sym.first;
           }
