[PATCH] Add RelocVisitor support for MachO

[Keno Fischer]

>
> I’m sorry, I’m out of work for a few weeks, and I won’t be able to review
> it correctly. However a couple of things stood out when I looked at it last
> week:
>

No problem, thanks for the reply - my apologies if my reply sounded harsh,
I've just been carrying some version of this patch locally for about a year
and carrying LLVM patches is a pain, even if they're small.


>  - you definitely should separate the debug line fix from the RelocVisitor
> implementation.
>

Ok, but the problem is that I'm not sure I can cook up a test case in ELF
that will properly demonstrate this problem. Similarly, the test case for
MachO fails without the bugfix. I suppose we can get the MachO RelocVisitor
implementation in with an incorrect test case and then have the bugfix
change that test case. Does that sound reasonable?


>  - MachO object files contain another relocation type
> (X86_64_RELOC_SUBTRACTOR if memory serves), and I don’t know if it really
> fits in the RelocVisitor model (as it needs access to the following reloc
> to make sense). Why is RELOC_UNSIGNED sufficient for your usecase?
>

I haven't had LLVM generate anything else on my test cases, so I didn't
need to implement it. I think it's easier to implement them one at a time
with proper test cases, which I didn't have for the X86_64_RELOC_SUBTRACTOR
case. I'd be happy to implement the other relocations, but I think it make
sense to get the simple case settled first.


> Fred
>
> >
> > REPOSITORY
> >  rL LLVM
> >
> > http://reviews.llvm.org/D8148
> >
> > EMAIL PREFERENCES
> >  http://reviews.llvm.org/settings/panel/emailpreferences/
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150317/768137ed/attachment.html>