[llvm-branch-commits] [lld] ELF: Only rewrite non-preemptible IFUNCs to IPLT functions if a non-IRELATIVE relocation is needed. (PR #133531)

[Peter Collingbourne via llvm-branch-commits]

pcc wrote:

> I have some small reservations about using ifunc resolvers like this. Mostly in that we are using a mechanism invented for a different purpose, and relying on some specific linker behaviour to make this case work.
>
> This is similar to comments made in the Discourse post https://discourse.llvm.org/t/rfc-structure-protection-a-family-of-uaf-mitigation-techniques/85555/9 but repeating them here as this is closest to the implementation.
>
> As I understand it, this has a more limited and more specific use case than ifuncs. Traditional ifuncs which can be address taken or called, possibly in multiple ways, so it makes sense to use a symbol type STT_GNU_IFUNC rather than special relocation directives. The initializers for structure field protection are compiler generated, can not be legally called or address taken from user code, and only have one relocation type R_ABS64 (or 32 on a 32-bit platform). With an addition of a single relocation, something like R_ADDRINIT64 which would target a STT_FUNC resolver symbol. We can isolate the structure field initialization use case from an actual ifunc.

I see, so the idea would be that an ADDRINIT64 pointing to an STT_FUNC would generate an IRELATIVE pointing to the symbol address if the symbol is non-preemptible and result in a linker error otherwise.

> I guess it all comes down to whether structure field initialization needs, or benefits from being distinguished from an ifunc. Ifuncs seem to be quite easy to get wrong so being able to isolate this case has some attraction to me at least. It also handles the structure field that points to an ifunc relatively gracefully.

I don't necessarily see usage errors as a convincing argument for ADDRINIT64. Because the relocations and resolvers are compiler generated (and the resolvers cannot be named by the user due to the use of local symbols) it is hard to commit a usage error if you are not a compiler developer who is expected to know what they are doing.

I do think that there are two convincing arguments for ADDRINIT64:
- We want linkers that do not support the new behavior to loudly error out instead of silently generating incorrect code. You'll note that this is the same argument that justifies INST32 (or whatever we end up calling it) when ABS32 already exists and could be made to retrofit the semantics that we want.
- It reduces linker complexity because the IRELATIVE relocations can be added in a single pass.

> As you pointed out in your response, this does mean adding 2 relocations to every psABI that supports structure field protection rather than just one. Although I'd expect the alternative of having relocations that alternatively write "directly call" ifunc resolver or take address of function might require new relocations too?

Yes, we'd still the relocation to select between calling the resolver and taking the address unless we decide to not support other use cases for the inner ifuncs (in which case we'd just generate code to take the address and the IPLT would be used as the address of affected ifuncs). This may be fine as it's the status quo for ifuncs referred to by an ABS64.

https://github.com/llvm/llvm-project/pull/133531