[PATCH] D144999: [RFC][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

Oops, I'd glossed over that in the commit message. (Also, wow, that is a *long* bug discussion.)

Now that I see it's explicitly driven by Rust needs, I guess my worries about making it a "second-class citizen" are unfounded :)

And if my Swift test case is representative, I guess Swift doesn't actually need a personality function then we don't have to worry about that either. I would like us to be certain of that before proceeding though...

> (Though, I haven't yet verified it functions as I believe it does with a real-world test).

Verifying this would help clarify the path forward too :) But assuming that it works:

> For __gxx_personality_v0 and __objc_personality_v0 (but not __gcc_personality_v0), emit only compact unwind (or only DWARF where compact-unwind isn't possible) for each function -- not both. (This saves object-file size/build time for the most common languages).

This seems reasonable.

> (This does mean we need to preserve the eh_frame info for all functions even when they have compact-unwind too -- right now both are stored in the same Defined::unwindEntry field, since we only ever care about eh-frame unwind if the fn doesn't have compact-unwind)

Now that you've pointed this out, I'm starting to balk at the added complexity (I know, even though I proposed the idea.) We would have to ensure that everything that accesses `unwindEntry` (in particular dead-stripping and ICF) now handles both cases correctly. This work will often be redundant too (since only one of the two copies is really needed.) We would also be increasing the size of `class Defined`, which is not ideal.

As I understand, ld64's best workaround for this right now is the `-no_compact_unwind` flag. This would be trivial for us to support but of course not ideal from a binary size standpoint. However, from a UX standpoint, "getting an error when you add an additional object to the link" is basically the current situation. I think it is more important for us to prioritize perf + implementation simplicity over a somewhat rare UX edge case...

So my proposal is to have a `--prefer-dwarf-unwind` flag. If both compact and DWARF unwind are available in the object file, this flag will tell LLD to prefer DWARF instead of compact unwind. Coupled with the abovementioned change to the assembler, this will get us compact unwind when compiling for "canonical" languages while giving us DWARF unwind for the others, while not requiring us to simultaneously track both encodings in the linker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144999