MCJIT MachO debugging support

[loladiro]

Summary

Adds the necessary hooks to MCJIT to be able to debug generated MachO files using the GDB JIT debugging interface. Corresponding patch to LLDB in a minute.

[echristo]

Lang has touched this code last...

[lhames]

Hi Keno,

The updatesSectionAddress implementation is a bit funky, but I don't have any better ideas for how to do it.

Regarding the comment:
TODO: Refactor RuntimeDyld.cpp, so we at least don't nede the symbol
thing anymore.

Any chance you could expand on how you see this being refactored in the future? Could you file a bug for it and add a reference to the TODO?

Otherwise this looks good to me.

• Lang.

[loladiro]

Hi Lang,

thanks for taking a look. When I originally wrote this code, I had an idea on how to do it, but I don't think it actually works the way I had hoped, especially because MachO makes the assumption on the original object file lay out in other places as well. I think I'm just to push a new version that removes that comment.

Keno

[lhames]

In D4005#7, @loladiro wrote:

...I think I'm just to push a new version that removes that comment.

That's ok. In that case though, could you tweak the implementation of updateSectionAddress so that it can be safely called multiple times? I believe LLDB sometimes does that (possibly redundantly - with the same "new" address each time).
As currently written, you'll overwrite the "old" section address the second time through.

Cheers,
Lang.

[lhames]

Hi Keno,

I think all you need, in updateSectionAddress, is a guard along the lines of:

if (OldSectionAddrList[Sec.getRawDataRefImpl().d.a] == 0)
  OldSectionAddrList[Sec.getRawDataRefImpl().d.a] = oldAddr;

That way the original section address will be preserved (so internal relocations will still be computed correctly), but successive moves should also update the symbol and section addresses in the object image. Does that sound right to you?

Cheers,
Lang.

[loladiro]

Yes, that's right. I was busy yesterday, but I was actually just rebuilding now with almost that exact patch (I made it an assertion failure to call the function twice with different addresses though).

[loladiro]

Addressed the comments.

[lhames]

Is it a problem if updateSectionAddress is called more than once with different addresses? I agree it's odd, but as long as the sections aren't finalized in between I think it's ok?

The other thing that occurred to me is that getOldSectionAddr will return '0' if the section hasn't been moved, which isn't right. I think that OldSectionAddrList should be initialized with the initial section addr when the MachOObjectImage is constructed. The updateSectionAddress method shouldn't need to touch OldSectionAddrList at all.

These two issues are connected: Since we need to populate OldSectionAddrList up-front, we can't use it to test whether we've moved a section before, so the test guarding the assertion won't work. If you want to disallow moving sections more than once you'll need to keep an "alreadyMoved" set too.

[loladiro]

Yeah, I guess it's ok to move a section again. Populating the list up-front sounds like a good idea to me.

[loladiro]

Now initializing the Array up-front.

[loladiro]

Anything else I should do here?

[echristo]

Format it with clang-format :)

[loladiro]

Small fix and formatting.

[echristo]

Missed lib/Object/MachOObjectFile.cpp :)

-eric

[loladiro]

I ran it through clang-format-diff.py. The files themselves are not entirely formatted acording to clang-format, so I didn't want to introduce a large set of unrelated changes. Not sure why that wasn't reformatted by clang-format-diff.py.

[echristo]

Definitely a bug in clang-format-diff since the opening brace should be on the line before.

[loladiro]

One more small reformat.

[echristo]

LGTM, I'll leave it up to Lang to review some of the grotty mach-o
details I've likely forgotten :)

-eric

[echristo]

One more nit since I noticed:

+ if (is64) {
+ ((MachO::section_64 *)data)->addr = Addr;
+ } else {
+ ((MachO::section *)data)->addr = Addr;
+ }

No need for the braces here.

I'm kindof dubious about this whole approach, but this matches what
the ELF stuff is doing so...

-eric

[lhames]

LGTM too.

