[MCJIT] Fix a crash in getPointerToFunction

[loladiro]

Summary

If the first function passed to getPointerToFunction has private linkage,
MCJIT crashes, because the TargetMachine's TargetLoweringObjectFile has
not been inititalized yet. Fix this by moving the name resolution past
code generation (which initializes the TargetLoweringObjectFile) in the
common case, and returning null early for declarations with private
linkage.

[lhames]

LGTM. Thanks Keno!

[Eugene.Zelenko]

This fix was not committed.

lib/ExecutionEngine/MCJIT/MCJIT.cpp

 }
 
 // Deprecated.  Use getFunctionAddress instead.
 void *MCJIT::getPointerToFunction(Function *F) {
   MutexGuard locked(lock);
 
   Mangler Mang(TM->getDataLayout());
   SmallString<128> Name;
-  TM->getNameWithPrefix(Name, F, Mang);
 
   if (F->isDeclaration() || F->hasAvailableExternallyLinkage()) {
+    if (F->hasPrivateLinkage())
+      return nullptr;
+    TM->getNameWithPrefix(Name, F, Mang);
     bool AbortOnFailure = !F->hasExternalWeakLinkage();
     void *Addr = getPointerToNamedFunction(Name, AbortOnFailure);
     updateGlobalMapping(F, Addr);
     return Addr;
   }
 
   Module *M = F->getParent();
   bool HasBeenAddedButNotLoaded = OwnedModules.hasModuleBeenAddedButNotLoaded(M);
 
   // Make sure the relevant module has been compiled and loaded.
   if (HasBeenAddedButNotLoaded)
     generateCodeForModule(M);
   else if (!OwnedModules.hasModuleBeenLoaded(M)) {
     // If this function doesn't belong to one of our modules, we're done.
     // FIXME: Asking for the pointer to a function that hasn't been registered,
     //        and isn't a declaration (which is handled above) should probably
     //        be an assertion.
     return nullptr;
   }
 
+  // Do not move this earlier - this function may rely on the target machine
+  // having been initialized by code generation.
+  TM->getNameWithPrefix(Name, F, Mang);
+
   // FIXME: Should the Dyld be retaining module information? Probably not.
   //
   // This is the accessor for the target address, so make sure to check the
   // load address of the symbol, not the local address.
   return (void*)Dyld.getSymbol(Name).getAddress();
 }
 
 void MCJIT::runStaticConstructorsDestructorsInModulePtrSet(

unittests/ExecutionEngine/MCJIT/MCJITTest.cpp

TEST_F(MCJITTest, multiple_decl_lookups) {
   createJIT(std::move(M));
   void *A = TheJIT->getPointerToFunction(Foo);
   void *B = TheJIT->getPointerToFunction(Foo);
 
   EXPECT_TRUE(A != 0) << "Failed lookup - test not correctly configured.";
   EXPECT_EQ(A, B) << "Repeat calls to getPointerToFunction fail.";
 }
 
+TEST_F(MCJITTest, private_function_lookup) {
+  SKIP_UNSUPPORTED_PLATFORM;
+
+  Function *Foo = insertMainFunction(M.get(), 0, GlobalValue::PrivateLinkage);
+  createJIT(std::move(M));
+  void *A = TheJIT->getPointerToFunction(Foo);
+  void *B = TheJIT->getPointerToFunction(Foo);
+
+  EXPECT_TRUE(A != 0) << "Failed lookup - test not correctly configured.";
+  EXPECT_EQ(A, B)
+      << "Repeat calls to getPointerToFunction fail for private function.";
+}
 }

unittests/ExecutionEngine/MCJIT/MCJITTestBase.h

TrivialModuleBuilder(const std::string &Triple)
     : Builder(Context), BuilderTriple(Triple) {}
 
   Module *createEmptyModule(StringRef Name = StringRef()) {
     Module * M = new Module(Name, Context);
     M->setTargetTriple(Triple::normalize(BuilderTriple));
     return M;
   }
 
-  template<typename FuncType>
-  Function *startFunction(Module *M, StringRef Name) {
+  template <typename FuncType>
+  Function *startFunction(
+      Module *M, StringRef Name,
+      GlobalValue::LinkageTypes linkage = GlobalValue::ExternalLinkage) {
     Function *Result = Function::Create(
-      TypeBuilder<FuncType, false>::get(Context),
-      GlobalValue::ExternalLinkage, Name, M);
+        TypeBuilder<FuncType, false>::get(Context), linkage, Name, M);
 
     BasicBlock *BB = BasicBlock::Create(Context, Name, Result);
     Builder.SetInsertPoint(BB);
 
     return Result;
   }
 
   void endFunctionWithRet(Function *Func, Value *RetValue) {
Function *insertSimpleCallFunction(Module *M, Function *Callee) {
     Value *ReturnCode = Builder.CreateCall(Callee, CallArgs);
     Builder.CreateRet(ReturnCode);
     return Result;
   }
 
   // Inserts a function named 'main' that returns a uint32_t:
   //    int32_t main() { return X; }
   // where X is given by returnCode
-  Function *insertMainFunction(Module *M, uint32_t returnCode) {
-    Function *Result = startFunction<int32_t(void)>(M, "main");
+  Function *insertMainFunction(
+      Module *M, uint32_t returnCode,
+      GlobalValue::LinkageTypes linkage = GlobalValue::ExternalLinkage) {
+    Function *Result = startFunction<int32_t(void)>(M, "main", linkage);
 
     Value *ReturnVal = ConstantInt::get(Context, APInt(32, returnCode));
     endFunctionWithRet(Result, ReturnVal);
 
     return Result;
   }
 
   // Inserts a function