[PATCH] D80854: [lld-macho] Properly handle & validate relocation r_length

[Jez Ng via Phabricator via llvm-commits]

int3 marked 4 inline comments as done.
int3 added inline comments.


================
Comment at: lld/MachO/Arch/X86_64.cpp:58
+
+  std::string msg = getErrorLocation(mb, sec, rel) + ": relocations of type " +
+                    std::to_string(rel.r_type) + " must have r_length of ";
----------------
smeenai wrote:
> It's an error path so efficiency doesn't matter, but in general, Twine is a better choice for concatenations like this.
hm I spent some time trying to use Twine but ended up with all sorts of corrupted strings. I even put all the intermediate temp values into local variables, but still no dice...


================
Comment at: lld/MachO/Arch/X86_64.cpp:124
     // since the RIP has advanced by 4 at this point.
-    write32le(loc, val - 4);
+    val -= 4;
     break;
----------------
smeenai wrote:
> In particular, this is incorrect if we're allowing the r_length for an X86_64_RELOC_BRANCH to be 0.
Good catch! Added to the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80854