[PATCH] D118331: [llvm-objdump][macho] Add initial --chained_fixups support

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/BinaryFormat/MachO.h:298
 
+// header of the LC_DYLD_CHAINED_FIXUPS payload
+struct dyld_chained_fixups_header {
----------------
Comments should use full English grammar, including leading capital letter and trailing period. Applies throughout.


================
Comment at: llvm/test/tools/llvm-objdump/MachO/chained_fixups_dump.test:1
+# RUN: llvm-objdump --macho --chained_fixups %p/Inputs/macho-fixup-chain | FileCheck %s
+
----------------
If possible, it would be preferable to have yaml2obj generate this input (or generate it from IR/assembly), so that the test doesn't rely on an opaque binary.


================
Comment at: llvm/test/tools/llvm-objdump/MachO/chained_fixups_dump.test:51
+# CHECK:   name_offset = 141 (_swift_release)
\ No newline at end of file

----------------
Nit: add new line at EOF.


================
Comment at: llvm/tools/llvm-objdump/MachODump.cpp:10774-10775
+  else
+    WithColor::error() << "This operation is only currently supported "
+                          "for Mach-O executable files.\n";
+}
----------------
Please refer to the LLVM coding standards for error and warning messages.

Is there a generic llvm-objdump error function you could use instead?


================
Comment at: llvm/tools/llvm-objdump/ObjdumpOpts.td:248-250
+def chained_fixups : Flag<["--"], "chained_fixups">,
+  HelpText<"Print raw chained fixup data present in a final linked binary built with chained fixups. (requires --macho)">,
+  Group<grp_mach_o>;
----------------
Please make sure new options are in the llvlm-objdump CommandGuide located in llvm/docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118331