[PATCH] D42178: [Doc] Guideline on adding exception handling support for a target

[David Chisnall via Phabricator via llvm-commits]

theraven added a comment.

Thank you very much for making a start on this!



================
Comment at: docs/ExceptionHandling.rst:853
+  ``.eh_frame`` section. Unwinder uses the information stored in ``.eh_frame``
+  to unwind stack.
+
----------------
Most of this is generic code.  The important thing for the back end is to make sure that every register has a unique DWARF number.

In your `TargetFrameLowering`'s `emitPrologue` method, you must add the `.cfi_def_cfa_offset` directive for the frame and then the `.cfi_def_cfa_register` or similar to tell the unwinder the base address for all subsequent offsets.


================
Comment at: docs/ExceptionHandling.rst:860
+  through the registers specified by ``getExceptionPointerRegister`` and
+  ``getExceptionSelectorRegister`` respectively.
+
----------------
Perhaps note that the first of these contains a pointer and the second an integer.  On most platforms, they will be GPRs, but on any architecture with distinct address and integer registers the first will likely be an address.


================
Comment at: docs/ExceptionHandling.rst:865
+  The ISD node represents ``__builtin_eh_return``. Depends on your target,
+  you might have to handle ``EH_RETURN`` in TargetLowering.
+
----------------
Perhaps document what this does?  I believe that it's an undocumented GCC extension that is used only in the GNU unwinder (LLVM's libUnwind doesn't use it).  According to comments in the MIPS back end, it is of the form `__builtin_eh_return (offset, handler)` and adjusts the stack by offset and then jumps to the handler.  


================
Comment at: docs/ExceptionHandling.rst:870
+`libunwind <https://clang.llvm.org/docs/Toolchain.html#unwind-library>`_
+to see what have to be done there.
----------------
It would be good to enumerate the things that are needed to get this working.  This diff adds 64-bit MIPS support (soft-float only):

https://github.com/CTSRD-CHERI/libunwind/commit/7b3c8ca32786b7b63975f3c5f93f41d69578416b

It contains a bug where a couple of registers that should have been saved weren't, but it provides an example of all of the things that you need to add to support a new architecture.

The only difficult thing is correctly setting `_LIBUNWIND_CONTEXT_SIZE` and `_LIBUNWIND_CURSOR_SIZE` - you get a compile error that tells you if you got them wrong, but it doesn't tell you what the correct size should be.  I did this by temporarily replacing the `static_assert` with a template that invoked `static_assert` and gave me the arguments in the failure backtrace.


https://reviews.llvm.org/D42178