[PATCH] D80049: [lld-macho] Add some relocation validation logic

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:28
 
-  uint64_t getImplicitAddend(const uint8_t *loc, uint8_t type) const override;
+  uint64_t getImplicitAddend(MemoryBufferRef, const section_64 &,
+                             const relocation_info &) const override;
----------------
Low-priority follow-up: be consistent about including or not including parameter names in all these declarations, both here and in the header.


================
Comment at: lld/MachO/Arch/X86_64.cpp:45-46
+                                    const relocation_info &rel) {
+  return ("invalid type " + std::to_string(rel.r_type) +
+          " relocation at offset " + std::to_string(rel.r_address) + " of " +
+          sec.segname + "," + sec.sectname + " in " + mb.getBufferIdentifier())
----------------
Nit: I feel like "invalid relocation of type X" would read better than "invalid type X relocation", both here and in getImplicitAddend below.

It would be really nice to be able to print the enum as a string instead of just its number (e.g. saying "relocation of type X86_64_RELOC_BRANCH" instead of "relocation of type 2"). There's `MachOObjectFile::getRelocationTypeName` for this purpose; it's a member function, but we could factor its main logic out into a static function. That definitely shouldn't block this change from going in though; it can easily just be another (low-pri) follow-up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80049