Evaluate the need and use for trapping floating point exceptions

[sebproell]

#1476 revealed that we are doing something that is UB: throwing from a trap handler, here: https://github.com/4C-multiphysics/4C/blob/main/apps/global_full/4C_global_full_main.cpp#L253

This has been introduced in 20ba962 with the goal to get a nice stack trace. However, the stack trace contains the C++ exception unwind mechanism, which fails to perform the unwind since we are not in a C++ context when the OS calls the handler. So this is a bit questionable. The actual call site shows up further down in the stack trace on my machine but this seems completely OS-dependent.

Overall, the program aborts suddenly and prints a garbled stack trace. I made the following observations:

• How useful is it to abort on a floating-point exception (overflow, divide by zero, etc.) in general? Should we rather sanity check results, e.g., after a linear solver? Note that these serious errors will often lead to wrong results that are so obvious that it is not possible to miss them.
• The treatment of floating-point exceptions is wild. Search for feenableexcept and you will see that structure_new suddenly enables FE_OVERFLOW.
• There is -fnon-call-exceptions which enables throwing from signal handlers, but this seems to be little used, and clang seems to have regressed on it.

[isteinbrecher]

Thanks for raising this question @sebproell.

In my experience whenever such an error is thrown, this means that a bug of some sort was triggered (often by a previously not considered edge case). In general having this "notification" is a good thing - however, it requires a lot of experience to debug 4C if such a floating point exception is raised. And as @sebproell mentioned, usually these "bugs" will also show up somewhere else, e.g., result values, convergence of algorithms, ... . So my take on this is: every improvement in usability/debuggability w.r.t. fe exceptions is a good thing four 4C. I am not an expert in how such things are handled on the c++, but maybe this could be optional or only in debug mode.

[sebproell]

Only doing this in debug mode seems reasonable. Still, the question remains of what exactly we want to happen when an SIGFPE is raised. The current approach (throwing a C++ exception) is UB.

[ppraegla]

TL;DR: Can we just remove the signal handling?

I'm not familiar with the signal handling stuff. From quickly reading into the topic, I get that the SIGPFE signal should terminate the program by default. So, there is no need to throw an exception.

Below is the output of how it is now, and when I just remove the signal handle (I introduced a divide by zero).

Output now

=========================================================================
PROC 0 ERROR in /workspace/4C/4C/apps/global_full/4C_global_full_main.cpp, line 253:
4C produced a FE_OVERFLOW floating point exception.
------------------
 0# 0x00005ECF36BD144B in ./build/release/4C
 1# 0x00007B5FDD442520 in /lib/x86_64-linux-gnu/libc.so.6
 2# FourC::Solid::TimeInt::Implicit::setup() in /workspace/4C/4C/build/release/lib4C.so
 3# FourC::Adapter::StructureBaseAlgorithmNew::setup_tim_int() in /workspace/4C/4C/build/release/lib4C.so
 4# FourC::Adapter::StructureBaseAlgorithmNew::setup() in /workspace/4C/4C/build/release/lib4C.so
 5# FourC::dyn_nlnstructural_drt() in /workspace/4C/4C/build/release/lib4C.so
 6# entrypoint_switch() in ./build/release/4C
 7# run(FourC::CommandlineArguments&) in ./build/release/4C
 8# main in ./build/release/4C
 9# 0x00007B5FDD429D90 in /lib/x86_64-linux-gnu/libc.so.6
10# __libc_start_main in /lib/x86_64-linux-gnu/libc.so.6
11# _start in ./build/release/4C

=========================================================================

Output without signal handler

[workstation:78319] *** Process received signal ***
[workstation:78319] Signal: Floating point exception (8)
[workstation:78319] Signal code: Floating point divide-by-zero (3)
[workstation:78319] Failing at address: 0x79faf0e587fc
[workstation:78319] [ 0] /lib/x86_64-linux-gnu/libc.so.6(+0x42520)[0x79fad8242520]
[workstation:78319] [ 1] /workspace/4C/4C/build/release/lib4C.so(_ZN5FourC5Solid7TimeInt8Implicit5setupEv+0x31c)[0x79faf0e587fc]
[workstation:78319] [ 2] /workspace/4C/4C/build/release/lib4C.so(_ZN5FourC7Adapter25StructureBaseAlgorithmNew13setup_tim_intEv+0xe2b)[0x79fae6c96e9b]
[workstation:78319] [ 3] /workspace/4C/4C/build/release/lib4C.so(_ZN5FourC7Adapter25StructureBaseAlgorithmNew5setupEv+0x90)[0x79fae6c975a0]
[workstation:78319] [ 4] /workspace/4C/4C/build/release/lib4C.so(_ZN5FourC21dyn_nlnstructural_drtEv+0x839)[0x79faf0d72229]
[workstation:78319] [ 5] ./build/release/4C(_Z17entrypoint_switchv+0x8d)[0x57f2792835ed]
[workstation:78319] [ 6] ./build/release/4C(_Z3runRN5FourC20CommandlineArgumentsE+0x25d)[0x57f2792b303d]
[workstation:78319] [ 7] ./build/release/4C(main+0x4ca)[0x57f2792be35a]
[workstation:78319] [ 8] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90)[0x79fad8229d90]
[workstation:78319] [ 9] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80)[0x79fad8229e40]
[workstation:78319] [10] ./build/release/4C(_start+0x25)[0x57f2792827f5]
[workstation:78319] *** End of error message ***
Floating point exception (core dumped)

Of course, the current output is nicer. But from the output without the signal handle, I still get enough information to start debugging.
I only debugged an FE_OVERFLOW exception once or twice. If I remember correctly, the stack trace did not help too much because the stack trace only showed where the code currently was when it received the signal, which is not necessarily the place where the FE_OVERFLOW happened (since the signal is called asynchronously). For debugging, I just spammed the code with the following code snippet to check where the overflow happened

#include <fenv.h>

std::cout << __FILE__ << ":" << __FUNCTION__ << ":" << __LINE__;
if (fetestexcept(FE_OVERFLOW))
  std::cout << ": bad" << std::endl;
else
  std::cout << ": good" << std::endl;

Maybe there are better ways to debug it, or I did something else wrong.

[sebproell]

Interesting, this looks different on my machine (expected, since it is UB). Are both outputs from the same error case? I am wondering because the first one reports FE_OVERFLOW but the second one divide-by-zero. I agree that the second call stack is as good as the first one. You can easily stop on the signal in your debugger or look into a core dump, so I don't see a difference in debuggability here.

TL;DR I think we drop the signal handler. Whether you want to trap floating-point exceptions is already a configurable option via FOUR_C_ENABLE_FE_TRAPPING. I think this gives the most flexibility and removes UB. The change is that you replace the UB stack trace of 4C with a stack trace from the OS (I guess).

[ppraegla]

Are both outputs from the same error case?

Good point. I did not notice that. It is the same test case: Division by zero 1.0/0.0.

I agree to remove the signal handler to remove the undefined behavior.

[sebproell]

Implemented in #1493