From: Lang Hames Date: Wed, 28 Jan 2015 01:30:37 +0000 (+0000) Subject: Revert r227247 and r227228: "Add weak symbol support to RuntimeDyld". X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=2edbad28d2c26b0a26622393a0acb68beef3621b Revert r227247 and r227228: "Add weak symbol support to RuntimeDyld". This has wider implications than I expected when I reviewed the patch: It can cause JIT crashes where clients have used the default value for AbortOnFailure during symbol lookup. I'm currently investigating alternative approaches and I hope to have this back in tree soon. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@227287 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp index 81b8eb2238f..fb53ba96055 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp @@ -168,7 +168,6 @@ RuntimeDyldImpl::loadObjectImpl(const object::ObjectFile &Obj) { uint32_t Flags = I->getFlags(); bool IsCommon = Flags & SymbolRef::SF_Common; - bool IsWeak = Flags & SymbolRef::SF_Weak; if (IsCommon) CommonSymbols.push_back(*I); else { @@ -189,11 +188,6 @@ RuntimeDyldImpl::loadObjectImpl(const object::ObjectFile &Obj) { continue; StringRef SectionData; Check(SI->getContents(SectionData)); - // TODO: It make make sense to delay emitting the section for weak - // symbols until they are actually required, but that's not possible - // currently, because we only know whether we will need the symbol - // in resolveRelocations, which happens after we have already finalized - // the Load. bool IsCode = SI->isText(); unsigned SectionID = findOrEmitSection(Obj, *SI, IsCode, LocalSections); @@ -204,11 +198,7 @@ RuntimeDyldImpl::loadObjectImpl(const object::ObjectFile &Obj) { SymbolInfo::Visibility Vis = (Flags & SymbolRef::SF_Exported) ? SymbolInfo::Default : SymbolInfo::Hidden; - if (!IsWeak) { - GlobalSymbolTable[Name] = SymbolInfo(SectionID, SectOffset, Vis); - } else { - WeakSymbolTable[Name] = SymbolInfo(SectionID, SectOffset, Vis); - } + GlobalSymbolTable[Name] = SymbolInfo(SectionID, SectOffset, Vis); } } } @@ -778,21 +768,6 @@ void RuntimeDyldImpl::resolveExternalSymbols() { SymInfo.getOffset(); } - // If we didn't find the symbol yet, and it is present in the weak symbol - // table, the definition from this object file needs to be used, so emit - // it now - if (!Addr) { - RTDyldSymbolTable::const_iterator Loc = WeakSymbolTable.find(Name); - if (Loc != WeakSymbolTable.end()) { - SymbolInfo SymInfo = Loc->second; - Addr = getSectionLoadAddress(SymInfo.getSectionID()) + SymInfo.getOffset(); - // Since the weak symbol is now, materialized, add it to the - // GlobalSymbolTable. If somebody later asks the ExecutionEngine - // for the address of this symbol that's where it'll look - GlobalSymbolTable[Name] = SymInfo; - } - } - // FIXME: Implement error handling that doesn't kill the host program! if (!Addr) report_fatal_error("Program used external function '" + Name + @@ -809,7 +784,6 @@ void RuntimeDyldImpl::resolveExternalSymbols() { ExternalSymbolRelocations.erase(i); } - WeakSymbolTable.clear(); } //===----------------------------------------------------------------------===// diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp index a01b063831a..0f3ca0f2f39 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp @@ -947,7 +947,6 @@ relocation_iterator RuntimeDyldELF::processRelocationRef( break; } case SymbolRef::ST_Data: - case SymbolRef::ST_Function: case SymbolRef::ST_Unknown: { Value.SymbolName = TargetName.data(); Value.Addend = Addend; diff --git a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h index 69d6cf53034..f37a9a768a2 100644 --- a/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h +++ b/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h @@ -203,9 +203,6 @@ protected: // A global symbol table for symbols from all loaded modules. RTDyldSymbolTable GlobalSymbolTable; - // Like the global symbol table but for weak symbols - RTDyldSymbolTable WeakSymbolTable; - // Keep a map of common symbols to their info pairs typedef std::vector CommonSymbolList; diff --git a/unittests/ExecutionEngine/MCJIT/CMakeLists.txt b/unittests/ExecutionEngine/MCJIT/CMakeLists.txt index dcf8264203e..b10cbb4c9ea 100644 --- a/unittests/ExecutionEngine/MCJIT/CMakeLists.txt +++ b/unittests/ExecutionEngine/MCJIT/CMakeLists.txt @@ -1,6 +1,5 @@ set(LLVM_LINK_COMPONENTS Analysis - AsmParser Core ExecutionEngine IPO diff --git a/unittests/ExecutionEngine/MCJIT/MCJITTest.cpp b/unittests/ExecutionEngine/MCJIT/MCJITTest.cpp index f9ca0fc8717..64d8c2fca04 100644 --- a/unittests/ExecutionEngine/MCJIT/MCJITTest.cpp +++ b/unittests/ExecutionEngine/MCJIT/MCJITTest.cpp @@ -13,7 +13,6 @@ //===----------------------------------------------------------------------===// #include "llvm/ExecutionEngine/MCJIT.h" -#include "llvm/Support/DynamicLibrary.h" #include "MCJITTestBase.h" #include "gtest/gtest.h" @@ -200,45 +199,4 @@ TEST_F(MCJITTest, multiple_decl_lookups) { EXPECT_EQ(A, B) << "Repeat calls to getPointerToFunction fail."; } -// Test weak symbol linking when the weak symbol is present in a shared -// library -TEST_F(MCJITTest, weak_symbol_present) { - SKIP_UNSUPPORTED_PLATFORM; - - int FakeWeakSymbol; - llvm::sys::DynamicLibrary::AddSymbol("FakeWeakSymbol", &FakeWeakSymbol); - createJITFromAssembly( - "$FakeWeakSymbol = comdat any\n" - "@FakeWeakSymbol = linkonce_odr global i32 42, comdat, align 4\n" - "define i32 @weak_test(i32* %arg) {\n" - " %r = icmp eq i32* %arg, @FakeWeakSymbol\n" - " %ret = zext i1 %r to i32\n" - " ret i32 %ret\n" - " }"); - - uint64_t Addr = TheJIT->getFunctionAddress("weak_test");; - EXPECT_TRUE(Addr != 0); - int32_t(*FuncPtr)(int32_t *) = (int32_t(*)(int32_t *))Addr; - EXPECT_EQ(FuncPtr(&FakeWeakSymbol),1); - EXPECT_TRUE(TheJIT->getGlobalValueAddress("FakeWeakSymbol") == 0); -} - -// Test weak symbol linking when the weak symbol is not present in a -// shared library -TEST_F(MCJITTest, weak_symbol_absent) { - SKIP_UNSUPPORTED_PLATFORM; - - SMDiagnostic Error; - createJITFromAssembly( - " $FakeWeakSymbol2 = comdat any\n" - " @FakeWeakSymbol2 = linkonce_odr global i32 42, comdat, align 4\n" - " define i32* @get_weak() {\n" - " ret i32* @FakeWeakSymbol2\n" - " }\n"); - void*(*FuncPtr)() = - (void*(*)(void))TheJIT->getFunctionAddress("get_weak"); - EXPECT_EQ(FuncPtr(),(void*)TheJIT->getGlobalValueAddress("FakeWeakSymbol2")); -} - - } diff --git a/unittests/ExecutionEngine/MCJIT/MCJITTestBase.h b/unittests/ExecutionEngine/MCJIT/MCJITTestBase.h index c09c2724881..35af417bf14 100644 --- a/unittests/ExecutionEngine/MCJIT/MCJITTestBase.h +++ b/unittests/ExecutionEngine/MCJIT/MCJITTestBase.h @@ -18,7 +18,6 @@ #define LLVM_UNITTESTS_EXECUTIONENGINE_MCJIT_MCJITTESTBASE_H #include "MCJITTestAPICommon.h" -#include "llvm/AsmParser/Parser.h" #include "llvm/Config/config.h" #include "llvm/ExecutionEngine/ExecutionEngine.h" #include "llvm/ExecutionEngine/SectionMemoryManager.h" @@ -28,8 +27,6 @@ #include "llvm/IR/Module.h" #include "llvm/IR/TypeBuilder.h" #include "llvm/Support/CodeGen.h" -#include "llvm/Support/SourceMgr.h" -#include "llvm/Support/raw_ostream.h" namespace llvm { @@ -341,22 +338,6 @@ protected: assert(TheJIT.get() != NULL && "error creating MCJIT with EngineBuilder"); } - void createJITFromAssembly(const char *Test) { - SMDiagnostic Error; - M = parseAssemblyString(Test, Error, Context); - M->setTargetTriple(Triple::normalize(BuilderTriple)); - - std::string errMsg; - raw_string_ostream os(errMsg); - Error.print("", os); - - // A failure here means that the test itself is buggy. - if (!M) - report_fatal_error(os.str().c_str()); - - createJIT(std::move(M)); - } - CodeGenOpt::Level OptLevel; Reloc::Model RelocModel; CodeModel::Model CodeModel; diff --git a/unittests/ExecutionEngine/MCJIT/Makefile b/unittests/ExecutionEngine/MCJIT/Makefile index 522ef750df7..2822b20cdda 100644 --- a/unittests/ExecutionEngine/MCJIT/Makefile +++ b/unittests/ExecutionEngine/MCJIT/Makefile @@ -9,7 +9,7 @@ LEVEL = ../../.. TESTNAME = MCJIT -LINK_COMPONENTS := core asmparser ipo mcjit native support +LINK_COMPONENTS := core ipo mcjit native support include $(LEVEL)/Makefile.config include $(LLVM_SRC_ROOT)/unittests/Makefile.unittest