[PATCH] D15781: [compiler-rt] Add support for ARM EHABI to gcc_personality_v0.

[Timon Van Overveldt via llvm-commits]

timonvo added a comment.

In http://reviews.llvm.org/D15781#317439, @compnerd wrote:

> SjLj EH isn't part of this routine (that personality is __gcc_personality_sj0 and is pretty different in behavior).


Right, but compiler-rt's gcc_personality_v0.c *does* use the function name __gcc_personality_sj0 instead, if that type of unwinding is requested, see line 147 (https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/gcc_personality_v0.c#L147). However, besides changing the function name, it uses the exact same implementation for SJLJ based exceptions. This seems wrong, and this seems to be what that Rust patch I linked to addresses.

In http://reviews.llvm.org/D15781#317517, @logan wrote:

> In http://reviews.llvm.org/D15781#317439, @compnerd wrote:
>
> > Finally, I think that if you generate the personality references correctly on the rust side, you won't need the __gcc_personality_v0 unless there is C code trying to handle exceptions.
>
>
> I am not familiar with the Rust compiler.  However, I believe they are leveraging LLVM code generators to translate `landingpad` instructions to exception handlers, LSDA, etc.  In the other words, they are not generating LSDA by themselves.  Besides, LLVM will only generate either Dwarf or SJLJ LSDA, so IMO it is OK to reuse `__gcc_personality_v0` if Rust compiler (or ABI) does not have extra requirements on the personality function.


Right. I believe that's exactly what's happening. I'm not very familiar with Rust's code generation details though.

> BTW, I don't think there is any problem to fix the personality for C language.


Right. Do you mean that, since GCC/Clang *will* generate references to __gcc_personality_v0 if the cleanup attribute is used, this personality should indeed be fixed to support ARM EHABI, regardless of whether Rust needs it? I do think that's the case, because I can't see how this personality routine would ever work for C code generated for ARM EHABI. Just the difference in function signature will lead to corrupt memory accesses if this isn't fixed.

In http://reviews.llvm.org/D15781#317533, @compnerd wrote:

> I didn't mean to imply the C++ personality.  The reconfiguration of the C personality makes things harder IMO.  Now you need to rebuild the library if something changes and control that.  If you absolutely must use this approach, why not introduce a new routine that dispatches as you want (just to be clear this would be different from the language specific personality)?


Are you suggesting having a single PR for C that dispatches to the ARM EHABI PR, the Dwarf PR, the SJLJ PR, depending on which macros are defined? You could do that, but since there's no single function signature that's common between all 3 of those PRs, you'd still end up with a bunch of #ifdefs. Using #ifdefs to switch out whole function implementations rather than using #ifdefs within the function body may make the code a littel cleaner though. Or, if you meant something else, could you elaborate further?

Either way if my understanding is correct that this personality routine would *also* be used by C anyway if the cleanup attribute is used, then we have to support unwinding in C for ARM EHABI somehow, right? I guess another option would be for Clang to be changed to output references to an EHABI specific routine instead, but we can't really change GCC's behavior, so I believe that would go against Clang's binary compatibility with GCC.

> The entire thing with the personalities is that they handle things one way only and you switch the one that is used.


I think it's correct to assume that if one compilation unit uses one style of exceptions (e.g. ARM EHABI), then the whole binary must use that style throughout, right? Hence, every reference to the C PR should be expecting it to be handling the same style of exceptions. Therefore the invariant that PR's "handle things one way only" will be satisfied, within the scope of that binary at least.

Perhaps the fact that GCC does implement multiple versions of this routine depending on the target arch, just like this CL is trying to, is telling that this is the best way to go?


http://reviews.llvm.org/D15781