We need exceptions/RTTI for non-embedded use cases; how should we let people opt out?

[swolchok]

The problem

The current setup in our build system is to disable exceptions and RTTI in all release builds. As I work on sharing code with PyTorch (mostly outside of ExecuTorch core, as in #7041, so we don't need to support embedded systems), that will become untenable: PyTorch code might throw. (I plan to accommodate this by adding a boundary converting exceptions in non-portable kernels into failure codes (pytorch/pytorch#144341), so that the core runtime can remain exception-agnostic.) However, we also want to support embedded systems use cases where size is critical.

Background

• I assume that embedded use cases will not involve Python, or at least will involve direct use of CMake.
• I assume that it is acceptable for us to leave exceptions and RTTI enabled for OSS mobile use cases; mobile developers who know what they are doing (see the next item) can attempt to disable them on their own.
• Mixing C++ code with -fexceptions and -fno-exceptions within the same binary or shared library is very bad; it has similar consequences to violating the One Definition Rule. Briefly, you can end up with two versions of functions that attempt to vary their behavior depending on whether exceptions are enabled (such as some standard library functions!): an -fexceptions version, which handles exception cleanly, and an -fno-exceptions version, which terminates. Which one actually gets linked into the final binary is unpredictable, so you can get unexpected terminations caused by exceptions that look like they should be properly handled

Options going forward

1. Keep the default as-is (global -fno-exceptions -fno-rtti in release builds). Any EXECUTORCH_BUILD_FOO option that results in a PyTorch dependency must change the global build flags to re-enable exceptions and RTTI, probably via a CMake macro or function that both adds the dependency and fixes the global build flags.
2. Change the default to enable exceptions and RTTI. Let embedded folks (and CI scripts) set build flags appropriately when they run CMake; after all, they're likely to want or need to fiddle with flags anyway. They might set build flags in one of a few ways:
   a) CMAKE_CXX_FLAGS (or, equivalently, the CXXFLAGS environment variable)
   b) an "embedded mode" CMake option we create that sets -fno-exceptions -fno-rtti. (I considered having it imply size optimization, but that is the job of CMAKE_BUILD_MODE=MinSizeRel.)

Personally:

• I think (1) is probably not the best thing to do, but it should be straightforward enough to implement.
• I am most partial to (2a); embedded developers seem to me to be among the most likely to Know What They Are Doing with regards to build flags and have strong opinions.

[orionr]

@dbort do you have thoughts on this one? Also @malfet as CMake expert for PyTorch broadly? I think I would trend toward 2a as well

[malfet]

I think I would trend toward 2a as well

I'm more in favor of 2b approach as it feels reasonable to have different defaults for different targets/compilers (most of the linux compilers access -fno-exceptions, but if not mistaken, for MSVC similar options is /EHa-), but giving user an option to override this behavior via pair of EXECUTORCH_FORCE_DISABLE_EXCEPTIONS/EXECUTORCH_FORCE_ENABLE_EXCEPTIONS or something like that.

I'm not following closely the most recent recommendations, but I think for iOS builds exceptions are enabled by default starting with iOS 11, and similarly recent Android targets are also fine with exceptions handling, but IMO they should be disabled for some embedded targets.

[malfet]

Mixing C++ code with -fexceptions and -fno-exceptions within the same binary or shared library is very bad; it has similar consequences to violating the One Definition Rule.

To the best of my knowledge it'll just lead to memory leaks, but totally legit and probably explained in the standard, as it's similar to pass C++ function that throws as a callback to C functions and catching the exception in C++. But mixing rtti and nortti is bad

[swolchok]

totally legit and probably explained in the standard

The ability to disable exceptions is not a part of standard C++.

To the best of my knowledge it'll just lead to memory leaks

nope, it's much worse. Consider std::make_exception_ptr from libc++, which I would summarize like so:

template<class _Ep>
exception_ptr
make_exception_ptr(_Ep __e) _NOEXCEPT
{
#ifndef _LIBCPP_NO_EXCEPTIONS
    try
    {
        throw __e;
    }
    catch (...)
    {
        return current_exception();
    }
#else
    ((void)__e);
    _VSTD::abort();
#endif
}

If make_exception_ptr is called but not inlined in multiple .o files in your binary, and you build some of those with -fexceptions and some with -fno-exceptions, the final version used by the linker could be either one. If you are unlucky and get the -fno-exceptions version, then your -fexceptions-compiled functions that call make_exception_ptr will abort()! The behavior here can be difficult to pin down because it depends on both inlining at each callsite and how your linker decides which of the multiple definitions to use.

(Just to be clear, make_exception_ptr is only an example. Another way to run into issues is to attempt to unwind through an unexpectedly-fno-exceptions of some innocent library function that the linker happened to pick for you; the manifestation in that case will be crashes caused by an exception that obviously has a try/catch that should catch it.)

[swolchok]

#7746 is up and implements (2a).

[mergennachin]

Okay, let's imagine that I'm an embedded engineer who wants to use ExecuTorch for my AI application. And there's some size/memory restrictions.

How should I go about, from a user's perspective?

It looks like depending on the model, I can use ExecuTorch or not. For example, if the model happens to use an op that is used as part of the PyTorch code sharing repo, I won't be able to use ExecuTorch?

How do they know about this in the first place and how do they fix this?

I imagine that I will compile with appropriate flags, and it will complain? Which is fine. But how do I go about fixing it?

It sounds like we're actually changing core requirements of ExecuTorch in this PR

[swolchok]

if the model happens to use an op that is used as part of the PyTorch code sharing repo, I won't be able to use ExecuTorch?

portable kernels and ExecuTorch core will not(*) share kernel code with PyTorch, for exactly the reasons you described.

• -- we need to share a minimal amount of things, notably the half/bfloat16 types, but these do not require exceptions.

How should I go about, from a user's perspective?

The same way you go about building any other project. Make sure build flags are configured the way you want them in the ordinary way, using CMake's support for that.

[mergennachin]

portable kernels and ExecuTorch core will not(*) share kernel code with PyTorch, for exactly the reasons you described.

Okay, maybe I misunderstood the larger context on your original code sharing body of work. So what code that is being shared between ET and PT, that add this new exception requirement?

[mergennachin]

portable kernels and ExecuTorch core will not(*) share kernel code with PyTorch, for exactly the reasons you described.

If that's the case, can we also make sure that size_test and size_test_all_ops can be can compiled with no exception compile flag?

Basically change the default, as you are doing in your PR, but adjust the current CI test for size_test_* to use --no-exceptions, --no-rtti flags.

[swolchok]

So what code that is being shared between ET and PT, that add this new exception requirement?

anything besides ExecuTorch core and the portable ops library, since those are the things that are subject to the additional requirements of embedded systems. mobile phone are much much more similar to desktop computers than embedded systems these days.

[digantdesai]

I guess from embedded user friendliness point of view 2(b) is what I like.

And, I think we should continue running CI for no-except, no-rtti flavor for non-embedded use as well, just so we don't regress from where we are today.