Refactor and improve crash handler stderr dumps

1. Prevent concurrent crashes to interleave in stderr
2. Cleaner and more compact formatting of registers dump
3. Omit `OLDMASK` (nobody seems to know what is it for)
4. Don't print the top backtrace frame twice
5. Code cosmetics
commit_hash:f7a4c960b3400d6dfb0f8f60317aa524611d2fd0

6 files changed, 123 insertions, 159 deletions

diff --git a/library/cpp/yt/backtrace/symbolizers/dummy/dummy_symbolizer.cpp b/library/cpp/yt/backtrace/symbolizers/dummy/dummy_symbolizer.cpp
index 19cb41e795..3b1d37aa02 100644
--- a/library/cpp/yt/backtrace/symbolizers/dummy/dummy_symbolizer.cpp
+++ b/library/cpp/yt/backtrace/symbolizers/dummy/dummy_symbolizer.cpp
@@ -13,8 +13,8 @@ void SymbolizeBacktrace(
     for (int index = 0; index < std::ssize(backtrace); ++index) {
         TRawFormatter<1024> formatter;
         formatter.AppendNumber(index + 1, 10, 2);
-        formatter.AppendString(". ");
-        formatter.AppendNumberAsHexWithPadding(reinterpret_cast<uintptr_t>(backtrace[index]), 12);
+        formatter.AppendString(". 0x");
+        formatter.AppendNumber(reinterpret_cast<uintptr_t>(backtrace[index]), /*radix*/ 16, /*width*/ 12);
         formatter.AppendString("\n");
         frameCallback(formatter.GetBuffer());
     }
diff --git a/library/cpp/yt/backtrace/symbolizers/dynload/dynload_symbolizer.cpp b/library/cpp/yt/backtrace/symbolizers/dynload/dynload_symbolizer.cpp
index 37ebda8e48..ca4b157045 100644
--- a/library/cpp/yt/backtrace/symbolizers/dynload/dynload_symbolizer.cpp
+++ b/library/cpp/yt/backtrace/symbolizers/dynload/dynload_symbolizer.cpp
@@ -75,10 +75,10 @@ int GetSymbolInfo(const void* pc, char* buffer, int length)
 
 void DumpStackFrameInfo(TBaseFormatter* formatter, const void* pc)
 {
-    formatter->AppendString("@ ");
+    formatter->AppendString("@ 0x");
     const int width = (sizeof(void*) == 8 ? 12 : 8) + 2;
     // +2 for "0x"; 12 for x86_64 because higher bits are always zeroed.
-    formatter->AppendNumberAsHexWithPadding(reinterpret_cast<uintptr_t>(pc), width);
+    formatter->AppendNumber(reinterpret_cast<uintptr_t>(pc), 16, width);
     formatter->AppendString(" ");
     // Get the symbol from the previous address of PC,
     // because PC may be in the next function.
diff --git a/library/cpp/yt/string/raw_formatter.h b/library/cpp/yt/string/raw_formatter.h
index 6956330883..23ea7f9af0 100644
--- a/library/cpp/yt/string/raw_formatter.h
+++ b/library/cpp/yt/string/raw_formatter.h
@@ -84,11 +84,12 @@ public:
         }
     }
 
-    //! Appends a single character and updates the internal cursor.
-    void AppendChar(char ch)
+    //! Appends a single character given number of times and updates the internal cursor.
+    void AppendChar(char ch, int count = 1)
     {
-        if (Cursor_ < End_) {
+        while (Cursor_ < End_ && count > 0) {
             *Cursor_++ = ch;
+            count--;
         }
     }
 
@@ -97,6 +98,8 @@ public:
     {
         int digits = 0;
 
+        width = std::min(width, GetBytesRemaining());
+
         if (radix == 16) {
             // Optimize output of hex numbers.
 
@@ -140,22 +143,6 @@ public:
         }
     }
 
-    //! Formats |number| as hexadecimal number and updates the internal cursor.
-    //! Padding will be added in front if needed.
-    void AppendNumberAsHexWithPadding(uintptr_t number, int width)
-    {
-        char* begin = Cursor_;
-        AppendString("0x");
-        AppendNumber(number, 16);
-        // Move to right and add padding in front if needed.
-        if (Cursor_ < begin + width) {
-            auto delta = begin + width - Cursor_;
-            std::copy(begin, Cursor_, begin + delta);
-            std::fill(begin, begin + delta, ' ');
-            Cursor_ = begin + width;
-        }
-    }
-
     //! Formats |guid| and updates the internal cursor.
     void AppendGuid(TGuid guid)
     {
@@ -185,7 +172,6 @@ private:
     char* const Begin_;
     char* Cursor_;
     char* const End_;
-
 };
 
 template <size_t N>
@@ -203,7 +189,6 @@ public:
 
 private:
     char Buffer_[N];
-
 };
 
 ////////////////////////////////////////////////////////////////////////////////
