[PATCH] D71647: [llvm-objcopy][MachO] Handle relocation entries where r_extern is 0

[Seiya Nuta via Phabricator via llvm-commits]

seiya marked 2 inline comments as done.
seiya added inline comments.


================
Comment at: llvm/test/tools/llvm-objcopy/MachO/copy-relocations.s:27-30
+# CHECK:      Relocation information (__DATA,__bar) 2 entries
+# CHECK-NEXT: address  pcrel length extern type    scattered symbolnum/value
+# CHECK-NEXT: 00000000 False quad   False  SUB     False     2 (__DATA,__bar)
+# CHECK-NEXT: 00000000 False quad   False  UNSIGND False     1 (__TEXT,__text)
----------------
jhenderson wrote:
> I think this should go in place of the comment above.
> 
> Is it possible to use yaml to create this situation instead of assembly? Assembly is less obvious as to what you want to achieve.
> Is it possible to use yaml to create this situation instead of assembly? Assembly is less obvious as to what you want to achieve.
That makes sense. AFAIK, yaml2obj does not yet support relocation entries so I'll write a patch for it first.



================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:93
   // corresponding content inside the binary.
-  std::vector<Section> Sections;
+  std::vector<std::unique_ptr<Section>> Sections;
 
----------------
jhenderson wrote:
> Is this really necessary? As far as I can tell, the only place it's actually important is where you create a vector of section pointers, but you can do that without needing these to be unique_ptrs. I've commented above.
I think this should be unique pointers. Pointers to vector elements could be invalidated by --add-section/--remove-section operations.



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

https://reviews.llvm.org/D71647