Add missing vtable anchors

[weimingz]

Summary

This patch adds anchor() for MemoryBuffer, raw_fd_ostream, RTDyldMemoryManager, SectionMemoryManager, etc.

[weimingz]

When I statically linking a project (built with RTTI) against LLVM OrcJIT (default built with no-RTTI), I got a couple of "undefined reference to type info" error.

Looks similar to http://lists.llvm.org/pipermail/llvm-dev/2016-February/095671.html

However, I'm not sure where to put the definition of anchor() for ObjectMemoryBuffer class. It's used by ThinLTOCodeGenerator.cpp and MCJIT.cpp, which belong to different libs.

[dblaikie]

Looks like ObjectMemoryBuffer is part of ExecutionEngine, so having libLTO
dependent on ExecutionEngine seems basically correct for the dependencies
that are there already. Whether or not those dependencies should be there
is another question, I guess. (maybe ObjectMemoryBuffer should be moved
somewhere more common, etc - but I wouldn't worry about doing that unless
the LTO dependency breaks things)

[weimingz]

If I choose to define ObjectMemoryBuffer::anchor() in MCJIT.cpp (the module that uses ObjectMemoryBuffer), then I got error like:
../../lib/libLLVMLTO.a(ThinLTOCodeGenerator.cpp.o): In function `ObjectMemoryBuffer':
llvm/ExecutionEngine/ObjectMemoryBuffer.h:41: undefined reference to `vtable for llvm::ObjectMemoryBuffer'

[dblaikie]

You'd have to add the ExecutionEngine library as a dependency from the LTO
library - by modifying lib/LTO/LLVMBuild.txt - then it should link, I would
think.

[weimingz]

add dependency upon MCJIT for LTO due to the use of ObjectMemoryBuffer

[dblaikie]

