[PATCH] D75508: [mlir] ExecutionEngine: fix assertion on the error path

[Alex Zinenko via Phabricator via llvm-commits]

ftynse created this revision.
ftynse added reviewers: rriddle, nicolasvasilache.
Herald added subscribers: llvm-commits, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, burmako, jpienaar, mehdi_amini.
Herald added a project: LLVM.
ftynse updated this revision to Diff 247826.
ftynse added a comment.
ftynse added a reviewer: mehdi_amini.

drop debug


MLIR ExecutionEngine and derived tools (e.g., mlir-cpu-runner) would trigger an
assertion inside ORC JIT while ExecutionEngine is being destructed after a
failed linking due to a missing function definition. The reason for this is the
JIT lookup that may return an Error referring to strings stored internally by
the JIT. If the Error outlives the ExecutionEngine, it would want have a
dangling reference, which is currently caught by an assertion inside JIT thanks
to hand-rolled reference counting. Rewrap the error message into a string
before returning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75508

Files:
  mlir/lib/ExecutionEngine/ExecutionEngine.cpp


Index: mlir/lib/ExecutionEngine/ExecutionEngine.cpp
===================================================================
--- mlir/lib/ExecutionEngine/ExecutionEngine.cpp
+++ mlir/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -294,8 +294,21 @@
 
 Expected<void (*)(void **)> ExecutionEngine::lookup(StringRef name) const {
   auto expectedSymbol = jit->lookup(makePackedFunctionName(name));
-  if (!expectedSymbol)
-    return expectedSymbol.takeError();
+
+  // JIT lookup may return an Error referring to strings stored internally by
+  // the JIT. If the Error outlives the ExecutionEngine, it would want have a
+  // dangling reference, which is currently caught by an assertion inside JIT
+  // thanks to hand-rolled reference counting. Rewrap the error message into a
+  // string before returning. Alternatively, ORC JIT should consider copying
+  // the string into the error message.
+  if (!expectedSymbol) {
+    std::string errorMessage;
+    llvm::raw_string_ostream os(errorMessage);
+    llvm::handleAllErrors(expectedSymbol.takeError(),
+                          [&os](llvm::ErrorInfoBase &ei) { ei.log(os); });
+    return make_string_error(os.str());
+  }
+
   auto rawFPtr = expectedSymbol->getAddress();
   auto fptr = reinterpret_cast<void (*)(void **)>(rawFPtr);
   if (!fptr)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D75508.247826.patch
Type: text/x-patch
Size: 1295 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200303/d300515b/attachment.bin>