[PATCH] D119671: [ObjectYAML][MachO] Add basic chained fixups support

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 14 00:45:25 PST 2022


jhenderson added a comment.

I don't really know Mach-O myself, so I can't really comment on the correctness of this patch.



================
Comment at: llvm/lib/Object/MachOObjectFile.cpp:1384-1387
+      if ((Err = checkLinkeditDataCommand(
+               *this, Load, I, &DyldChainedFixupsLoadCmd,
+               "LC_DYLD_CHAINED_FIXUPS", Elements, "chained fixups data")))
+        return;
----------------
I see this pattern is very common. I think it would be worth factoring out the repetivtive function call into a lambda, to save repeating most of the arguments (`*this`, `Load`, `I`, and `Elements` appear to be common to all `checkLinkeditDataCommand`). This might be worth doing as a prerequisite patch, and tidying up the `checkDyldInfoCommand` calls in the same manner. There might be other refactorings worth doing to reduce repetitiveness even further, but that's probably going a bit far.


================
Comment at: llvm/test/ObjectYAML/MachO/chained_fixups.yaml:183-184
+# RUN: obj2yaml %t | FileCheck %s
+# CHECK: ChainedFixups: [ 0x0, 0x0, 0x0, 0x0, 0x20, 0x0, 0x0, 0x0, 0x30, 0x0,
+# CHECK: DyldExportsTrie: [ 0x0, 0x1,
----------------
These look incomplete to me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119671



More information about the llvm-commits mailing list