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

Shoaib Meenai via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 19 17:06:39 PDT 2020


smeenai added a comment.

I'm torn about this one.

One the one hand, as you said, putting the validation in relocateOne is justifiable, since it's making a bunch of assumptions about various relocations. On the other, it feels a bit iffy to be mixing concerns like that, and it also means we won't get errors from broken input files until we start the writing process, which is pretty late.

Would it be possible to do the validation in getImplicitAddend instead? That's at least earlier. It also feels a bit better conceptually ... we could have `getImplicitAddend` be the function that does any target-specific relocation processing and validation. Unfortunately, I can't think of a good name that conveys that it also computes and returns the implicit addend :/



================
Comment at: lld/test/MachO/invalid/invalid-relocation.yaml:43
+        content:         '000000000000000000'
+        relocations:
+          - address:         0x00000001
----------------
CC @alexshap the support you added is coming in handy :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80049





More information about the llvm-commits mailing list