[ExecutionEngine] FindFunctionNamed: Skip declarations

[loladiro]

Summary

Basically all other methods that look up functions by name skip them if they are mere declarations.
Do the same in FindFunctionNamed.

[loladiro]

Same change in the overridden method in MCJIT.

[lhames]

Looks good to me. Thanks Keno!

[lhames]

Actually, it would be great if you could add a unit test for this too - it doesn't look like anyone is testing FundFunctionNamed yet.

llvm/trunk/lib/ExecutionEngine/ExecutionEngine.cpp

if (Found == M) {
       return true;
     }
   }
   return false;
 }
 
 Function *ExecutionEngine::FindFunctionNamed(const char *FnName) {
   for (unsigned i = 0, e = Modules.size(); i != e; ++i) {
-    if (Function *F = Modules[i]->getFunction(FnName))
+    Function *F = Modules[i]->getFunction(FnName);
+    if (F && !F->isDeclaration())
       return F;
   }
   return nullptr;
 }
 
 
 void *ExecutionEngineState::RemoveMapping(const GlobalValue *ToUnmap) {
   GlobalAddressMapTy::iterator I = GlobalAddressMap.find(ToUnmap);

llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp

void MCJIT::runStaticConstructorsDestructors(bool isDtors) {
   runStaticConstructorsDestructorsInModulePtrSet(
       isDtors, OwnedModules.begin_finalized(), OwnedModules.end_finalized());
 }
 
 Function *MCJIT::FindFunctionNamedInModulePtrSet(const char *FnName,
                                                  ModulePtrSet::iterator I,
                                                  ModulePtrSet::iterator E) {
   for (; I != E; ++I) {
-    if (Function *F = (*I)->getFunction(FnName))
+    Function *F = (*I)->getFunction(FnName);
+    if (F && !F->isDeclaration())
       return F;
   }
   return nullptr;
 }
 
 Function *MCJIT::FindFunctionNamed(const char *FnName) {
   Function *F = FindFunctionNamedInModulePtrSet(
       FnName, OwnedModules.begin_added(), OwnedModules.end_added());

llvm/trunk/unittests/ExecutionEngine/MCJIT/MCJITMultipleModuleTest.cpp

TEST_F(MCJITMultipleModuleTest, cross_module_dependency_case3) {
   TheJIT->addModule(std::move(B));
 
   uint64_t ptr = TheJIT->getFunctionAddress(FB1->getName().str());
   checkAccumulate(ptr);
 
   ptr = TheJIT->getFunctionAddress(FB2->getName().str());
   checkAccumulate(ptr);
 }
+
+// Test that FindFunctionNamed finds the definition of
+// a function in the correct module. We check two functions
+// in two different modules, to make sure that for at least
+// one of them MCJIT had to ignore the extern declaration.
+TEST_F(MCJITMultipleModuleTest, FindFunctionNamed_test) {
+  SKIP_UNSUPPORTED_PLATFORM;
+
+  std::unique_ptr<Module> A, B;
+  Function *FA, *FB1, *FB2;
+  createCrossModuleRecursiveCase(A, FA, B, FB1, FB2);
+
+  createJIT(std::move(A));
+  TheJIT->addModule(std::move(B));
+
+  EXPECT_EQ(FA, TheJIT->FindFunctionNamed(FA->getName().data()));
+  EXPECT_EQ(FB1, TheJIT->FindFunctionNamed(FB1->getName().data()));
+}
+
 }