[PATCH] D136879: [LLD] [COFF] Include "rust_eh_personality" among the known personality functions

Thu Oct 27 14:47:16 PDT 2022

mstorsjo added a comment.

In D136879#3889916 <https://reviews.llvm.org/D136879#3889916>, @rnk wrote:

> In D136879#3889906 <https://reviews.llvm.org/D136879#3889906>, @mstorsjo wrote:
>
>> In D136879#3889829 <https://reviews.llvm.org/D136879#3889829>, @rnk wrote:
>>
>>> I read the Rust issue. Do you think we should do more to promote relocations to discarded sections into errors on mingw, so that this issue becomes a link error, not a runtime error?
>>
>> Ideally yes, but apparently it's not that easy; we blanket disable the error for mingw due to how GCC handles their non-associative .rdata sections for jump tables: D52600 <https://reviews.llvm.org/D52600> Also we disable the error entirely for DWARF - since those aren't split up into nice associative sections, so if building with `-ffunction-sections` and GC, there will be plenty of relocations against discarded things. So offhand I don't really have any good ideas for doing it better, other than going all in for parsing `.eh_frame` like the ELF linker apparently does - but that's pretty much overkill wrt the effort I had hoped to spend on this issue...
>
> I guess .eh_frame has relocations to ~all .text sections, so we can't treat it as a GC root either. =/ I guess we'll just live with it.

Yeah, pretty much.

BTW, the kinda surprising thing here was that we ended up with base relocations for these relocations with a discarded target. Without that, we would just have had a null personality pointer, which would have made the unwind fail differently (probably harder to track down), but a null pointer with a base relocation ends up as non-null, if the image is loaded at a nondefault address. I guess that can lead to some bit of extra overhead, with pointless base relocations in discarded bits in DWARF sections - but probably not worth spending time and effort on either - as it doesn't happen in the PDB/SEH cases.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136879