Move GOTPass and StubsPass from Core to MachO

[kledzik]

Summary

When I originally wrote the GOT and Stubs Passes I had hoped
they were generic enough to be usable by all platforms. But ELF
is using private Passes to create PLT entries and I don't think
COFF needs these Passes. I'd plan to change the interface to these
Passes to make it easier to add architectures in mach-o. As
a first step, I'd like to move these Passes out of "Core" and make
them mach-o specific.

[ruiu]

I support this.

Instead of deleting test files (you removed two files), can you move them to the mach-o directory?

[kledzik]

Those tests need the glue code also deleted from CoreLinkingContext.cpp. That glue code identifies call sites, etc and specific to the flavor. I'll be adding new mach-o specific test cases for the passes as part of the refactoring I'm planning.

This came about because I was adding ARM support to mach-o and realized I would need to copy and paste lots of code from x86[_64] mach-o support. I'd rather refactor it so there is less copying needed.

[ruiu]

LGTM

[kledzik]

committed in r212652

include/lld/Core/Pass.h

 #include "lld/Core/Atom.h"
 #include "lld/Core/File.h"
 #include "lld/Core/range.h"
 #include "lld/Core/Reference.h"
 
 #include <vector>
 
 namespace lld {
-class DefinedAtom;
 class MutableFile;
 
 /// Once the core linking is done (which resolves references, coalesces atoms
 /// and produces a complete Atom graph), the linker runs a series of passes
 /// on the Atom graph. The graph is modeled as a File, which means the pass
 /// has access to all the atoms and to File level attributes. Each pass does
 /// a particular transformation to the Atom graph or to the File attributes.
 ///
 /// This is the abstract base class for all passes.  A Pass does its
 /// actual work in it perform() method.  It can iterator over Atoms in the
 /// graph using the *begin()/*end() atom iterator of the File.  It can add
 /// new Atoms to the graph using the File's addAtom() method.
 class Pass {
 public:
   virtual ~Pass() { }
 
   /// Do the actual work of the Pass.
   virtual void perform(std::unique_ptr<MutableFile> &mergedFile) = 0;
 
 protected:
   // Only subclassess can be instantiated.
   Pass() { }
 };
 
-/// Pass for adding stubs (PLT entries) for calls to functions
-/// outside the linkage unit.  This class is subclassed by each
-/// file format Writer which implements the pure virtual methods.
-class StubsPass : public Pass {
-public:
-  StubsPass() : Pass() {}
-
-  /// Scans all Atoms looking for call-site uses of SharedLibraryAtoms
-  /// and transfroms the call-site to call a stub instead using the
-  /// helper methods below.
-  void perform(std::unique_ptr<MutableFile> &mergedFile) override;
-
-  /// If true, the pass should use stubs for references
-  /// to shared library symbols. If false, the pass
-  /// will generate relocations on the text segment which the
-  /// runtime loader will use to patch the program at runtime.
-  virtual bool noTextRelocs() = 0;
-
-  /// Returns whether the Reference kind is for a call site.  The pass
-  /// uses this to find calls that need to be indirected through a stub.
-  virtual bool isCallSite(const Reference &) = 0;
-
-  /// Returns a file format specific atom for a stub/PLT entry which contains
-  /// instructions which jump to the specified atom.  May be called multiple
-  /// times for the same target atom, in which case this method should return
-  /// the same stub atom.
-  virtual const DefinedAtom *getStub(const Atom &target) = 0;
-
-  /// After the default implementation of perform() is done calling getStub(),
-  /// it will call this method to add all the stub (and support) atoms to the
-  /// master file object.
-  virtual void addStubAtoms(MutableFile &masterFile) = 0;
-
-private:
-  void replaceCalleeWithStub(const Atom *target, const Reference *ref);
-};
-
-/// Pass for adding GOT entries for pointers to functions/data
-/// outside the linkage unit. This class is subclassed by each
-/// file format Writer which implements the pure virtual methods.
-class GOTPass : public Pass {
-public:
-  GOTPass() : Pass() {}
-
-  /// Scans all Atoms looking for pointer to SharedLibraryAtoms
-  /// and transfroms them to a pointer to a GOT entry using the
-  /// helper methods below.
-  void perform(std::unique_ptr<MutableFile> &mergedFile) override;
-
-  /// If true, the pass will use GOT entries for references
-  /// to shared library symbols. If false, the pass
-  /// will generate relocations on the text segment which the
-  /// runtime loader will use to patch the program at runtime.
-  virtual bool noTextRelocs() = 0;
-
-  /// Returns whether the Reference kind is a pre-instantiated GOT access.
-  /// The default implementation of perform() uses this to figure out
-  /// what GOT entries to instantiate.
-  virtual bool isGOTAccess(const Reference &, bool &canBypassGOT) = 0;
-
-  /// The file format Writer needs to alter the reference kind from a
-  /// pre-instantiated GOT access to an actual access.  If targetIsNowGOT is
-  /// true, the pass has instantiated a GOT atom and altered the reference's
-  /// target to point to that atom.  If targetIsNowGOT is false, the pass
-  /// determined a GOT entry is not needed because the reference site can
-  /// directly access the target.
-  virtual void updateReferenceToGOT(const Reference*, bool targetIsNowGOT) = 0;
-
-  /// Returns a file format specific atom for a GOT entry targeting
-  /// the specified atom.
-  virtual const DefinedAtom *makeGOTEntry(const Atom &target) = 0;
-};
-
 } // namespace lld
 
 #endif // LLD_CORE_PASS_H

