diff --git a/eeschema/sim/ngspice.cpp b/eeschema/sim/ngspice.cpp index bde54a63b7..3b7c16df76 100644 --- a/eeschema/sim/ngspice.cpp +++ b/eeschema/sim/ngspice.cpp @@ -44,6 +44,16 @@ #include #include +#include +#ifdef __WINDOWS__ +#ifndef NOMINMAX +#define NOMINMAX +#endif +#include +#else +#include +#endif + /** * Flag to enable debug output of Ngspice simulator. @@ -332,6 +342,10 @@ bool NGSPICE::LoadNetlist( const std::string& aNetlist ) bool NGSPICE::Run() { LOCALE_IO toggle; // ngspice works correctly only with C locale + + // Install signal handlers to catch ngspice crashes in the background thread + installSignalHandlers(); + return Command( "bg_run" ); // bg_* commands execute in a separate thread } @@ -339,12 +353,51 @@ bool NGSPICE::Run() bool NGSPICE::Stop() { LOCALE_IO c_locale; // ngspice works correctly only with C locale - return Command( "bg_halt" ); // bg_* commands execute in a separate thread + bool result = Command( "bg_halt" ); // bg_* commands execute in a separate thread + + // Restore signal handlers when simulation is stopped + restoreSignalHandlers(); + + return result; } bool NGSPICE::IsRunning() { + // Check if ngspice crashed while running in the background + if( s_crashed.load() ) + { + int signal = s_crashSignal.load(); + s_crashed.store( false ); + s_crashSignal.store( 0 ); + m_error = true; + + // Restore signal handlers after a crash + restoreSignalHandlers(); + + // Report the crash to the user + if( m_reporter ) + { + wxString signalName; + + switch( signal ) + { + case SIGSEGV: signalName = wxT( "SIGSEGV (segmentation fault)" ); break; + case SIGABRT: signalName = wxT( "SIGABRT (abort)" ); break; + case SIGFPE: signalName = wxT( "SIGFPE (floating point exception)" ); break; + case SIGILL: signalName = wxT( "SIGILL (illegal instruction)" ); break; + default: signalName = wxString::Format( wxT( "signal %d" ), signal ); break; + } + + m_reporter->Report( wxString::Format( + _( "Simulation crashed (%s). This is usually caused by a bug in ngspice " + "or an invalid netlist. The simulator will be reset." ), + signalName ) ); + } + + return false; + } + // No need to use C locale here return m_ngSpice_Running(); } @@ -700,6 +753,10 @@ int NGSPICE::cbBGThreadRunning( NG_BOOL aFinished, int aId, void* aUser ) { NGSPICE* sim = reinterpret_cast( aUser ); + // Restore signal handlers when simulation finishes + if( aFinished ) + sim->restoreSignalHandlers(); + if( sim->m_reporter ) sim->m_reporter->OnSimStateChange( sim, aFinished ? SIM_IDLE : SIM_RUNNING ); @@ -735,3 +792,188 @@ void NGSPICE::Clean() bool NGSPICE::m_initialized = false; + +std::atomic NGSPICE::s_crashed( false ); +std::atomic NGSPICE::s_crashSignal( 0 ); +NGSPICE* NGSPICE::s_currentInstance = nullptr; + +#ifndef __WINDOWS__ +static struct sigaction s_oldSigSegv; +static struct sigaction s_oldSigAbrt; +static struct sigaction s_oldSigFpe; +static bool s_signalHandlersInstalled = false; +static pthread_t s_mainThread; + + +void NGSPICE::signalHandler( int aSignal ) +{ + // Only handle signals from background threads, not the main thread. + // This is a safety check to prevent catching crashes from the main application. + if( pthread_equal( pthread_self(), s_mainThread ) ) + { + // This is the main thread, re-raise with the original handler + struct sigaction* oldAction = nullptr; + + switch( aSignal ) + { + case SIGSEGV: oldAction = &s_oldSigSegv; break; + case SIGABRT: oldAction = &s_oldSigAbrt; break; + case SIGFPE: oldAction = &s_oldSigFpe; break; + default: break; + } + + if( oldAction && oldAction->sa_handler != SIG_DFL && oldAction->sa_handler != SIG_IGN ) + { + oldAction->sa_handler( aSignal ); + } + else + { + // Restore default handler and re-raise + signal( aSignal, SIG_DFL ); + raise( aSignal ); + } + + return; + } + + // We're in a background thread (likely ngspice's simulation thread). + // Mark that ngspice crashed so the main thread can handle it. + s_crashed.store( true ); + s_crashSignal.store( aSignal ); + + if( s_currentInstance ) + s_currentInstance->m_error = true; + + // Terminate just this thread. pthread_exit is not technically async-signal-safe, but + // it's the best option we have for terminating the ngspice thread without bringing + // down the whole process. Since the thread state is already corrupted from the crash, + // this is a best-effort recovery. + pthread_exit( nullptr ); +} + + +void NGSPICE::installSignalHandlers() +{ + if( s_signalHandlersInstalled ) + return; + + s_mainThread = pthread_self(); + s_currentInstance = this; + s_crashed.store( false ); + s_crashSignal.store( 0 ); + + struct sigaction newAction; + newAction.sa_handler = signalHandler; + sigemptyset( &newAction.sa_mask ); + newAction.sa_flags = 0; + + sigaction( SIGSEGV, &newAction, &s_oldSigSegv ); + sigaction( SIGABRT, &newAction, &s_oldSigAbrt ); + sigaction( SIGFPE, &newAction, &s_oldSigFpe ); + + s_signalHandlersInstalled = true; +} + + +void NGSPICE::restoreSignalHandlers() +{ + if( !s_signalHandlersInstalled ) + return; + + sigaction( SIGSEGV, &s_oldSigSegv, nullptr ); + sigaction( SIGABRT, &s_oldSigAbrt, nullptr ); + sigaction( SIGFPE, &s_oldSigFpe, nullptr ); + + s_currentInstance = nullptr; + s_signalHandlersInstalled = false; +} +#else +static bool s_exceptionHandlersInstalled = false; +static PVOID s_vectoredHandler = nullptr; +static DWORD s_mainThreadId = 0; + +long __stdcall NGSPICE::sehHandler( _EXCEPTION_POINTERS* aException ) +{ + if( !aException || !aException->ExceptionRecord ) + return EXCEPTION_CONTINUE_SEARCH; + + if( GetCurrentThreadId() == s_mainThreadId ) + return EXCEPTION_CONTINUE_SEARCH; + + int signal = 0; + + switch( aException->ExceptionRecord->ExceptionCode ) + { + case EXCEPTION_ACCESS_VIOLATION: + case EXCEPTION_ARRAY_BOUNDS_EXCEEDED: + case EXCEPTION_DATATYPE_MISALIGNMENT: + case EXCEPTION_STACK_OVERFLOW: + signal = SIGSEGV; + break; + case EXCEPTION_ILLEGAL_INSTRUCTION: + case EXCEPTION_PRIV_INSTRUCTION: + signal = SIGILL; + break; + case EXCEPTION_INT_DIVIDE_BY_ZERO: + case EXCEPTION_INT_OVERFLOW: + case EXCEPTION_FLT_DIVIDE_BY_ZERO: + case EXCEPTION_FLT_INVALID_OPERATION: + case EXCEPTION_FLT_OVERFLOW: + case EXCEPTION_FLT_UNDERFLOW: + case EXCEPTION_FLT_INEXACT_RESULT: + case EXCEPTION_FLT_STACK_CHECK: + signal = SIGFPE; + break; + default: + return EXCEPTION_CONTINUE_SEARCH; + } + + s_crashed.store( true ); + s_crashSignal.store( signal ); + + if( s_currentInstance ) + s_currentInstance->m_error = true; + + // Best-effort termination of the crashing thread to keep KiCad alive. + if( aException->ExceptionRecord->ExceptionCode == EXCEPTION_STACK_OVERFLOW ) + TerminateThread( GetCurrentThread(), 1 ); + else + ExitThread( 1 ); + + return EXCEPTION_CONTINUE_EXECUTION; +} + +// Windows implementations +void NGSPICE::signalHandler( int aSignal ) +{ + wxUnusedVar( aSignal ); +} + + +void NGSPICE::installSignalHandlers() +{ + if( s_exceptionHandlersInstalled ) + return; + + s_mainThreadId = GetCurrentThreadId(); + s_currentInstance = this; + s_crashed.store( false ); + s_crashSignal.store( 0 ); + + s_vectoredHandler = AddVectoredExceptionHandler( 1, &NGSPICE::sehHandler ); + s_exceptionHandlersInstalled = ( s_vectoredHandler != nullptr ); +} + + +void NGSPICE::restoreSignalHandlers() +{ + if( s_exceptionHandlersInstalled ) + { + RemoveVectoredExceptionHandler( s_vectoredHandler ); + s_vectoredHandler = nullptr; + s_exceptionHandlersInstalled = false; + } + + s_currentInstance = nullptr; +} +#endif diff --git a/eeschema/sim/ngspice.h b/eeschema/sim/ngspice.h index 0c07b4b7be..4ffa93ba3e 100644 --- a/eeschema/sim/ngspice.h +++ b/eeschema/sim/ngspice.h @@ -37,6 +37,12 @@ #include +#include + +#if defined(__WINDOWS__) +struct _EXCEPTION_POINTERS; +#endif + // We have an issue here where NGSPICE incorrectly used bool for years // and defined it to be int when in C-mode. We cannot adjust the function // signatures without re-writing sharedspice.h for KiCad. @@ -188,12 +194,30 @@ private: // Assure ngspice is in a valid state and reinitializes it if need be. void validate(); + // Install signal handlers to catch ngspice crashes + void installSignalHandlers(); + + // Restore original signal handlers + void restoreSignalHandlers(); + + // Signal handler for crashes + static void signalHandler( int aSignal ); + +#if defined(__WINDOWS__) + // Structured exception handler for Windows crashes + static long __stdcall sehHandler( struct _EXCEPTION_POINTERS* aException ); +#endif + private: bool m_error; ///< Error flag indicating that ngspice needs to be reloaded. static bool m_initialized; ///< Ngspice should be initialized only once. std::string m_netlist; ///< Current netlist + + static std::atomic s_crashed; ///< Set by signal handler when ngspice crashes. + static std::atomic s_crashSignal; ///< Signal that caused the crash. + static NGSPICE* s_currentInstance; ///< Instance that is currently running ngspice. }; #endif /* NGSPICE_H */