[lld] [llvm] [lld-macho] Clean up chained fixup entry emission (NFC) (PR #149483)

Daniel Rodríguez Troitiño via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 21 10:03:23 PDT 2025


drodriguez wrote:

> LLD uses manual packing/unpacking for other bitfields exclusively

Do you mean the whole LLD or LLD for Mach-O?

(also, that something is done all over the place doesn't mean it is the best thing to keep doing, specially when a really thin abstraction is possible and (IMO) improves the usability of the code).

> have a single place where the offsets are defined

This was my main concern. The usages seems to be limited to those couple of places, which was why I was proposing changing it at once now, but I understand that the proposal makes it look like nothing is going on, but there's some code going on under the hood (which probably need some docstrings, honestly). One thing with your changes is that `dyld_chained_ptr_64_bind/rebase` is still left there as a free-for-all footgun.

One compromise is removing most of the current struct, still use `Bitfields.h` to create all those masks and offsets. You don't have to use `Bitfield::set` and `Bitfield::get` if you don't want (but it will make things easier).

```
struct dyld_chained_ptr_64_bind {
  using Ordinal = llvm::Bitfield::Element<uint64_t, 0, 24>;
  using Addend = llvm::Bitfield::Element<uint64_t, Ordinal::NextBit, 8>;
  using Reserved = llvm::Bitfield::Element<uint64_t, Addend::NextBit, 19>;
  using Next = llvm::Bitfield::Element<uint64_t, Reserved::NextBit, 12>;
  using Bind = llvm::Bitfield::Element<uint64_t, Next::NextBit, 1>;  // set to 1
};
```

Allows you to write:

```
  if (!isUInt<dyld_chained_ptr_64_bind::Ordinal::Bits>(ordinal))
    error("Import ordinal for symbol '" + sym->getName() + "' (" +
          utohexstr(ordinal) +
          ") is larger than maximum import count (" + utohexstr(BitPatterns<uint64_t, dyld_chained_ptr_64_bind::Ordinal::Bits>::Umax) + ". Re-link with "
          "-no_fixup_chains");

  ordinal &= BitPatterns<uint64_t, dyld_chained_ptr_64_bind::Ordinal::Bits>::Umax << dyld_chained_ptr_64_bind::Ordinal::Offset;

  switch (in.chainedFixups->pointerFormat()) {
  case DYLD_CHAINED_PTR_64: {
    assert(isUInt<dyld_chained_ptr_64_bind::Addend::Bits>(inlineAddend) &&
           "large addend should be stored out-of-line");

    write64le(buf, ordinal | (inlineAddend << dyld_chained_ptr_64_bind::Addend::Shift) | (1ULL << dyld_chained_ptr_64_bind::Bind::Shift));
    break;
  }
  default:
    llvm_unreachable("unsupported chained fixup pointer format");
  }
```

But you can merge as-is, if you prefer. It might become very difficult to change this after more and more changes are introduced, though, so I wanted to get this early feedback out.


https://github.com/llvm/llvm-project/pull/149483


More information about the llvm-commits mailing list