llvm dev - Dec 2020 - Catching exceptions while unwinding through -fno-exceptions code

That makes sense.  Really appreciate the feedback, all.

I think the approach I'll look at implementing is probably to:
- Implement a dedicated 'termination' personality function (in
compiler-rt?) that does appropriate logging + exit.
- Add a flag (-fterminate-exceptions?). This is because this is a very
clear behavior change, so at least providing an opt-in/opt-out mechanism
seems important. Possible option: make this an enum, have 'strict' = all
exceptions crash, 'normal' = exceptions passing through methods that
would
require a cleanup /etc. terminate, none (default) = current behavior where
things just leak/etc.
- During code generation, when -fno-exceptions is turned on, if
-fterminate-exceptions was passed, it changes the personality function from
being not-present to being the dedicated -fno-exceptions termination
personality function.

Not sure how much binary size balances with other concerns, but it
sounds
> to me that the methods proposed are ones that would result in false
> positives where unwinding through the frame would have resulted in no
> action even when compiled with exceptions fully on.
>
> Perhaps leaving functions that would otherwise be "transparent"
to
> exception handling alone is already implied?
>
So I think this is actually not ideal behavior, at least for the use case I
have in mind.

I think I'd prefer (and the team I partner with would prefer) /any/
exception passing from code compiled with -fexceptions to code compiled
with -fno-exceptions to halt & return an error, even if the method
wouldn't
have participated in exception handling (e.x. no classes that need to have
destructors called, etc.)  I think the most desirable behavior is "the
program halts if exceptions pass through code compiled without exceptions
enabled".

There's a few reasons for this:
First, because you can imagine that you could wind up with a situation
where a "happy path" will usually work (but then you get an unexpected
halt
on a less well tested path).
Second, because you can imagine a situation where that winds up putting
code in a very weird position where adding a local variable with a
destructor that must be called changes how a particular method participates
in exception handling from "it just passes exceptions through"  to
"it
crashes". This could leave code in a weird state where it's hard to
reason
about the impact of a change or it goes from a perceived "working
fine"
state to a crashing state.
Third, it makes using *SAN code harder and less predictable. The reason I
became aware of this issue at all is that various sanitizers will insert
landing pads to keep track of stack unwinding (but don't do that with
-fno-exceptions code or if they believe an exception cannot pass through
the method).  That then leads to very, very weird behavior with those
sanitizers (memory leaks, very weird/hard to unravel stacks as new frames
get consistently added, etc.).  Sure--you could make the *SAN build
terminate in that case instead of just behaving weirdly, but it'd be nice
to get that behavior for normal code (since it's unrelated to the *SAN
behavior).

If having an intermediate scenario makes sense (only halting when a method
would have taken part in exception handling), then making the flag have 3
states (strict/normal/none) seems like the right choice to me.

Thoughts/feedback on this approach?

I have verified that changing the personality function to std::terminate
during code generation (via a boolean flag/etc.) does exactly what I'm
looking for (but also has an impact on intermediate sizes of about what I
expected).

Thanks,
--EJM


