[PATCH] D159294: [llvm-nm][MachO] Add support for `MH_FILESET`

[Antonio Frighetto via Phabricator via llvm-commits]

antoniofrighetto added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/MachO.h:901-906
+union lc_str {
+  uint32_t offset;
+#ifndef __LP64__
+  char *ptr;
+#endif
+};
----------------
JDevlieghere wrote:
> Why is the ifdef necessary? At first I thought the layout of the struct is different for 32-bit vs 64-bit targets but looking at `loader.h` I don't see any immediate evidence of that. Either way, changing the layout based on a compile-time constant/host property seems almost guaranteed to be wrong.
Right, clearly the fact of compiling LLVM for a 32-bit target does not entail that the `lc_str` union has a `ptr` member. I think my original intent here was – mistakenly – to adhere to the XNU header, but the layout of the struct should remain constant regardless of the host's architecture (I still left the union just to reflect the header). Dropped it, thanks!


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

https://reviews.llvm.org/D159294