coregrind: Don't call final_tidyup (__libc_freeres) on FatalSignal

When a program gets a fatal signal (one it doesn't handle) valgrind
terminates the program. Before termination it will try to call
final_tidyup which tries to run __libc_freeres and
__gnu_cxx::__freeres to get rid of some memory glibc or libstdc++
don't normally release.

But when the program got the fatal signal in a critical section inside
glibc it might leave the datastructures in a bad state and cause
__libc_freeres to crash.  This makes valgrind itself crash just before
producing its own error summary, making the valgrind run unusable.

A reproducer can found at
https://bugzilla.redhat.com/show_bug.cgi?id=1952836 and
https://bugzilla.redhat.com/show_bug.cgi?id=1225994#c7

This reproducer is really a worse case scenario with multiple threads
racing to get into the critical section that when interrupted will
make __libc_freeres unable to cleanup. But it seems a good policy in
general. If a program is terminated by a fatal signal instead of
normal termination, it seems not having some of the glibc/libstdc++
resource cleaned up is an expected thing.

https://bugs.kde.org/show_bug.cgi?id=443605

diff --git a/NEWS b/NEWS
index d8addbb203384cc597b71de79abd2413ccb6a415..bd4458daefb36bde843118d63dcd55f1bf7b7a75 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -86,6 +86,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 443180  The subnormal test and the ISA 3.0 test generate compiler warnings.
 443314  In the latest GIT version, Valgrind with "--trace-flags" crashes at
         "al" register
+443605  Don't call final_tidyup (__libc_freeres) on FatalSignal
 
 To see details of a given bug, visit
   https://bugs.kde.org/show_bug.cgi?id=XXXXXX

diff --git a/coregrind/m_main.c b/coregrind/m_main.c
index 56f9c6cbf036da29832ea5bb0402dd79b89774ee..70b6c054950291e40925ee6ea2ff8b8c45dc03e0 100644 (file)
--- a/coregrind/m_main.c
+++ b/coregrind/m_main.c
@@ -2168,6 +2168,7 @@ void shutdown_actions_NORETURN( ThreadId tid,
             || tids_schedretcode == VgSrc_ExitProcess
              || tids_schedretcode == VgSrc_FatalSig );
 
+   /* Try to do final tidyup on "normal" exit, not on FatalSig.  */
    if (tids_schedretcode == VgSrc_ExitThread) {
 
       // We are the last surviving thread.  Right?
@@ -2185,7 +2186,7 @@ void shutdown_actions_NORETURN( ThreadId tid,
       vg_assert(VG_(is_running_thread)(tid));
       vg_assert(VG_(count_living_threads)() == 1);
 
-   } else {
+   } else if (tids_schedretcode == VgSrc_ExitProcess) {
 
       // We may not be the last surviving thread.  However, we
       // want to shut down the entire process.  We hold the lock