[llvm] 9f1dcdc - [JITLink] Allow JITLinkContext::notifyResolved to return an Error.

[Lang Hames via llvm-commits]

Author: Lang Hames
Date: 2020-07-30T15:26:18-07:00
New Revision: 9f1dcdca71c4109354ec916ad27c24caf269c705

URL: https://github.com/llvm/llvm-project/commit/9f1dcdca71c4109354ec916ad27c24caf269c705
DIFF: https://github.com/llvm/llvm-project/commit/9f1dcdca71c4109354ec916ad27c24caf269c705.diff

LOG: [JITLink] Allow JITLinkContext::notifyResolved to return an Error.

This allows clients to detect invalid transformations applied by JITLink passes
(e.g. inserting or removing symbols in unexpected ways) and terminate linking
with an error.

This change is used to simplify the error propagation logic in
ObjectLinkingLayer.

Added: 
    

Modified: 
    llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
    llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
    llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
index 76f9dea4160f..46b574750735 100644
--- a/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
+++ b/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h
@@ -1279,7 +1279,11 @@ class JITLinkContext {
   /// their final memory locations in the target process. At this point the
   /// LinkGraph can be inspected to build a symbol table, however the block
   /// content will not generally have been copied to the target location yet.
-  virtual void notifyResolved(LinkGraph &G) = 0;
+  ///
+  /// If the client detects an error in the LinkGraph state (e.g. unexpected or
+  /// missing symbols) they may return an error here. The error will be
+  /// propagated to notifyFailed and the linker will bail out.
+  virtual Error notifyResolved(LinkGraph &G) = 0;
 
   /// Called by JITLink to notify the context that the object has been
   /// finalized (i.e. emitted to memory and memory permissions set). If all of

diff  --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 1d76a49939dc..e74dd4d5c8b1 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -67,7 +67,9 @@ void JITLinkerBase::linkPhase1(std::unique_ptr<JITLinkerBase> Self) {
   // Notify client that the defined symbols have been assigned addresses.
   LLVM_DEBUG(
       { dbgs() << "Resolving symbols defined in " << G->getName() << "\n"; });
-  Ctx->notifyResolved(*G);
+
+  if (auto Err = Ctx->notifyResolved(*G))
+    return Ctx->notifyFailed(std::move(Err));
 
   auto ExternalSymbols = getExternalSymbolNames();
 

diff  --git a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
index 8ad2e338ca82..5b828ed84462 100644
--- a/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
+++ b/llvm/lib/ExecutionEngine/Orc/ObjectLinkingLayer.cpp
@@ -97,7 +97,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
               });
   }
 
-  void notifyResolved(LinkGraph &G) override {
+  Error notifyResolved(LinkGraph &G) override {
     auto &ES = Layer.getExecutionSession();
 
     SymbolFlagsMap ExtraSymbolsToClaim;
@@ -143,7 +143,7 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
 
     if (!ExtraSymbolsToClaim.empty())
       if (auto Err = MR.defineMaterializing(ExtraSymbolsToClaim))
-        return notifyFailed(std::move(Err));
+        return Err;
 
     {
 
@@ -169,12 +169,9 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       }
 
       // If there were missing symbols then report the error.
-      if (!MissingSymbols.empty()) {
-        ES.reportError(make_error<MissingSymbolDefinitions>(
-            G.getName(), std::move(MissingSymbols)));
-        MR.failMaterialization();
-        return;
-      }
+      if (!MissingSymbols.empty())
+        return make_error<MissingSymbolDefinitions>(G.getName(),
+                                                    std::move(MissingSymbols));
 
       // If there are more definitions than expected, add them to the
       // ExtraSymbols vector.
@@ -186,20 +183,16 @@ class ObjectLinkingLayerJITLinkContext final : public JITLinkContext {
       }
 
       // If there were extra definitions then report the error.
-      if (!ExtraSymbols.empty()) {
-        ES.reportError(make_error<UnexpectedSymbolDefinitions>(
-            G.getName(), std::move(ExtraSymbols)));
-        MR.failMaterialization();
-        return;
-      }
+      if (!ExtraSymbols.empty())
+        return make_error<UnexpectedSymbolDefinitions>(G.getName(),
+                                                       std::move(ExtraSymbols));
     }
 
-    if (auto Err = MR.notifyResolved(InternedResult)) {
-      Layer.getExecutionSession().reportError(std::move(Err));
-      MR.failMaterialization();
-      return;
-    }
+    if (auto Err = MR.notifyResolved(InternedResult))
+      return Err;
+
     Layer.notifyLoaded(MR);
+    return Error::success();
   }
 
   void notifyFinalized(