There's a serious caveat attached to this, which is that I think the idea of re-writing object files in-place is fundamentally broken. This will probably work for MCJIT because it emits its object files directly into memory. For RuntimeDyld, which may memmap its object files, this almost certainly won't work. We should aim to have a nicer interface between the JIT and the debugger. This patch doesn't make things any worse than they already were though, so I don't want to hold it up.

Thanks again for working on this Keno!

[loladiro]

Thanks, Lang. Note that I don't have commit access, so somebody else will have to commit (I can fix the one remaining style nit in a moment). I agree that we should think about a different interface to the debugger. For now I'm happy that this works though. Maybe the mailing list would be a better place to discuss what such an interface would look like.

[lhames]

Committed in r211652. Thanks Keno!

include/llvm/Object/MachO.h

static StringRef guessLibraryShortName(StringRef Name, bool &isFramework,
                                          StringRef &Suffix);
 
   static Triple::ArchType getArch(uint32_t CPUType);
 
   static bool classof(const Binary *v) {
     return v->isMachO();
   }
 
+  const char *getSectionPointer(DataRefImpl Rel) const;
+
 private:
-  typedef SmallVector<const char*, 1> SectionList;
+  typedef SmallVector<const char *, 1> SectionList;
   SectionList Sections;
-  typedef SmallVector<const char*, 1> LibraryList;
+  typedef SmallVector<const char *, 1> LibraryList;
   LibraryList Libraries;
   typedef SmallVector<StringRef, 1> LibraryShortName;
   LibraryShortName LibrariesShortNames;
   const char *SymtabLoadCmd;
   const char *DysymtabLoadCmd;
   const char *DataInCodeLoadCmd;
 };
 

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h

processRelocationRef(unsigned SectionID, relocation_iterator RelI,
                        ObjectImage &Obj, ObjSectionToIDMap &ObjSectionToID,
                        const SymbolTableMap &Symbols, StubMap &Stubs) override;
   bool isCompatibleFormat(const ObjectBuffer *Buffer) const override;
   bool isCompatibleFile(const object::ObjectFile *Obj) const override;
   void registerEHFrames() override;
   void finalizeLoad(ObjectImage &ObjImg,
                     ObjSectionToIDMap &SectionMap) override;
 
-  static ObjectImage *createObjectImage(ObjectBuffer *InputBuffer) {
-    return new ObjectImageCommon(InputBuffer);
-  }
-
+  static ObjectImage *createObjectImage(ObjectBuffer *Buffer);
   static ObjectImage *
-  createObjectImageFromFile(std::unique_ptr<object::ObjectFile> InputObject) {
-    return new ObjectImageCommon(std::move(InputObject));
-  }
+  createObjectImageFromFile(std::unique_ptr<object::ObjectFile> InputObject);
 };
 
 } // end namespace llvm
 
 #endif

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.cpp

 //===-- RuntimeDyldMachO.cpp - Run-time dynamic linker for MC-JIT -*- C++ -*-=//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Implementation of the MC-JIT runtime dynamic linker.
 //
 //===----------------------------------------------------------------------===//
 
 #include "RuntimeDyldMachO.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "ObjectImageCommon.h"
