[PATCH] Allow ARM ELF relocs to be applied twice

[Amara Emerson]

Thanks for working on this Tim. Patch looks good to me. I don't envisage any
scenarios when the R_ARM_PRIVATE_O should escape the MCJIT so I think this
is ok to commit.

Cheers,
Amara

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Tim Northover
Sent: 27 May 2013 20:00
To: Renato Golin
Cc: llvm-commits
Subject: Re: [PATCH] Allow ARM ELF relocs to be applied twice

Hi Renato,

> Sorry, it's holiday today in UK...

Ah! I'd forgotten. Thanks for your comments despite the holiday.

> First, you're re-defining the place-holder many times. I take it it's not
> used on all cases, but often enough to maybe be commoned up and hope the
> compiler does a good job?

That sounds reasonable enough, I've attached a version making this
change. Technically, it may risk undefined behaviour (creating a
pointer outside an object), but it's probably harmless enough to
justify anyway.

> Also, you're using the PRIVATE-0, which has no use AFAICT, but also no
> comment on it about the expected behaviour on ELF.h. I'm not sure what's
the
> policy on enum reserved entries, but one way is to rename it to something
> meaningful, another is to add a one-line comment on ELF.h about its usage
in
> the EE.

I think I'd be opposed to that. I don't want to give any kind of
impression that this relocation has well-defined semantics, which both
renaming and header-documentation would do. (Even the R_ARM_PRIVATE_0
name worried me slightly, since it's not called that in the ELF ABI;
but at least it's obviously different).

This is strictly an LLVM-internal (ELF-MCJIT-internal, even)
implementation-detail, localised to one file and commented there.

Tim.