[JIT] Fix crash in unit tests

[redstar]

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.

Fixing this problem uncovered another problem: if AnonymousPtrCreator or PtrJumpStubCreator is an error, then the object is not registered at the resource manager. However, the destructor tries to de-register the object. This fails then with

Assertion `I != ResourceManagers.end() && "RM not registered"'

Fix is to always register the object at the resource manager.

[lhames]

Thanks for answering those questions.

I think there's an even easier solution here: We can hoist the creator functions up into the named constructor and then pass them into the regular constructor, which would no longer be able to error out:

static Expected<std::unique_ptr<RedirectableSymbolManager>>
Create(ObjectLinkingLayer &ObjLinkingLayer, JITDylib &JD) {
  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());
  return std::unique_ptr<RedirectableSymbolManager>(
      new JITLinkRedirectableSymbolManager(ObjLinkingLayer, JD,
      AnonymousPtrCreator, PtrJumpStubCreator));
}

[redstar]

Changed the code according to your proposal. Looks much better now, thanks!

[redstar]

Hm. Why does the PR not close after manually merging?

[lhames]

Weird -- I thought it had been merged too, but it doesn't look like it. Should be fixed now.