lib/Passes/CMakeLists.txt

 add_lld_library(lldPasses
-  GOTPass.cpp
-  StubsPass.cpp
   LayoutPass.cpp
   RoundTripNativePass.cpp
   RoundTripYAMLPass.cpp
   )
 
 target_link_libraries(lldPasses lldReaderWriter)

lib/ReaderWriter/CoreLinkingContext.cpp

 private:
   atom_collection_vector<DefinedAtom> _definedAtoms;
   atom_collection_vector<UndefinedAtom> _undefinedAtoms;
   atom_collection_vector<SharedLibraryAtom> _sharedLibraryAtoms;
   atom_collection_vector<AbsoluteAtom> _absoluteAtoms;
 };
 
 
-class TestingStubsPass : public StubsPass {
-public:
-  TestingStubsPass(const LinkingContext &ctx) : _file(ctx) {}
-
-  bool noTextRelocs() override { return true; }
-
-  bool isCallSite(const Reference &ref) override {
-    if (ref.kindNamespace() != Reference::KindNamespace::testing)
-      return false;
-    return (ref.kindValue() == CoreLinkingContext::TEST_RELOC_CALL32);
-  }
-
-  const DefinedAtom *getStub(const Atom &target) override {
-    const DefinedAtom *result = new TestingStubAtom(_file, target);
-    _file.addAtom(*result);
-    return result;
-  }
-
-  void addStubAtoms(MutableFile &mergedFile) override {
-    for (const DefinedAtom *stub : _file.defined()) {
-      mergedFile.addAtom(*stub);
-    }
-  }
-
-private:
-  TestingPassFile _file;
-};
-
-class TestingGOTPass : public GOTPass {
-public:
-  TestingGOTPass(const LinkingContext &ctx) : _file(ctx) {}
-
-  bool noTextRelocs() override { return true; }
-
-  bool isGOTAccess(const Reference &ref, bool &canBypassGOT) override {
-    if (ref.kindNamespace() != Reference::KindNamespace::testing)
-      return false;
-    switch (ref.kindValue()) {
-    case CoreLinkingContext::TEST_RELOC_GOT_LOAD32:
-      canBypassGOT = true;
-      return true;
-    case CoreLinkingContext::TEST_RELOC_GOT_USE32:
-      canBypassGOT = false;
-      return true;
-    }
-    return false;
-  }
-
-  void updateReferenceToGOT(const Reference *ref, bool targetIsNowGOT) override {
-    const_cast<Reference *>(ref)->setKindValue(
-        targetIsNowGOT ? CoreLinkingContext::TEST_RELOC_PCREL32
-                       : CoreLinkingContext::TEST_RELOC_LEA32_WAS_GOT);
-  }
-
-  const DefinedAtom *makeGOTEntry(const Atom &target) override {
-    return new TestingGOTAtom(_file, target);
-  }
-
-private:
-  TestingPassFile _file;
-};
 
 } // anonymous namespace
 
 CoreLinkingContext::CoreLinkingContext() {}
 
 bool CoreLinkingContext::validateImpl(raw_ostream &) {
   _writer = createWriterYAML(*this);
   return true;
 }
 
 void CoreLinkingContext::addPasses(PassManager &pm) {
   for (StringRef name : _passNames) {
     if (name.equals("layout"))
       pm.add(std::unique_ptr<Pass>(new LayoutPass(registry())));
-    else if (name.equals("GOT"))
-      pm.add(std::unique_ptr<Pass>(new TestingGOTPass(*this)));
-    else if (name.equals("stubs"))
-      pm.add(std::unique_ptr<Pass>(new TestingStubsPass(*this)));
     else
       llvm_unreachable("bad pass name");
   }
 }
 
 Writer &CoreLinkingContext::writer() const { return *_writer; }