+#include "JITRegistrar.h"
 using namespace llvm;
 using namespace llvm::object;
 
 #define DEBUG_TYPE "dyld"
 
 namespace llvm {
 
+class MachOObjectImage : public ObjectImageCommon {
+private:
+  typedef SmallVector<uint64_t, 1> SectionAddrList;
+  SectionAddrList OldSectionAddrList;
+
+protected:
+  bool is64;
+  bool Registered;
+
+private:
+  void initOldAddress() {
+    MachOObjectFile *objf = static_cast<MachOObjectFile *>(ObjFile.get());
+    // Unfortunately we need to do this, since there's information encoded
+    // in the original addr of the section that we could not otherwise
+    // recover. The reason for this is that symbols do not actually store
+    // their file offset, but only their vmaddr. This means that in order
+    // to locate the symbol correctly in the object file, we need to know
+    // where the original start of the section was (including any padding,
+    // etc).
+    for (section_iterator i = objf->section_begin(), e = objf->section_end();
+         i != e; ++i) {
+      uint64_t Addr;
+      i->getAddress(Addr);
+      OldSectionAddrList[i->getRawDataRefImpl().d.a] = Addr;
+    }
+  }
+
+public:
+  MachOObjectImage(ObjectBuffer *Input, bool is64)
+      : ObjectImageCommon(Input),
+        OldSectionAddrList(ObjFile->section_end()->getRawDataRefImpl().d.a, 0),
+        is64(is64), Registered(false) {
+    initOldAddress();
+  }
+
+  MachOObjectImage(std::unique_ptr<object::ObjectFile> Input, bool is64)
+      : ObjectImageCommon(std::move(Input)),
+        OldSectionAddrList(ObjFile->section_end()->getRawDataRefImpl().d.a, 0),
+        is64(is64), Registered(false) {
+    initOldAddress();
+  }
+
+  virtual ~MachOObjectImage() {}
+
+  // Subclasses can override these methods to update the image with loaded
+  // addresses for sections and common symbols
+  virtual void updateSectionAddress(const SectionRef &Sec, uint64_t Addr) {
+    MachOObjectFile *objf = static_cast<MachOObjectFile *>(ObjFile.get());
+    char *data =
+        const_cast<char *>(objf->getSectionPointer(Sec.getRawDataRefImpl()));
+
+    uint64_t oldAddr = OldSectionAddrList[Sec.getRawDataRefImpl().d.a];
+
+    if (is64) {
+      ((MachO::section_64 *)data)->addr = Addr;
+    } else {
+      ((MachO::section *)data)->addr = Addr;
+    }
+
+    for (symbol_iterator i = objf->symbol_begin(), e = objf->symbol_end();
+         i != e; ++i) {
+      section_iterator symSec(objf->section_end());
+      (*i).getSection(symSec);
+      if (*symSec == Sec) {
+        uint64_t symAddr;
+        (*i).getAddress(symAddr);
+        updateSymbolAddress(*i, symAddr + Addr - oldAddr);
+      }
+    }
+  }
+
+  uint64_t getOldSectionAddr(const SectionRef &Sec) const {
+    return OldSectionAddrList[Sec.getRawDataRefImpl().d.a];
+  }
+
+  virtual void updateSymbolAddress(const SymbolRef &Sym, uint64_t Addr) {
+    char *data = const_cast<char *>(
+        reinterpret_cast<const char *>(Sym.getRawDataRefImpl().p));
+    if (is64)
+      ((MachO::nlist_64 *)data)->n_value = Addr;
+    else
+      ((MachO::nlist *)data)->n_value = Addr;
+  }
+
+  virtual void registerWithDebugger() {
+    JITRegistrar::getGDBRegistrar().registerObject(*Buffer);
+    Registered = true;
+  }
+
+  virtual void deregisterWithDebugger() {
+    JITRegistrar::getGDBRegistrar().deregisterObject(*Buffer);
+  }
+};
+
+ObjectImage *RuntimeDyldMachO::createObjectImage(ObjectBuffer *Buffer) {
+  uint32_t magic = *((uint32_t *)Buffer->getBufferStart());
+  bool is64 = (magic == MachO::MH_MAGIC_64);
+  assert((magic == MachO::MH_MAGIC_64 || magic == MachO::MH_MAGIC) &&
+         "Unrecognized Macho Magic");
+  return new MachOObjectImage(Buffer, is64);
+}
+
+ObjectImage *RuntimeDyldMachO::createObjectImageFromFile(
+    std::unique_ptr<object::ObjectFile> ObjFile) {
+  if (!ObjFile)
+    return nullptr;
+
+  MemoryBuffer *Buffer =
+      MemoryBuffer::getMemBuffer(ObjFile->getData(), "", false);
+
+  uint32_t magic = *((uint32_t *)Buffer->getBufferStart());
+  bool is64 = (magic == MachO::MH_MAGIC_64);
+  assert((magic == MachO::MH_MAGIC_64 || magic == MachO::MH_MAGIC) &&
+         "Unrecognized Macho Magic");
+  return new MachOObjectImage(std::move(ObjFile), is64);
+}
+
 static unsigned char *processFDE(unsigned char *P, intptr_t DeltaForText,
                                  intptr_t DeltaForEH) {
   DEBUG(dbgs() << "Processing FDE: Delta for text: " << DeltaForText
                << ", Delta for EH: " << DeltaForEH << "\n");
   uint32_t Length = *((uint32_t *)P);
   P += 4;
   unsigned char *Ret = P + Length;
   uint32_t Offset = *((uint32_t *)P);
relocation_iterator RuntimeDyldMachO::processI386ScatteredVANILLA(
   return ++RelI;
 }
 
 relocation_iterator RuntimeDyldMachO::processRelocationRef(
     unsigned SectionID, relocation_iterator RelI, ObjectImage &Obj,
     ObjSectionToIDMap &ObjSectionToID, const SymbolTableMap &Symbols,
     StubMap &Stubs) {
   const ObjectFile *OF = Obj.getObjectFile();
+  const MachOObjectImage &MachOObj = *static_cast<MachOObjectImage *>(&Obj);
   const MachOObjectFile *MachO = static_cast<const MachOObjectFile *>(OF);
   MachO::any_relocation_info RE =
       MachO->getRelocation(RelI->getRawDataRefImpl());
 
   uint32_t RelType = MachO->getAnyRelocationType(RE);
 
   // FIXME: Properly handle scattered relocations.
   //        Special case the couple of scattered relocations that we know how
if (Arch == Triple::x86 && IsPCRel) {
       Value.Addend += RelocAddr + 4;
     }
 
   } else {
     SectionRef Sec = MachO->getRelocationSection(RE);
     bool IsCode = false;
     Sec.isText(IsCode);
     Value.SectionID = findOrEmitSection(Obj, Sec, IsCode, ObjSectionToID);
-    uint64_t Addr;
-    Sec.getAddress(Addr);
+    uint64_t Addr = MachOObj.getOldSectionAddr(Sec);
+    DEBUG(dbgs() << "\nAddr: " << Addr << "\nAddend: " << Addend);
     Value.Addend = Addend - Addr;
     if (IsPCRel)
       Value.Addend += Offset + NumBytes;
   }
 
   if (Arch == Triple::x86_64 && (RelType == MachO::X86_64_RELOC_GOT ||
                                  RelType == MachO::X86_64_RELOC_GOT_LOAD)) {
     assert(IsPCRel);

lib/Object/MachOObjectFile.cpp

 
 StringRef MachOObjectFile::getStringTableData() const {
   MachO::symtab_command S = getSymtabLoadCommand();
   return getData().substr(S.stroff, S.strsize);
 }
 
 bool MachOObjectFile::is64Bit() const {
   return getType() == getMachOType(false, true) ||
          getType() == getMachOType(true, true);
 }
 
 void MachOObjectFile::ReadULEB128s(uint64_t Index,
                                    SmallVectorImpl<uint64_t> &Out) const {
   DataExtractor extractor(ObjectFile::getData(), true, 0);
 
   uint32_t offset = Index;
   uint64_t data = 0;
   while (uint64_t delta = extractor.getULEB128(&offset)) {
     data += delta;
     Out.push_back(data);
   }
 }
 
+const char *MachOObjectFile::getSectionPointer(DataRefImpl Rel) const {
+  return Sections[Rel.d.a];
+}
+
 ErrorOr<ObjectFile *> ObjectFile::createMachOObjectFile(MemoryBuffer *Buffer,
                                                         bool BufferOwned) {
   StringRef Magic = Buffer->getBuffer().slice(0, 4);
   std::error_code EC;
   std::unique_ptr<MachOObjectFile> Ret;
   if (Magic == "\xFE\xED\xFA\xCE")
     Ret.reset(new MachOObjectFile(Buffer, false, false, EC, BufferOwned));
   else if (Magic == "\xCE\xFA\xED\xFE")