On Tue, Dec 8, 2020 at 10:05 AM Reid Kleckner <rnk at google.com> wrote:
> I would suggest using a custom personality function for this. It will
> optimize better and be much smaller than using a standard personality
> function. It saves the LSDA tables.
>
> LLVM supports custom personality functions, so only clang changes are
> required. You could either do something like add a flag to override the EH
> personality with a custom one, or come up with a new dedicated
> fno-exceptions termination personality and add it to compiler-rt.
>
> On Mon, Dec 7, 2020 at 3:31 PM Modi Mo via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> If you don’t need to capture more information and can just terminate,
you
>> can directly register std::terminate as the personality routine as
opposed
>> to __gxx_personality_v0 or __CxxFrameHandler3/4 (Windows) which lets
you
>> omit other metadata and work cross-platform.
>>
>>
>>
>> Modi
>>
>>
>>
>> *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on behalf
of Everett
>> Maus via llvm-dev <llvm-dev at lists.llvm.org>
>> *Reply-To: *Everett Maus <evmaus at google.com>
>> *Date: *Monday, December 7, 2020 at 12:47 PM
>> *To: *"llvm-dev at lists.llvm.org" <llvm-dev at
lists.llvm.org>
>> *Subject: *[llvm-dev] Catching exceptions while unwinding through
>> -fno-exceptions code
>>
>>
>>
>> Hey all:
>>
>>
>>
>> I wanted to bring up something that was discussed a few years ago
around
>> the behavior of exceptions when interacting with code compiled with
>> -fno-exceptions. (In
>> https://lists.llvm.org/pipermail/llvm-dev/2017-February/109992.html and
>> https://lists.llvm.org/pipermail/llvm-dev/2017-February/109995.html)
>>
>>
>>
>> It's possible to compile (and link/etc.) code with -fexceptions for
some
>> compilation units and -fno-exceptions for others.  Unlike the behavior
of
>> noexcept (which requires termination), this doesn't have a
specified
>> behavior in the C++ standard as far as I can tell.  However, it can
lead to
>> memory leaks & other issues (e.x. with TSAN, it messes up the
tracking of
>> the current stack frame).
>>
>>
>>
>> I'd be interested in looking into potentially doing the work to add
an
>> option to clang/etc. to terminate when an exception traverses code
compiled
>> with -fno-exceptions, instead of simply allowing the unwinder to walk
>> through the stack frame & leak memory/etc. (possibly behind a
flag?).  This
>> particular issue bit a team I work closely with, and I'd imagine it
could
>> be causing subtle issues for other clang users.
>>
>>
>>
>> I'm mostly concerned with solving this on Linux/x86_64, although if
>> there's a way to solve it more generally I'm open to looking
into doing
>> that instead.
>>
>>
>>
>> I /think/ the right place to change this (from the discussions I
linked)
>> would be in the LLVM -> assembly layer, adding an appropriate
>> .gcc_except_table for functions that are determined to be unable to
throw
>> exceptions (either due to noexcept or due to -fno-exceptions). Then the
>> unwinder would find .eh_frame but no entry in the .gcc_except_table and
>> should terminate (via  __gxx_personality_v0).
>>
>>
>>
>> Am I understanding that correctly?  What's the best way to propose
this
>> sort of change to clang? (document/just try to look at putting together
a
>> PR/other?)
>>
>>
>>
>> Alternatively--one other thing that occurred to me is that it could be
>> reasonably cheap to simply add try/catch blocks that report an UBSAN
error
>> in all methods that shouldn't be able to throw an exception.  This
>> obviously doesn't fix the code-generation problem and would lead to
larger
>> binary sizes, but that seems less bad for an UBSAN build in particular.
>> That would likely meet my needs around wanting a way to automatically
>> detect this behavior/problem, but might not address the more generic
issue.
>>
>>
>>
>> Thanks,
>>
>> --
>>
>> --EJM
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-- 
--EJM
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201208/0e965cff/attachment-0001.html>

One additional question, actually:
>From looking at the current location of exception-handling code, would it
make more sense to have the personality function in libcxxabi (where
__gxx_personality_v0 and friends live) or in compiler-rt (which doesn't
seem to have any exception handling code at the moment)?

Thanks,
Everett Maus

On Tue, Dec 8, 2020 at 11:26 AM Everett Maus <evmaus at google.com> wrote:
> That makes sense.  Really appreciate the feedback, all.
>
> I think the approach I'll look at implementing is probably to:
> - Implement a dedicated 'termination' personality function (in
> compiler-rt?) that does appropriate logging + exit.
> - Add a flag (-fterminate-exceptions?). This is because this is a very
> clear behavior change, so at least providing an opt-in/opt-out mechanism
> seems important. Possible option: make this an enum, have 'strict'
= all
> exceptions crash, 'normal' = exceptions passing through methods
that would
> require a cleanup /etc. terminate, none (default) = current behavior where
> things just leak/etc.
> - During code generation, when -fno-exceptions is turned on, if
> -fterminate-exceptions was passed, it changes the personality function from
> being not-present to being the dedicated -fno-exceptions termination
> personality function.
>
> Not sure how much binary size balances with other concerns, but it sounds
>> to me that the methods proposed are ones that would result in false
>> positives where unwinding through the frame would have resulted in no
>> action even when compiled with exceptions fully on.
>>
>> Perhaps leaving functions that would otherwise be
"transparent" to
>> exception handling alone is already implied?
>>
>
> So I think this is actually not ideal behavior, at least for the use case
> I have in mind.
>
> I think I'd prefer (and the team I partner with would prefer) /any/
> exception passing from code compiled with -fexceptions to code compiled
> with -fno-exceptions to halt & return an error, even if the method
wouldn't
> have participated in exception handling (e.x. no classes that need to have
> destructors called, etc.)  I think the most desirable behavior is "the
> program halts if exceptions pass through code compiled without exceptions
> enabled".
>
> There's a few reasons for this:
> First, because you can imagine that you could wind up with a situation
> where a "happy path" will usually work (but then you get an
unexpected halt
> on a less well tested path).
> Second, because you can imagine a situation where that winds up putting
> code in a very weird position where adding a local variable with a
> destructor that must be called changes how a particular method participates
> in exception handling from "it just passes exceptions through" 
to "it
> crashes". This could leave code in a weird state where it's hard
to reason
> about the impact of a change or it goes from a perceived "working
fine"
> state to a crashing state.
> Third, it makes using *SAN code harder and less predictable. The reason I
> became aware of this issue at all is that various sanitizers will insert
> landing pads to keep track of stack unwinding (but don't do that with
> -fno-exceptions code or if they believe an exception cannot pass through
> the method).  That then leads to very, very weird behavior with those
> sanitizers (memory leaks, very weird/hard to unravel stacks as new frames
> get consistently added, etc.).  Sure--you could make the *SAN build
> terminate in that case instead of just behaving weirdly, but it'd be
nice
> to get that behavior for normal code (since it's unrelated to the *SAN
> behavior).
>
> If having an intermediate scenario makes sense (only halting when a method
> would have taken part in exception handling), then making the flag have 3
> states (strict/normal/none) seems like the right choice to me.
>
> Thoughts/feedback on this approach?
>
> I have verified that changing the personality function to std::terminate
> during code generation (via a boolean flag/etc.) does exactly what I'm
> looking for (but also has an impact on intermediate sizes of about what I
> expected).
>
> Thanks,
> --EJM
>
>
> On Tue, Dec 8, 2020 at 10:05 AM Reid Kleckner <rnk at google.com>
wrote:
>
>> I would suggest using a custom personality function for this. It will
>> optimize better and be much smaller than using a standard personality
>> function. It saves the LSDA tables.
>>
>> LLVM supports custom personality functions, so only clang changes are
>> required. You could either do something like add a flag to override the
EH
>> personality with a custom one, or come up with a new dedicated
>> fno-exceptions termination personality and add it to compiler-rt.
>>
>> On Mon, Dec 7, 2020 at 3:31 PM Modi Mo via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> If you don’t need to capture more information and can just
terminate,
>>> you can directly register std::terminate as the personality routine
as
>>> opposed to __gxx_personality_v0 or __CxxFrameHandler3/4 (Windows)
which
>>> lets you omit other metadata and work cross-platform.
>>>
>>>
>>>
>>> Modi
>>>
>>>
>>>
>>> *From: *llvm-dev <llvm-dev-bounces at lists.llvm.org> on
behalf of Everett
>>> Maus via llvm-dev <llvm-dev at lists.llvm.org>
>>> *Reply-To: *Everett Maus <evmaus at google.com>
>>> *Date: *Monday, December 7, 2020 at 12:47 PM
>>> *To: *"llvm-dev at lists.llvm.org" <llvm-dev at
lists.llvm.org>
>>> *Subject: *[llvm-dev] Catching exceptions while unwinding through
>>> -fno-exceptions code
>>>
>>>
>>>
>>> Hey all:
>>>
>>>
>>>
>>> I wanted to bring up something that was discussed a few years ago
around
>>> the behavior of exceptions when interacting with code compiled with
>>> -fno-exceptions. (In
>>> https://lists.llvm.org/pipermail/llvm-dev/2017-February/109992.html
and
>>>
https://lists.llvm.org/pipermail/llvm-dev/2017-February/109995.html)
>>>
>>>
>>>
>>> It's possible to compile (and link/etc.) code with -fexceptions
for some
>>> compilation units and -fno-exceptions for others.  Unlike the
behavior of
>>> noexcept (which requires termination), this doesn't have a
specified
>>> behavior in the C++ standard as far as I can tell.  However, it can
lead to
>>> memory leaks & other issues (e.x. with TSAN, it messes up the
tracking of
>>> the current stack frame).
>>>
>>>
>>>
>>> I'd be interested in looking into potentially doing the work to
add an
>>> option to clang/etc. to terminate when an exception traverses code
compiled
>>> with -fno-exceptions, instead of simply allowing the unwinder to
walk
>>> through the stack frame & leak memory/etc. (possibly behind a
flag?).  This
>>> particular issue bit a team I work closely with, and I'd
imagine it could
>>> be causing subtle issues for other clang users.
>>>
>>>
>>>
>>> I'm mostly concerned with solving this on Linux/x86_64,
although if
>>> there's a way to solve it more generally I'm open to
looking into doing
>>> that instead.
>>>
>>>
>>>
>>> I /think/ the right place to change this (from the discussions I
linked)
>>> would be in the LLVM -> assembly layer, adding an appropriate
>>> .gcc_except_table for functions that are determined to be unable to
throw
>>> exceptions (either due to noexcept or due to -fno-exceptions). Then
the
>>> unwinder would find .eh_frame but no entry in the .gcc_except_table
and
>>> should terminate (via  __gxx_personality_v0).
>>>
>>>
>>>
>>> Am I understanding that correctly?  What's the best way to
propose this
>>> sort of change to clang? (document/just try to look at putting
together a
>>> PR/other?)
>>>
>>>
>>>
>>> Alternatively--one other thing that occurred to me is that it could
be
>>> reasonably cheap to simply add try/catch blocks that report an
UBSAN error
>>> in all methods that shouldn't be able to throw an exception. 
This
>>> obviously doesn't fix the code-generation problem and would
lead to larger
>>> binary sizes, but that seems less bad for an UBSAN build in
particular.
>>> That would likely meet my needs around wanting a way to
automatically
>>> detect this behavior/problem, but might not address the more
generic issue.
>>>
>>>
>>>
>>> Thanks,
>>>
>>> --
>>>
>>> --EJM
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>
>
> --
> --EJM
>

-- 
--EJM
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201208/e47da002/attachment.html>

On Tue, Dec 8, 2020 at 2:26 PM Everett Maus via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> That makes sense.  Really appreciate the feedback, all.
>
> I think the approach I'll look at implementing is probably to:
> - Implement a dedicated 'termination' personality function (in
> compiler-rt?) that does appropriate logging + exit.
>
I still think we don't actually need to introduce a new personality
function -- just reusing the standard C++ one, instead.

- Add a flag (-fterminate-exceptions?). This is because this is a
very
> clear behavior change, so at least providing an opt-in/opt-out mechanism
> seems important. Possible option: make this an enum, have 'strict'
= all
> exceptions crash, 'normal' = exceptions passing through methods
that would
> require a cleanup /etc. terminate, none (default) = current behavior where
> things just leak/etc.
>
I agree we probably do need a flag for this change in behavior, but I think
this flag is going to be very difficult to name and explain, because there
are currently *different* behaviors depending on the target and other flags
you pass. This existing behavior is very inconsistent and confusing...

1. With -fno-asynchronous-unwind-tables -fno-exceptions, no unwind info is
emitted, and therefore, attempting to throw an exception through such a
function already aborts. This is the historically-common behavior.

2. With -fasynchronous-unwind-tables -fno-exceptions, unwind info is
emitted for functions, and that unwind info currently specifies that
exceptions can be thrown through such functions transparently.

To make matters worse, the default of -fasynchronous-unwind-tables is
different per-platform. In Clang, it's enabled for x86-64-linux, and off
for i386-linux and ppc64-linux. And, in GCC, the default value even depends
on other flags, and is customized in some distro's builds of the compiler.
Whether it's on or off for i386-linux depends on whether you specify
-fomit-frame-pointer or not...except that Debian (and maybe other distros?)
have patched the spec file to enable async unwind tables, regardless.


- During code generation, when -fno-exceptions is turned on,
if
> -fterminate-exceptions was passed, it changes the personality function from
> being not-present to being the dedicated -fno-exceptions termination
> personality function.
>
We'll also probably want to adjust the inliner, because it otherwise
prohibits inlining between functions with different personality routines
(which will be relevant with cross-TU inlining with LTO).

Anyways -- what I'd really like to see done here is to BOTH fix the
handling of C++ "noexcept" functions to have minimal overhead, and
then
simply use the same mechanism to implement -fno-exceptions. This means we
do in general need to be able to implement this behavior with the
__gxx_personality_v0 personality, because noexcept functions can have
try/catch blocks inside.

I believe that the current design of noexcept -- which requires explicit
invoke and landingpads saying that they terminate -- should be revisited.
It adds a lot of unnecessary overhead.

What we need is a way to explicitly declare that we want the
personality-routine's default action to be taken on unwind -- implemented
via leaving a gap in the LSDA table. I think we might want to do this both
as syntax on invoke [e.g. `invoke void @xyz() to label %l unwind default`],
as well as a function attribute declaring that any call in the function
should be treated as having the default action of the personality function.

"nothrow" would emit this new function attribute in combination with
nounwind. Inlining a function having this attribute into a function not
having the attribute would need to convert `call ...` into `invoke ...
unwind default` -- but since the inliner already has such code, that's not
a big deal.

Not sure how much binary size balances with other concerns, but it
sounds
>> to me that the methods proposed are ones that would result in false
>> positives where unwinding through the frame would have resulted in no
>> action even when compiled with exceptions fully on.
>>
>> Perhaps leaving functions that would otherwise be
"transparent" to
>> exception handling alone is already implied?
>>
>
> So I think this is actually not ideal behavior, at least for the use case
> I have in mind.
>
> I think I'd prefer (and the team I partner with would prefer) /any/
> exception passing from code compiled with -fexceptions to code compiled
> with -fno-exceptions to halt & return an error, even if the method
wouldn't
> have participated in exception handling (e.x. no classes that need to have
> destructors called, etc.)  I think the most desirable behavior is "the
> program halts if exceptions pass through code compiled without exceptions
> enabled".
>
I agree that is a desirable behavior.

There's the additional complication of C code.  Historically, before
-fasynchronous-unwind-tables became a common default, throwing through C
functions would also abort, by default. That's why Glibc builds C functions
that might call user C++ code with -fexceptions (example: qsort) -- that
way it can ensure that unwinding a C++ exception does actually work
properly.

Unfortunately, unlike in C++, we don't have a ready-made personality
function we can use for C functions which has "terminate" behavior.
The
_gcc_personality_v0 doesn't terminate upon failing to find a LSDA table
entry.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201210/51959e07/attachment.html>