[PATCH] D131982: [llvm-objdump] Complete -chained_fixups support

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

We chatted a bit offline. Rough summary

- https://github.com/freebsd/freebsd-src/blob/master/sys/netinet/ip.h#L51-L59 works if the on-disk format is the same for BE and LE, but since `$(xcrun -show-sdk-path)/usr/include/mach-o/fixup-chains.h` doesn't have any ifdefs, that's likely not the case here
- …but all chained fixup apple platforms are little endian, so it's really unknowable
- the current code does work on all hosts for when the on-disk data is little-endian
- it'd be nice to do the swapping anyways, since that puts all the endianness handling in a single place
- Bitfield swapping is different for little-endian-file-on-big-endian-host and the other way round, and the swapStruct() wrappers in BinaryFormat/MachO.h don't know which direction they're swapping in, so there's have to be a dedicated wrapper for each struct. Example for dyld_chained_import:

  // Little-endian file running on big-endian host:
  Raw = ((Raw & 0xff) << 24) | ((Raw & 0x100) << 15) || (Raw >> 9);
  
  // Big-endian file running on little-endian host:
  Raw = (Raw >> 24) | ((Raw & 0x800000) >> 15) || ((Raw & 0x7fffff << 9);



- Since there's a bunch of those structs, it'd be nice to make a general helper for this. This seems to roughly work:

  #include <stdio.h>
  
  template<typename T, unsigned W>
  T bitfield_swap_le(T t) {
    static_assert(W == 0, "");
    return 0;
  }
  
  template<typename T, unsigned W, unsigned First, int... Sizes>
  T bitfield_swap_le(T t) {
    // Peel off rightmost field, add it on left, recurse.
    return ((t & ((1u << First) - 1)) << (W - First)) |
           bitfield_swap_le<T, W - First, Sizes...>(t >> First);
  }
  
  template<typename T, unsigned W>
  T bitfield_swap_be(T t) {
    static_assert(W == 0, "");
    return 0;
  }
  
  template<typename T, unsigned W, unsigned First, int... Sizes>
  T bitfield_swap_be(T t) {
    // Peel off leftmost field, add it on right, recurse.
    return ((t >> (W - First)) |
           (bitfield_swap_be<T, W - First, Sizes...>(t & ((1u << (W - First)) - 1))) << First);
  }
  
  int main() {
    {
      unsigned i = bitfield_swap_le<unsigned, 32, 8, 1, 23>(0xff);
      unsigned j = bitfield_swap_le<unsigned, 32, 8, 1, 23>(0x100);
      unsigned k = bitfield_swap_le<unsigned, 32, 8, 1, 23>(0xffff'fe00);
      printf("%08x %08x %08x\n", i, j, k);
    }
    {
      unsigned i = bitfield_swap_be<unsigned, 32, 8, 1, 23>(0xff00'0000);
      unsigned j = bitfield_swap_be<unsigned, 32, 8, 1, 23>(0x0080'0000);
      unsigned k = bitfield_swap_be<unsigned, 32, 8, 1, 23>(0x007f'ffff);
      printf("%08x %08x %08x\n", i, j, k);
    }
  }
  
  % clang bitfield_swap.cc -std=c++17
  % ./a.out                          
  ff000000 00800000 007fffff
  000000ff 00000100 fffffe00



- Then there could be a `bitfield_swap<>` member template that calls either of those based on file and host bitness, and that'd make it convenient to write a swapStruct() wrapper for each type with a bitfield

- However, all likely host systems are little-endian too, so it also seems fine to just go back to the original code, add a comment, mark the test `REQUIRES: host-byteorder-little-endian` and do the rest later (or have someone who actually needs it do it)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131982/new/

https://reviews.llvm.org/D131982