Is it sufficient to put the dependency MCJIT in lib/LTO/LLVMBuild.txt (since that's where the dependency on is) & not in the tools that /use/ the LTO lib (I expect they should pickup the indirect dependency implicitly?)?

[weimingz]

I put a dependency on tools/llvm-lto as well. From my build (default CMake config), it works.

[dblaikie]

I meant what happens if you remove the changes to tools/lto/LLVMBuild.txt
and tools/llvm-lto/LLVMBuild.txt and /only/ have a change to
lib/LTO/LLVMBuild.txt - is that sufficient to make the build work?

[weimingz]

In D45244#1064527, @dblaikie wrote:

I meant what happens if you remove the changes to tools/lto/LLVMBuild.txt
and tools/llvm-lto/LLVMBuild.txt and /only/ have a change to
lib/LTO/LLVMBuild.txt - is that sufficient to make the build work?

I tried before and it didn't work. I think it's because lib/LTO is for the generation of libLLVMLTO.a (at least for the default build config), so the dependency on MCJIT has no effect on the archive generation.

[dblaikie]

That sort of surprises me - if that were the case, there would be no point
having LLVMBuild.txt files in any libraries, I would think?

[weimingz]

In D45244#1064649, @dblaikie wrote:

That sort of surprises me - if that were the case, there would be no point
having LLVMBuild.txt files in any libraries, I would think?

Hi David,

Do you mean changes like:
diff --git a/lib/LTO/CMakeLists.txt b/lib/LTO/CMakeLists.txt

• a/lib/LTO/CMakeLists.txt

+++ b/lib/LTO/CMakeLists.txt
@@ -13,4 +13,5 @@ add_llvm_library(LLVMLTO

DEPENDS
intrinsics_gen
llvm_vcsrevision_h

+ LLVMLTO
)
In this case, I think the dependency rule won't affect the build of libLLVMLTO.a. I tried and it didn't work. Or maybe I misunderstood what you meant?

[dblaikie]

I meant that lib/LTO/LLVMBuild.txt should include "ExecutionEngine" in its
required_libraries. I wouldn't expect any changes would be needed to
tools/lto/* or tools/llvm-lto/*.

[weimingz]

@dblaikie you're right. I modified CMakeLists.txt rather than LLVMBuild.txt

[dblaikie]

Great - thanks for sticking with it, sorry for my dodgy explanations!

If you're adding in anchors, are you using -Wweak-vtables to find them? If you're pushing through to make LLVM (& hopefully Clang/other LLVM subprojects) -Wweak-vtables clean, perhaps you can turn on the warning in the CMake config once it's clean so we don't regress this again? (it'd also be interesting to do some kind of analysis to see whether avoiding weak vtables is /actually/ worthwhile - like maybe making a temporary/local/non-committed change to change all the explicit anchors into inline functions (unanchoring them) & see if builds are much larger or longer, etc)

[weimingz]

In D45244#1064905, @dblaikie wrote:

Great - thanks for sticking with it, sorry for my dodgy explanations!

If you're adding in anchors, are you using -Wweak-vtables to find them? If you're pushing through to make LLVM (& hopefully Clang/other LLVM subprojects) -Wweak-vtables clean, perhaps you can turn on the warning in the CMake config once it's clean so we don't regress this again? (it'd also be interesting to do some kind of analysis to see whether avoiding weak vtables is /actually/ worthwhile - like maybe making a temporary/local/non-committed change to change all the explicit anchors into inline functions (unanchoring them) & see if builds are much larger or longer, etc)

I got linker error when my code tries to statically link against some LLVM libs (like libLLVMOrc). It's a good idea to turn on -Wweak-vtables to check if we miss other cases. I will try it. Thanks

include/llvm/ExecutionEngine/ObjectMemoryBuffer.h

public:
 
   StringRef getBufferIdentifier() const override { return BufferName; }
 
   BufferKind getBufferKind() const override { return MemoryBuffer_Malloc; }
 
 private:
   SmallVector<char, 0> SV;
   std::string BufferName;
+  // FIXME: both ThinLTOCodeGenerator.cpp and MCJIT.cpp use this class.
+  void anchor() override{};
 };
 
 } // namespace llvm
 
 #endif

include/llvm/ExecutionEngine/RTDyldMemoryManager.h

public:
   /// the memory manager is being notified will be finalized immediately after
   /// the memory manager returns from this call.
   ///
   /// Memory managers which are preparing code for execution in an external
   /// address space can use this call to remap the section addresses for the
   /// newly loaded object.
   virtual void notifyObjectLoaded(ExecutionEngine *EE,
                                   const object::ObjectFile &) {}
+
+private:
+  void anchor() override;
 };
 
 // RuntimeDyld clients often want to handle the memory management of
 // what gets placed where. For JIT clients, this is the subset of
 // JITMemoryManager required for dynamic loading of binaries.
 //
 // FIXME: As the RuntimeDyld fills out, additional routines will be needed
 //        for the varying types of objects to be allocated.

 
 protected:
   struct EHFrame {
     uint8_t *Addr;
     size_t Size;
   };
   typedef std::vector<EHFrame> EHFrameInfos;
   EHFrameInfos EHFrames;
+
+private:
+  void anchor() override;
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(
     RTDyldMemoryManager, LLVMMCJITMemoryManagerRef)
 
 } // end namespace llvm
 
 #endif // LLVM_EXECUTIONENGINE_RTDYLDMEMORYMANAGER_H

include/llvm/ExecutionEngine/SectionMemoryManager.h

private:
   };
 
   uint8_t *allocateSection(AllocationPurpose Purpose, uintptr_t Size,
                            unsigned Alignment);
 
   std::error_code applyMemoryGroupPermissions(MemoryGroup &MemGroup,
                                               unsigned Permissions);
 
+  void anchor() override;
+
   MemoryGroup CodeMem;
   MemoryGroup RWDataMem;
   MemoryGroup RODataMem;
   MemoryMapper &MMapper;
 };
 
 } // end namespace llvm
 
 #endif // LLVM_EXECUTION_ENGINE_SECTION_MEMORY_MANAGER_H

include/llvm/Support/MemoryBuffer.h

enum BufferKind {
     MemoryBuffer_MMap
   };
 
   /// Return information on the memory mechanism used to support the
   /// MemoryBuffer.
   virtual BufferKind getBufferKind() const = 0;
 
   MemoryBufferRef getMemBufferRef() const;
+
+private:
+  virtual void anchor();
 };
 
 /// This class is an extension of MemoryBuffer, which allows copy-on-write
 /// access to the underlying contents.  It only supports creation methods that
 /// are guaranteed to produce a writable buffer.  For example, mapping a file
 /// read-only is not supported.
 class WritableMemoryBuffer : public MemoryBuffer {
 protected:

include/llvm/Support/raw_ostream.h

private:
 
   /// Flush the current buffer, which is known to be non-empty. This outputs the
   /// currently buffered data and resets the buffer to empty.
   void flush_nonempty();
 
   /// Copy data into the buffer. Size must not be greater than the number of
   /// unused bytes in the buffer.
   void copy_to_buffer(const char *Ptr, size_t Size);
+
+  virtual void anchor();
 };
 
 /// An abstract base class for streams implementations that also support a
 /// pwrite operation. This is useful for code that can mostly stream out data,
 /// but needs to patch in a header that needs to know the output size.
 class raw_pwrite_stream : public raw_ostream {
   virtual void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) = 0;
+  void anchor() override;
 
 public:
   explicit raw_pwrite_stream(bool Unbuffered = false)
       : raw_ostream(Unbuffered) {}
   void pwrite(const char *Ptr, size_t Size, uint64_t Offset) {
 #ifndef NDBEBUG
     uint64_t Pos = tell();
     // /dev/null always reports a pos of 0, so we cannot perform this check
class raw_fd_ostream : public raw_pwrite_stream {
   uint64_t current_pos() const override { return pos; }
 
   /// Determine an efficient buffer size.
   size_t preferred_buffer_size() const override;
 
   /// Set the flag indicating that an output error has been encountered.
   void error_detected(std::error_code EC) { this->EC = EC; }
 
+  void anchor() override;
+
 public:
   /// Open the specified file for writing. If an error occurs, information
   /// about the error is put into EC, and the stream should be immediately
   /// destroyed;
   /// \p Flags allows optional flags to control how the file will be opened.
   ///
   /// As a special case, if Filename is "-", then the stream will use
   /// STDOUT_FILENO instead of opening a file. This will not close the stdout

lib/ExecutionEngine/MCJIT/MCJIT.h

public:
   // MCJIT doesn't support logical dylibs.
   JITSymbol findSymbolInLogicalDylib(const std::string &Name) override {
     return nullptr;
   }
 
 private:
   MCJIT &ParentEngine;
   std::shared_ptr<LegacyJITSymbolResolver> ClientResolver;
+  void anchor() override;
 };
 
 // About Module states: added->loaded->finalized.
 //
 // The purpose of the "added" state is having modules in standby. (added=known
 // but not compiled). The idea is that you can add a module to provide function
 // definitions but if nothing in that module is referenced by a module in which
 // a function is executed (note the wording here because it's not exactly the

lib/ExecutionEngine/MCJIT/MCJIT.cpp

 LinkingSymbolResolver::findSymbol(const std::string &Name) {
   auto Result = ParentEngine.findSymbol(Name, false);
   if (Result)
     return Result;
   if (ParentEngine.isSymbolSearchingDisabled())
     return nullptr;
   return ClientResolver->findSymbol(Name);
 }
+
+void LinkingSymbolResolver::anchor() {}

lib/ExecutionEngine/RuntimeDyld/RTDyldMemoryManager.cpp

void *RTDyldMemoryManager::getPointerToNamedFunction(const std::string &Name,
 
   if (!Addr && AbortOnFailure)
     report_fatal_error("Program used external function '" + Name +
                        "' which could not be resolved!");
 
   return (void*)Addr;
 }
 
+void RTDyldMemoryManager::anchor() {}
+void MCJITMemoryManager::anchor() {}
 } // namespace llvm

lib/ExecutionEngine/SectionMemoryManager.cpp

SectionMemoryManager::~SectionMemoryManager() {
   for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
     for (sys::MemoryBlock &Block : Group->AllocatedMem)
       MMapper.releaseMappedMemory(Block);
   }
 }
 
 SectionMemoryManager::MemoryMapper::~MemoryMapper() {}
 
+void SectionMemoryManager::anchor() {}
+
 namespace {
 // Trivial implementation of SectionMemoryManager::MemoryMapper that just calls
 // into sys::Memory.
 class DefaultMMapper final : public SectionMemoryManager::MemoryMapper {
 public:
   sys::MemoryBlock
   allocateMappedMemory(SectionMemoryManager::AllocationPurpose Purpose,
                        size_t NumBytes, const sys::MemoryBlock *const NearBlock,

lib/Support/MemoryBuffer.cpp

MemoryBuffer::getFileAsStream(const Twine &Filename) {
   return Ret;
 }
 
 MemoryBufferRef MemoryBuffer::getMemBufferRef() const {
   StringRef Data = getBuffer();
   StringRef Identifier = getBufferIdentifier();
   return MemoryBufferRef(Data, Identifier);
 }
+
+void MemoryBuffer::anchor() {}

lib/Support/raw_ostream.cpp

while (NumSpaces) {
     unsigned NumToWrite = std::min(NumSpaces,
                                    (unsigned)array_lengthof(Spaces)-1);
     write(Spaces, NumToWrite);
     NumSpaces -= NumToWrite;
   }
   return *this;
 }
 
+void raw_ostream::anchor() {}
+
 //===----------------------------------------------------------------------===//
 //  Formatted Output
 //===----------------------------------------------------------------------===//
 
 // Out of line virtual method.
 void format_object_base::home() {
 }
 

 bool raw_fd_ostream::is_displayed() const {
   return sys::Process::FileDescriptorIsDisplayed(FD);
 }
 
 bool raw_fd_ostream::has_colors() const {
   return sys::Process::FileDescriptorHasColors(FD);
 }
 
+void raw_fd_ostream::anchor() {}
+
 //===----------------------------------------------------------------------===//
 //  outs(), errs(), nulls()
 //===----------------------------------------------------------------------===//
 
 /// outs() - This returns a reference to a raw_ostream for standard output.
 /// Use it like: outs() << "foo" << "bar";
 raw_ostream &llvm::outs() {
   // Set buffer settings to model stdout behavior.

 }
 
 uint64_t raw_null_ostream::current_pos() const {
   return 0;
 }
 
 void raw_null_ostream::pwrite_impl(const char *Ptr, size_t Size,
                                    uint64_t Offset) {}
+
+void raw_pwrite_stream::anchor() {}