lib/ReaderWriter/MachO/CMakeLists.txt

 add_lld_library(lldMachO
+  GOTPass.cpp
   MachOLinkingContext.cpp
   MachONormalizedFileBinaryReader.cpp
   MachONormalizedFileBinaryWriter.cpp
   MachONormalizedFileFromAtoms.cpp
   MachONormalizedFileToAtoms.cpp
   MachONormalizedFileYAML.cpp
   ReferenceKinds.cpp
+  StubsPass.cpp
   WriterMachO.cpp
   )
 
 target_link_libraries(lldMachO
   lldReaderWriter
   lldPasses
   )
 
 include_directories(.)

lib/ReaderWriter/MachO/GOTPass.hpp

 #define LLD_READER_WRITER_MACHO_GOT_PASS_H
 
 #include "lld/Core/DefinedAtom.h"
 #include "lld/Core/SharedLibraryAtom.h"
 #include "lld/Core/File.h"
 #include "lld/Core/Reference.h"
 #include "lld/Core/Pass.h"
 
+#include "MachOPasses.h"
 #include "ReferenceKinds.h"
 #include "StubAtoms.hpp"
 
 namespace lld {
 namespace mach_o {
 
 
 class GOTPass : public lld::GOTPass {

lib/ReaderWriter/MachO/GOTPass.cpp

-//===- Passes/GOTPass.cpp - Adds GOT entries ------------------------------===//
+//===- lib/ReaderWriter/MachO/GOTPass.cpp ---------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///

 /// an LEA instruction that is the same size.  The method isGOTAccess() should
 /// only return true for "canBypassGOT" if this optimization is supported.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "lld/Core/DefinedAtom.h"
 #include "lld/Core/File.h"
 #include "lld/Core/LLVM.h"
-#include "lld/Core/Pass.h"
 #include "lld/Core/Reference.h"
 #include "llvm/ADT/DenseMap.h"
 
+#include "MachOPasses.h"
+
 namespace lld {
 
 static bool shouldReplaceTargetWithGOTAtom(const Atom *target,
                                            bool canBypassGOT) {
   // Accesses to shared library symbols must go through GOT.
   if (target->definition() == Atom::definitionSharedLibrary)
     return true;
   // Accesses to interposable symbols in same linkage unit must also go

lib/ReaderWriter/MachO/MachOPasses.h

-//===------ Core/Pass.h - Base class for linker passes --------------------===//
+//===- lib/ReaderWriter/MachO/MachOPasses.h -------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLD_CORE_PASS_H
-#define LLD_CORE_PASS_H
+#ifndef LLD_READER_WRITER_MACHO_PASSES_H
+#define LLD_READER_WRITER_MACHO_PASSES_H
 
 #include "lld/Core/Atom.h"
 #include "lld/Core/File.h"
+#include "lld/Core/Pass.h"
 #include "lld/Core/range.h"
 #include "lld/Core/Reference.h"
 
 #include <vector>
 
 namespace lld {
 class DefinedAtom;
 class MutableFile;
 
-/// Once the core linking is done (which resolves references, coalesces atoms
-/// and produces a complete Atom graph), the linker runs a series of passes
-/// on the Atom graph. The graph is modeled as a File, which means the pass
-/// has access to all the atoms and to File level attributes. Each pass does
-/// a particular transformation to the Atom graph or to the File attributes.
-///
-/// This is the abstract base class for all passes.  A Pass does its
-/// actual work in it perform() method.  It can iterator over Atoms in the
-/// graph using the *begin()/*end() atom iterator of the File.  It can add
-/// new Atoms to the graph using the File's addAtom() method.
-class Pass {
-public:
-  virtual ~Pass() { }
-
-  /// Do the actual work of the Pass.
-  virtual void perform(std::unique_ptr<MutableFile> &mergedFile) = 0;
-
-protected:
-  // Only subclassess can be instantiated.
-  Pass() { }
-};
 
 /// Pass for adding stubs (PLT entries) for calls to functions
 /// outside the linkage unit.  This class is subclassed by each
 /// file format Writer which implements the pure virtual methods.
 class StubsPass : public Pass {
 public:
   StubsPass() : Pass() {}
 
public:
 
   /// Returns a file format specific atom for a GOT entry targeting
   /// the specified atom.
   virtual const DefinedAtom *makeGOTEntry(const Atom &target) = 0;
 };
 
 } // namespace lld
 
-#endif // LLD_CORE_PASS_H
+#endif // LLD_READER_WRITER_MACHO_PASSES_H

lib/ReaderWriter/MachO/StubsPass.hpp

 
 #include "lld/Core/DefinedAtom.h"
 #include "lld/Core/File.h"
 #include "lld/Core/Pass.h"
 #include "lld/Core/Reference.h"
 #include "lld/Core/SharedLibraryAtom.h"
 #include "lld/Core/Simple.h"
 
+#include "MachOPasses.h"
 #include "ReferenceKinds.h"
 #include "StubAtoms.hpp"
 
 namespace lld {
 namespace mach_o {
 
 
 class StubsPass : public lld::StubsPass {

lib/ReaderWriter/MachO/StubsPass.cpp

-//===- Passes/StubsPass.cpp - Adds stubs ----------------------------------===//
+//===- lib/ReaderWriter/MachO/StubsPass.cpp -------------------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This linker pass updates call-sites which have references to shared library
 // atoms to instead have a reference to a stub (PLT entry) for the specified
 // symbol.  Each file format defines a subclass of StubsPass which implements
 // the abstract methods for creating the file format specific StubAtoms.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lld/Core/DefinedAtom.h"
 #include "lld/Core/File.h"
 #include "lld/Core/LLVM.h"
-#include "lld/Core/Pass.h"
 #include "lld/Core/Reference.h"
 #include "llvm/ADT/DenseMap.h"
 
+#include "MachOPasses.h"
+
 namespace lld {
 
 void StubsPass::perform(std::unique_ptr<MutableFile> &mergedFile) {
   // Skip this pass if output format uses text relocations instead of stubs.
   if (!this->noTextRelocs())
     return;
 
   // Scan all references in all atoms.

test/core/pass-got-basic.objtxt

-# RUN: lld -core %s --add-pass GOT  | FileCheck %s
-
-#
-# Test that GOT pass instantiates GOT entires and alters references
-#
-
----
-defined-atoms:
-    - name:              foo
-      type:              code
-      content:           [ 48, 8B, 0D, 00, 00, 00, 00,
-                           48, 8B, 0D, 00, 00, 00, 00,
-                           48, 8B, 0D, 00, 00, 00, 00,
-                           48, 83, 3D, 00, 00, 00, 00, 00,
-                           48, 83, 3D, 00, 00, 00, 00, 00,
-                           48, 83, 3D, 00, 00, 00, 00, 00,
-                           48, 83, 3D, 00, 00, 00, 00, 00 ]
-      references:
-      - offset:          3
-        kind:            gotLoad32
-        target:          malloc
-      - offset:          10
-        kind:            gotLoad32
-        target:          myPrivate
-      - offset:          17
-        kind:            gotLoad32
-        target:          myInterposable
-      - offset:          24
-        kind:            gotUse32
-        target:          malloc
-      - offset:          32
-        kind:            gotUse32
-        target:          myPrivate
-      - offset:          40
-        kind:            gotUse32
-        target:          myInterposable
-
-    - name:              myPrivate
-      scope:             global
-      interposable:      no
-
-    - name:              myInterposable
-      scope:             global
-      interposable:      yes
-
-shared-library-atoms:
-    - name:              malloc
-      load-name:         libc.so
-
-...
-
-# CHECK:  defined-atoms:
-# CHECK:       name:         foo
-# CHECK:       references:
-# CHECK:        kind:         pcrel32
-# CHECK:        offset:       3
-# CHECK:        target:       L
-# CHECK:        kind:         lea32wasGot
-# CHECK:        offset:       10
-# CHECK:        target:       myPrivate
-# CHECK:        kind:         pcrel32
-# CHECK:        offset:       17
-# CHECK:        target:       L
-# CHECK:        kind:         pcrel32
-# CHECK:        offset:       24
-# CHECK:        target:       L
-# CHECK:        kind:         pcrel32
-# CHECK:        offset:       32
-# CHECK:        target:       L
-# CHECK:        kind:         pcrel32
-# CHECK:        offset:       40
-# CHECK:        target:       L
-# CHECK:       name:         myPrivate
-# CHECK:       name:         myInterposable
-# CHECK:       interposable: yes
-# CHECK:       name:         L
-# CHECK:       type:         got
-# CHECK:       type:         got
-# CHECK:       type:         got
-# CHECK:   shared-library-atoms:
-# CHECK:       name:         malloc
-# CHECK:       ...

test/core/pass-stubs-basic.objtxt

-# RUN: lld -core %s --add-pass stubs  | FileCheck %s
-
-#
-# Test that stubs pass adds stubs and rebinds call sites to the stub
-#
-
----
-defined-atoms:
-    - name:              foo
-      type:              code
-      content:           [ E8, 00, 00, 00, 00, E8, 00, 00, 00,
-                           00, 48 ,8B, 05, 00, 00, 00, 00 ]
-      references:
-      - offset:          1
-        kind:            call32
-        target:          malloc
-      - offset:          6
-        kind:            call32
-        target:          free
-      - offset:          13
-        kind:            gotLoad32
-        target:          malloc
-
-shared-library-atoms:
-    - name:              malloc
-      load-name:         libc.so
-
-    - name:              free
-      load-name:         libc.so
-
-...
-
-# CHECK:       name:         foo
-# CHECK:       references:
-# CHECK:       kind:         call32
-# CHECK:       target:       L
-# CHECK:       kind:         call32
-# CHECK:       target:       L
-# CHECK:       kind:         gotLoad32
-# CHECK:       target:       malloc
-# CHECK:       name:         L
-# CHECK:       type:         stub
-# CHECK:       name:         L
-# CHECK:       type:         stub
-# CHECK:       name:         malloc
-# CHECK:       name:         free
-# CHECK:       ...