[llvm] [JIT] Fix crash in unit tests (PR #113492)

Kai Nacke via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 24 13:17:38 PDT 2024


https://github.com/redstar updated https://github.com/llvm/llvm-project/pull/113492

>From 09444174a7640c6b1ae68639423fb75de0e7d3b5 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Wed, 23 Oct 2024 16:13:39 -0400
Subject: [PATCH 1/4] [JIT] Fix crash in unit tests

The unit tests `ReOptimizeLayerTest.BasicReOptimization` and `JITLinkRedirectionManagerTest.BasicRedirectionOperation` are failing for me with the error:

```
Program aborted due to an unhandled Error:
Error value was Success. (Note: Success values must still be checked prior to being destroyed).
```

The error is raised when a value is assigned to `Err`, due to the the missing `ErrorAsOutParameter`.
---
 .../llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h  | 1 +
 1 file changed, 1 insertion(+)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
index 52f284c89bdade..5b73635d3b1cac 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
@@ -59,6 +59,7 @@ class JITLinkRedirectableSymbolManager : public RedirectableSymbolManager,
             ObjLinkingLayer.getExecutionSession().getTargetTriple())),
         PtrJumpStubCreator(jitlink::getPointerJumpStubCreator(
             ObjLinkingLayer.getExecutionSession().getTargetTriple())) {
+    ErrorAsOutParameter _(&Err);
     if (!AnonymousPtrCreator || !PtrJumpStubCreator)
       Err = make_error<StringError>("Architecture not supported",
                                     inconvertibleErrorCode());

>From a837581fdd8d82829f4c7da36ecae50301a672d3 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Wed, 23 Oct 2024 17:11:10 -0400
Subject: [PATCH 2/4] Fix another problem

---
 .../llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
index 5b73635d3b1cac..69489fb50bc59a 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
@@ -60,12 +60,12 @@ class JITLinkRedirectableSymbolManager : public RedirectableSymbolManager,
         PtrJumpStubCreator(jitlink::getPointerJumpStubCreator(
             ObjLinkingLayer.getExecutionSession().getTargetTriple())) {
     ErrorAsOutParameter _(&Err);
+    ObjLinkingLayer.getExecutionSession().registerResourceManager(*this);
     if (!AnonymousPtrCreator || !PtrJumpStubCreator)
       Err = make_error<StringError>("Architecture not supported",
                                     inconvertibleErrorCode());
     if (Err)
       return;
-    ObjLinkingLayer.getExecutionSession().registerResourceManager(*this);
   }
 
   ~JITLinkRedirectableSymbolManager() {

>From 61508af6cb2452f959eeef37aae51758c884e970 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Thu, 24 Oct 2024 08:56:11 -0400
Subject: [PATCH 3/4] Remove now useless if

---
 .../llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
index 69489fb50bc59a..a7c551f279cbc2 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
@@ -64,8 +64,6 @@ class JITLinkRedirectableSymbolManager : public RedirectableSymbolManager,
     if (!AnonymousPtrCreator || !PtrJumpStubCreator)
       Err = make_error<StringError>("Architecture not supported",
                                     inconvertibleErrorCode());
-    if (Err)
-      return;
   }
 
   ~JITLinkRedirectableSymbolManager() {

>From 4a33b7220634c5ed97d8703dd58165eb6737a748 Mon Sep 17 00:00:00 2001
From: Kai Nacke <kai.peter.nacke at ibm.com>
Date: Thu, 24 Oct 2024 16:17:23 -0400
Subject: [PATCH 4/4] Move instantiation into factory method

---
 .../Orc/JITLinkRedirectableSymbolManager.h    | 29 ++++++++++---------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
index a7c551f279cbc2..379bb5bc92f1fe 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/JITLinkRedirectableSymbolManager.h
@@ -26,11 +26,16 @@ class JITLinkRedirectableSymbolManager : public RedirectableSymbolManager,
   /// Create redirection manager that uses JITLink based implementaion.
   static Expected<std::unique_ptr<RedirectableSymbolManager>>
   Create(ObjectLinkingLayer &ObjLinkingLayer, JITDylib &JD) {
-    Error Err = Error::success();
+    auto AnonymousPtrCreator(jitlink::getAnonymousPointerCreator(
+        ObjLinkingLayer.getExecutionSession().getTargetTriple()));
+    auto PtrJumpStubCreator(jitlink::getPointerJumpStubCreator(
+        ObjLinkingLayer.getExecutionSession().getTargetTriple()));
+    if (!AnonymousPtrCreator || !PtrJumpStubCreator)
+      return make_error<StringError>("Architecture not supported",
+                                     inconvertibleErrorCode());
     auto RM = std::unique_ptr<RedirectableSymbolManager>(
-        new JITLinkRedirectableSymbolManager(ObjLinkingLayer, JD, Err));
-    if (Err)
-      return Err;
+        new JITLinkRedirectableSymbolManager(
+            ObjLinkingLayer, JD, AnonymousPtrCreator, PtrJumpStubCreator));
     return std::move(RM);
   }
 
@@ -52,18 +57,14 @@ class JITLinkRedirectableSymbolManager : public RedirectableSymbolManager,
   constexpr static StringRef JumpStubTableName = "$IND_JUMP_";
   constexpr static StringRef StubPtrTableName = "$__IND_JUMP_PTRS";
 
-  JITLinkRedirectableSymbolManager(ObjectLinkingLayer &ObjLinkingLayer,
-                                   JITDylib &JD, Error &Err)
+  JITLinkRedirectableSymbolManager(
+      ObjectLinkingLayer &ObjLinkingLayer, JITDylib &JD,
+      jitlink::AnonymousPointerCreator &AnonymousPtrCreator,
+      jitlink::PointerJumpStubCreator &PtrJumpStubCreator)
       : ObjLinkingLayer(ObjLinkingLayer), JD(JD),
-        AnonymousPtrCreator(jitlink::getAnonymousPointerCreator(
-            ObjLinkingLayer.getExecutionSession().getTargetTriple())),
-        PtrJumpStubCreator(jitlink::getPointerJumpStubCreator(
-            ObjLinkingLayer.getExecutionSession().getTargetTriple())) {
-    ErrorAsOutParameter _(&Err);
+        AnonymousPtrCreator(std::move(AnonymousPtrCreator)),
+        PtrJumpStubCreator(std::move(PtrJumpStubCreator)) {
     ObjLinkingLayer.getExecutionSession().registerResourceManager(*this);
-    if (!AnonymousPtrCreator || !PtrJumpStubCreator)
-      Err = make_error<StringError>("Architecture not supported",
-                                    inconvertibleErrorCode());
   }
 
   ~JITLinkRedirectableSymbolManager() {



More information about the llvm-commits mailing list