diff --git a/yt/yt/core/misc/assert.cpp b/yt/yt/core/misc/assert.cpp
deleted file mode 100644
index 3fe7a2863d..0000000000
--- a/yt/yt/core/misc/assert.cpp
+++ /dev/null
@@ -1,65 +0,0 @@
-#include "assert.h"
-
-#include "proc.h"
-
-#include <yt/yt/core/logging/log_manager.h>
-
-#include <library/cpp/yt/assert/assert.h>
-
-#include <library/cpp/yt/string/raw_formatter.h>
-
-#include <library/cpp/yt/system/handle_eintr.h>
-
-#ifdef _win_
-    #include <io.h>
-#else
-    #include <unistd.h>
-#endif
-
-#include <errno.h>
-
-namespace NYT::NDetail {
-
-using namespace NConcurrency;
-
-////////////////////////////////////////////////////////////////////////////////
-
-Y_WEAK void MaybeThrowSafeAssertionException(const char* /*message*/, int /*length*/)
-{
-    // A default implementation has no means of safety.
-    // Actual implementation lives in yt/yt/library/safe_assert.
-}
-
-void AssertTrapImpl(
-    TStringBuf trapType,
-    TStringBuf expr,
-    TStringBuf file,
-    int line,
-    TStringBuf function)
-{
-    TRawFormatter<1024> formatter;
-    formatter.AppendString(trapType);
-    formatter.AppendString("(");
-    formatter.AppendString(expr);
-    formatter.AppendString(") at ");
-    formatter.AppendString(file);
-    formatter.AppendString(":");
-    formatter.AppendNumber(line);
-    if (function) {
-        formatter.AppendString(" in ");
-        formatter.AppendString(function);
-        formatter.AppendString("\n");
-    }
-
-    MaybeThrowSafeAssertionException(formatter.GetData(), formatter.GetBytesWritten());
-
-    HandleEintr(::write, 2, formatter.GetData(), formatter.GetBytesWritten());
-
-    NLogging::TLogManager::Get()->Shutdown();
-
-    YT_BUILTIN_TRAP();
-}
-
-////////////////////////////////////////////////////////////////////////////////
-
-} // namespace NYT::NDetail
diff --git a/yt/yt/core/misc/crash_handler.cpp b/yt/yt/core/misc/crash_handler.cpp
index 80e657531e..fcc4d356c3 100644
--- a/yt/yt/core/misc/crash_handler.cpp
+++ b/yt/yt/core/misc/crash_handler.cpp
@@ -20,6 +20,8 @@
 
 #include <library/cpp/yt/system/handle_eintr.h>
 
+#include <library/cpp/yt/misc/tls.h>
+
 #ifdef _unix_
 #include <library/cpp/yt/backtrace/cursors/libunwind/libunwind_cursor.h>
 #else
