[all-commits] [llvm/llvm-project] 1752f2: [lld-macho][nfc] Remove `MachO::` prefix where pos...

Thu Mar 11 10:28:52 PST 2021

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1752f28506859220d893aa9303b51f386f2e6995
      https://github.com/llvm/llvm-project/commit/1752f28506859220d893aa9303b51f386f2e6995
  Author: Jez Ng <jezng at fb.com>
  Date:   2021-03-11 (Thu, 11 Mar 2021)

  Changed paths:
    M lld/MachO/Driver.cpp
    M lld/MachO/DriverUtils.cpp
    M lld/MachO/InputFiles.cpp
    M lld/MachO/InputFiles.h
    M lld/MachO/InputSection.cpp
    M lld/MachO/MergedOutputSection.cpp
    M lld/MachO/SyntheticSections.cpp
    M lld/MachO/Writer.cpp

  Log Message:
  -----------
  [lld-macho][nfc] Remove `MachO::` prefix where possible

Previously, SyntheticSections.cpp did not have a top-level `using namespace
llvm::MachO` because it caused a naming conflict: `llvm::MachO::Symbol` would
collide with `lld::macho::Symbol`.

`MachO::Symbol` represents the symbols defined in InterfaceFiles (TBDs). By
moving the inclusion of InterfaceFile.h into our .cpp files, we can avoid this
name collision in other files where we are only dealing with LLD's own symbols.

Along the way, I removed all unnecessary "MachO::" prefixes in our code.

Cons of this approach: If TextAPI/MachO/Symbol.h gets included via some other
header file in the future, we could run into this collision again.

Alternative 1: Have either TextAPI/MachO or BinaryFormat/MachO.h use a different
namespace. Most of the benefit of `using namespace llvm::MachO` comes from being
able to use things in BinaryFormat/MachO.h conveniently; if TextAPI was under a
different (and fully-qualified) namespace like `llvm::tapi` that would solve our
problems. Cons: lots of files across llvm-project will need to be updated, and
folks who own the TextAPI code need to agree to the name change.

Alternative 2: Rename our Symbol to something like `LldSymbol`. I think this is
ugly.

Personally I think alternative #1 is ideal, but I'm not sure the effort to do it is
worthwhile, this diff's halfway solution seems good enough to me. Thoughts?

Reviewed By: #lld-macho, oontvoo, MaskRay

Differential Revision: https://reviews.llvm.org/D98149

  Commit: 5433a79176a3dac3158c6db792a5938bdc24b905
      https://github.com/llvm/llvm-project/commit/5433a79176a3dac3158c6db792a5938bdc24b905
  Author: Jez Ng <jezng at fb.com>
  Date:   2021-03-11 (Thu, 11 Mar 2021)

  Changed paths:
    M lld/MachO/Arch/ARM64.cpp
    M lld/MachO/Arch/X86_64.cpp
    M lld/MachO/CMakeLists.txt
    M lld/MachO/InputFiles.cpp
    M lld/MachO/InputSection.cpp
    M lld/MachO/InputSection.h
    A lld/MachO/Relocations.cpp
    A lld/MachO/Relocations.h
    M lld/MachO/Target.cpp
    M lld/MachO/Target.h
    M lld/MachO/Writer.cpp

  Log Message:
  -----------
  [lld-macho][nfc] Create Relocations.{h,cpp} for relocation-specific code

This more closely mirrors the structure of lld-ELF.

Reviewed By: #lld-macho, thakis

Differential Revision: https://reviews.llvm.org/D98384

  Commit: e8a30583033596b80d51dc89ad166526b5446d88
      https://github.com/llvm/llvm-project/commit/e8a30583033596b80d51dc89ad166526b5446d88
  Author: Jez Ng <jezng at fb.com>
  Date:   2021-03-11 (Thu, 11 Mar 2021)

  Changed paths:
    M lld/MachO/Arch/X86_64.cpp
    M lld/MachO/InputFiles.cpp
    M lld/MachO/Target.h
    M lld/test/MachO/x86-64-reloc-signed.s

  Log Message:
  -----------
  [lld-macho] Fix handling of X86_64_RELOC_SIGNED_{1,2,4}

The previous implementation miscalculated the addend, resulting
in an underflow. This meant that every SIGNED_N section relocation would
be associated with the last subsection (since the addend would now be a
huge number). We were "lucky" that this mistake was typically cancelled
out -- 64-to-32-bit-truncation meant that the final value was correct,
as long as subsections were not rearranged.

Reviewed By: #lld-macho, thakis

Differential Revision: https://reviews.llvm.org/D98385

  Commit: a723db92d87d098d564ac8ace8b1d24bb02a9fdb
      https://github.com/llvm/llvm-project/commit/a723db92d87d098d564ac8ace8b1d24bb02a9fdb
  Author: Jez Ng <jezng at fb.com>
  Date:   2021-03-11 (Thu, 11 Mar 2021)

  Changed paths:
    M lld/MachO/Arch/ARM64.cpp
    M lld/MachO/InputFiles.cpp
    M lld/MachO/InputSection.cpp
    M lld/test/MachO/reloc-subtractor.s

  Log Message:
  -----------
  [lld-macho][nfc] Refactor subtractor reloc handling

SUBTRACTOR relocations are always paired with UNSIGNED
relocations to indicate a pair of symbols whose address difference we
want. Functionally they are like a single relocation: only one pointer
gets written / relocated. Previously, we would handle these pairs by
skipping over the SUBTRACTOR relocation and writing the pointer when
handling the UNSIGNED reloc. This diff reverses things, so we write
while handling SUBTRACTORs and skip over the UNSIGNED relocs instead.

Being able to distinguish between SUBTRACTOR and UNSIGNED relocs in the
write phase (i.e. inside `relocateOne`) is useful for the upcoming range
check diff: we want to check that SUBTRACTOR relocs write signed values,
but UNSIGNED relocs (naturally) write unsigned values.

Reviewed By: #lld-macho, thakis

Differential Revision: https://reviews.llvm.org/D98386

  Commit: d1e57ee99aa815e51bf2541fa4d3e965d7f4b8de
      https://github.com/llvm/llvm-project/commit/d1e57ee99aa815e51bf2541fa4d3e965d7f4b8de
  Author: Jez Ng <jezng at fb.com>
  Date:   2021-03-11 (Thu, 11 Mar 2021)

  Changed paths:
    M lld/test/MachO/arm64-relocs.s
    M lld/test/MachO/dylink-lazy.s
    M lld/test/MachO/function-starts.s
    M lld/test/MachO/segments.s

  Log Message:
  -----------
  [lld-macho] Avoid requiring shell in tests

There are 3 remaining tests that still have `REQUIRE: shell`:
* color-diagnostics.test -- seems necessary for ANSI escape sequence support
* stabs.s -- the shell part could be removed, but I don't think we can support
  the test on Windows anyway due to its reliance on `touch` to set the modtime
* framework.s -- uses symlinks, I'm not sure this works on Windows

Addresses PR49512.

Reviewed By: #lld-macho, alexshap

Differential Revision: https://reviews.llvm.org/D98395

Compare: https://github.com/llvm/llvm-project/compare/8ba05e14897e...d1e57ee99aa8