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

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: llvm/lib/Object/MachOObjectFile.cpp:5004
+
+      LibOrdinal = getEncodedOrdinal<uint8_t>(RawValue[0] & 0xFF);
+      WeakImport = (RawValue[0] >> 8) & 1;
----------------
thakis wrote:
> The in-register view (i.e. not in-memory, we can ignore the byte-swapping of big-endian vs little-endian here) of
> 
> ```
> struct dyld_chained_import {
>   uint32_t lib_ordinal : 8;
>   uint32_t weak_import : 1;
>   uint32_t name_offset : 23;
> };
> ```
> 
> is:
> 
> * In little-endian (bits assigned right-to-left):
>   * most significant 23 bits are name_offset
>   * then 1 bit weak_import
>   * then 8 bit lib_ordinal in the lowest 8 bits
> 
> * In big-endian (bits assigned left-to-right):
>   * 8 bit lib_ordinal in the highest 8 bits
>   * then 1 bit weak_import
>   * least significant 23 bits are name_offset
> 
> So I think even with the manual masking, this currently gets things wrong.
> 
> Also, it's weird that we don't use the dyld_chained_import structs at all. Strange for greppability, using something like http://llvm-cs.pcc.me.uk/ for cross-references, etc.
> 
> What do you think about adding back the Swap<> functions and manually swapping the bitfields around too after swapping the bytes? That assumes sysv abi, but that'll work almost everywhere (can't think of a place where it wouldn't? Maybe windows big endian? Didn't try that, but I wouldn't be surprised if clang-cl didn't work great there either), and if someone needs this to work on some obscure system, they can worry about it then?
Alternatively, something like https://github.com/freebsd/freebsd-src/blob/master/sys/netinet/ip.h#L51-L59 might also be an option.


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

https://reviews.llvm.org/D131982