@@ -76,6 +78,23 @@ void WriteToStderr(const char* buffer)
 
 namespace NDetail {
 
+constinit NThreading::TSpinLock CrashLock;
+constinit YT_DEFINE_THREAD_LOCAL(bool, CrashLockAcquiredByCurrentThread);
+
+void AcquireCrashLock()
+{
+    if (!std::exchange(CrashLockAcquiredByCurrentThread(), true)) {
+        CrashLock.Acquire();
+    }
+}
+
+void ReleaseCrashLock()
+{
+    if (std::exchange(CrashLockAcquiredByCurrentThread(), false)) {
+        CrashLock.Release();
+    }
+}
+
 Y_NO_INLINE TStackTrace GetStackTrace(TStackTraceBuffer* buffer)
 {
 #ifdef _unix_
@@ -264,7 +283,7 @@ const char* GetSignalCodeName(int signo, int code)
 // From include/asm/traps.h
 
 [[maybe_unused]]
-const char* GetTrapName(int trapno)
+const char* FindTrapName(int trapno)
 {
 #define XX(name, value, message) case value: return #name " (" message ")";
 
@@ -309,7 +328,8 @@ void FormatErrorCodeName(TBaseFormatter* formatter, int codeno)
      *   bit 4 ==               1: fault was an instruction fetch
      *   bit 5 ==               1: protection keys block access
      */
-    enum x86_pf_error_code {
+    enum x86_pf_error_code
+    {
         X86_PF_PROT  =   1 << 0,
         X86_PF_WRITE =   1 << 1,
         X86_PF_USER  =   1 << 2,
@@ -343,7 +363,6 @@ void DumpSignalInfo(siginfo_t* si)
 {
     TFormatter formatter;
 
-    formatter.AppendString("*** ");
     if (const char* name = GetSignalName(si->si_signo)) {
         formatter.AppendString(name);
     } else {
@@ -354,20 +373,20 @@ void DumpSignalInfo(siginfo_t* si)
     }
 
     formatter.AppendString(" (@0x");
-    formatter.AppendNumber(reinterpret_cast<uintptr_t>(si->si_addr), 16);
+    formatter.AppendNumber(reinterpret_cast<uintptr_t>(si->si_addr), /*radix*/ 16);
     formatter.AppendString(")");
     formatter.AppendString(" received by PID ");
     formatter.AppendNumber(getpid());
 
     formatter.AppendString(" (FID 0x");
-    formatter.AppendNumber(NConcurrency::GetCurrentFiberId(), 16);
+    formatter.AppendNumber(NConcurrency::GetCurrentFiberId(), /*radix*/ 16);
     formatter.AppendString(" TID 0x");
     // We assume pthread_t is an integral number or a pointer, rather
     // than a complex struct. In some environments, pthread_self()
     // returns an uint64 but in some other environments pthread_self()
     // returns a pointer. Hence we use C-style cast here, rather than
     // reinterpret/static_cast, to support both types of environments.
-    formatter.AppendNumber((uintptr_t)pthread_self(), 16);
+    formatter.AppendNumber(reinterpret_cast<uintptr_t>(pthread_self()), /*radix*/ 16);
     formatter.AppendString(") ");
     // Only linux has the PID of the signal sender in si_pid.
 #ifdef _unix_
@@ -381,10 +400,8 @@ void DumpSignalInfo(siginfo_t* si)
     } else {
         formatter.AppendNumber(si->si_code);
     }
-
-    formatter.AppendString(" ");
 #endif
-    formatter.AppendString("***\n");
+    formatter.AppendChar('\n');
 
     WriteToStderr(formatter);
 }
@@ -396,59 +413,52 @@ void DumpSigcontext(void* uc)
 
     TFormatter formatter;
 
-    formatter.AppendString("\nERR ");
+    formatter.AppendString("ERR    ");
     FormatErrorCodeName(&formatter, context->uc_mcontext.gregs[REG_ERR]);
+    formatter.AppendChar('\n');
 
-    formatter.AppendString("\nTRAPNO ");
-    if (const char* trapName = GetTrapName(context->uc_mcontext.gregs[REG_TRAPNO])) {
+    formatter.AppendString("TRAPNO ");
+    if (const char* trapName = FindTrapName(context->uc_mcontext.gregs[REG_TRAPNO])) {
         formatter.AppendString(trapName);
     } else {
         formatter.AppendString("0x");
         formatter.AppendNumber(context->uc_mcontext.gregs[REG_TRAPNO], 16);
     }
+    formatter.AppendChar('\n');
 
-    formatter.AppendString("\nR8 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R8], 16);
-    formatter.AppendString("\nR9 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R9], 16);
-    formatter.AppendString("\nR10 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R10], 16);
-    formatter.AppendString("\nR11 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R11], 16);
-    formatter.AppendString("\nR12 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R12], 16);
-    formatter.AppendString("\nR13 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R13], 16);
-    formatter.AppendString("\nR14 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R14], 16);
-    formatter.AppendString("\nR15 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_R15], 16);
-    formatter.AppendString("\nRDI 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RDI], 16);
-    formatter.AppendString("\nRSI 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RSI], 16);
-    formatter.AppendString("\nRBP 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RBP], 16);
-    formatter.AppendString("\nRBX 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RBX], 16);
-    formatter.AppendString("\nRDX 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RDX], 16);
-    formatter.AppendString("\nRAX 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RAX], 16);
-    formatter.AppendString("\nRCX 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RCX], 16);
-    formatter.AppendString("\nRSP 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RSP], 16);
-    formatter.AppendString("\nRIP 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_RIP], 16);
-    formatter.AppendString("\nEFL 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_EFL], 16);
-    formatter.AppendString("\nCSGSFS 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_CSGSFS], 16);
-    formatter.AppendString("\nOLDMASK 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_OLDMASK], 16);
-    formatter.AppendString("\nCR2 0x");
-    formatter.AppendNumber(context->uc_mcontext.gregs[REG_CR2], 16);
+    auto formatRegister = [&, horizontalPos = 0] (TStringBuf name, int reg) mutable {
+        if (horizontalPos > 1) {
+            formatter.AppendChar('\n');
+            horizontalPos = 0;
+        } else if (horizontalPos > 0) {
+            formatter.AppendChar(' ', 4);
+        }
+        formatter.AppendString(name);
+        formatter.AppendChar(' ', 7 - name.length());
+        formatter.AppendString("0x");
+        formatter.AppendNumber(context->uc_mcontext.gregs[reg], /*radix*/ 16, /*width*/ 16);
+        ++horizontalPos;
+    };
+    formatRegister("RAX", REG_RAX);
+    formatRegister("RBX", REG_RBX);
+    formatRegister("RCX", REG_RCX);
+    formatRegister("RDX", REG_RDX);
+    formatRegister("RSI", REG_RSI);
+    formatRegister("RDI", REG_RDI);
+    formatRegister("RBP", REG_RBP);
+    formatRegister("RSP", REG_RSP);
+    formatRegister("R8", REG_R8);
+    formatRegister("R9", REG_R9);
+    formatRegister("R10", REG_R10);
+    formatRegister("R11", REG_R11);
+    formatRegister("R12", REG_R12);
+    formatRegister("R13", REG_R13);
+    formatRegister("R14", REG_R14);
+    formatRegister("R15", REG_R15);
+    formatRegister("RIP", REG_RIP);
+    formatRegister("EFL", REG_EFL);
+    formatRegister("CR2", REG_CR2);
+    formatRegister("CSGSFS", REG_CSGSFS);
     formatter.AppendChar('\n');
 
     WriteToStderr(formatter);
@@ -493,6 +503,48 @@ void DumpUndumpableBlocksInfo()
 
 #endif
 
+Y_WEAK void MaybeThrowSafeAssertionException(TStringBuf /*message*/)
+{
+    // A default implementation has no means of safety.
+    // Actual implementation lives in yt/yt/library/safe_assert.
+}
+
+void AssertTrapImpl(
+    TStringBuf trapType,
+    TStringBuf expr,
+    TStringBuf file,
+    int line,
+    TStringBuf function)
+{
+    TRawFormatter<1024> formatter;
+    formatter.AppendString("*** ");
+    formatter.AppendString(trapType);
+    formatter.AppendString("(");
+    formatter.AppendString(expr);
+    formatter.AppendString(") at ");
+    formatter.AppendString(file);
+    formatter.AppendString(":");
+    formatter.AppendNumber(line);
+    if (function) {
+        formatter.AppendString(" in ");
+        formatter.AppendString(function);
+        formatter.AppendString("\n");
+    }
+
+    MaybeThrowSafeAssertionException(formatter.GetBuffer());
+
+    // Prevent clashes in stderr.
+    AcquireCrashLock();
+
+    WriteToStderr(formatter.GetBuffer());
+
+    // This (hopefully) invokes CrashSignalHandler.
+    YT_BUILTIN_TRAP();
+
+    // Not expected to get here but anyway...
+    ReleaseCrashLock();
+}
+
 } // namespace NDetail
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -508,23 +560,13 @@ void CrashSignalHandler(int /*signal*/, siginfo_t* si, void* uc)
     ::signal(SIGALRM, NDetail::CrashTimeoutHandler);
     ::alarm(60);
 
-    // When did the crash happen?
+    // Prevent clashes in stderr.
+    NDetail::AcquireCrashLock();
+
     NDetail::DumpTimeInfo();
 
-    // Dump codicils.
     NDetail::DumpCodicils();
 
-    // Where did the crash happen?
-    {
-        std::array<const void*, 1> frames{NDetail::GetPC(uc)};
-        NBacktrace::SymbolizeBacktrace(
-            TRange(frames),
-            [] (TStringBuf info) {
-                info.SkipPrefix(" 1. ");
-                WriteToStderr(info);
-            });
-    }
-
     NDetail::DumpSignalInfo(si);
 
     NDetail::DumpSigcontext(uc);
@@ -534,6 +576,9 @@ void CrashSignalHandler(int /*signal*/, siginfo_t* si, void* uc)
 
     NDetail::DumpUndumpableBlocksInfo();
 
+    // Releasing the lock gives other threads a chance to yell at us.
+    NDetail::ReleaseCrashLock();
+
     WriteToStderr("*** Waiting for logger to shut down\n");
 
     NLogging::TLogManager::Get()->Shutdown();
diff --git a/yt/yt/core/ya.make b/yt/yt/core/ya.make
index a958bb1c6a..b7850148e7 100644
--- a/yt/yt/core/ya.make
+++ b/yt/yt/core/ya.make
@@ -115,7 +115,6 @@ SRCS(
     logging/zstd_compression.cpp
 
     misc/arithmetic_formula.cpp
-    GLOBAL misc/assert.cpp
     misc/backoff_strategy.cpp
     misc/bitmap.cpp
     misc/bit_packed_unsigned_vector.cpp
@@ -126,7 +125,7 @@ SRCS(
     misc/codicil.cpp
     misc/config.cpp
     misc/coro_pipe.cpp
-    misc/crash_handler.cpp
+    GLOBAL misc/crash_handler.cpp
     misc/digest.cpp
     misc/error.cpp
     misc/fs.cpp