Code review feedback for the alternate stack changes (PR #19309). (#19476)

Fixed SOS plugin for core dumps.

diff --git a/src/ToolBox/SOS/lldbplugin/services.cpp b/src/ToolBox/SOS/lldbplugin/services.cpp
index a0914baab338bb61903907142705e9a933849d41..f5ec14d08d2d5cc41e4bdfcbd613943a92a071c3 100644 (file)
--- a/src/ToolBox/SOS/lldbplugin/services.cpp
+++ b/src/ToolBox/SOS/lldbplugin/services.cpp
@@ -771,7 +771,7 @@ exit:
     {
         *bytesRead = read;
     }
-    return error.Success() ? S_OK : E_FAIL;
+    return error.Success() || (read != 0) ? S_OK : E_FAIL;
 }
 
 HRESULT 
@@ -800,7 +800,7 @@ exit:
     {
         *bytesWritten = written;
     }
-    return error.Success() ? S_OK : E_FAIL;
+    return error.Success() || (written != 0) ? S_OK : E_FAIL;
 }
 
 //----------------------------------------------------------------------------

diff --git a/src/pal/inc/pal.h b/src/pal/inc/pal.h
index d474a7c9785055cca2a0f497e202ccafa5d0c60f..97fee605e008acd0bbd7463fb2e2f6e8ee30c446 100644 (file)
--- a/src/pal/inc/pal.h
+++ b/src/pal/inc/pal.h
@@ -322,7 +322,6 @@ typedef long time_t;
 #define PAL_INITIALIZE_DEBUGGER_EXCEPTIONS          0x10
 #define PAL_INITIALIZE_ENSURE_STACK_SIZE            0x20
 #define PAL_INITIALIZE_REGISTER_SIGNALS             0x40
-#define PAL_INITIALIZE_ENSURE_ALT_SIGNAL_STACK      0x80
 
 // PAL_Initialize() flags
 #define PAL_INITIALIZE                 (PAL_INITIALIZE_SYNC_THREAD | \
@@ -337,8 +336,7 @@ typedef long time_t;
                                         PAL_INITIALIZE_REGISTER_SIGTERM_HANDLER | \
                                         PAL_INITIALIZE_DEBUGGER_EXCEPTIONS | \
                                         PAL_INITIALIZE_ENSURE_STACK_SIZE | \
-                                        PAL_INITIALIZE_REGISTER_SIGNALS | \
-                                        PAL_INITIALIZE_ENSURE_ALT_SIGNAL_STACK)
+                                        PAL_INITIALIZE_REGISTER_SIGNALS)
 
 typedef DWORD (PALAPI *PTHREAD_START_ROUTINE)(LPVOID lpThreadParameter);
 typedef PTHREAD_START_ROUTINE LPTHREAD_START_ROUTINE;
@@ -355,7 +353,7 @@ PAL_Initialize(
 PALIMPORT
 void
 PALAPI
-PAL_SetInitializeFlags(
+PAL_InitializeWithFlags(
     DWORD flags);
 
 PALIMPORT

diff --git a/src/pal/src/exception/signal.cpp b/src/pal/src/exception/signal.cpp
index ff3096a48c93208cbed38c8448c8e35e446abcc7..d0c0d696ec277ce4dd6ad055b3d00720f30d6839 100644 (file)
--- a/src/pal/src/exception/signal.cpp
+++ b/src/pal/src/exception/signal.cpp
@@ -103,11 +103,10 @@ static void restore_signal(int signal_id, struct sigaction *previousAction);
 
 /* internal data declarations *********************************************/
 
-static bool ensure_alt_signal_stack = false;
-static bool registered_sigterm_handler = false;
 #if !HAVE_MACH_EXCEPTIONS
 static bool registered_signal_handlers = false;
 #endif // !HAVE_MACH_EXCEPTIONS
+static bool registered_sigterm_handler = false;
 
 struct sigaction g_previous_sigterm;
 #if !HAVE_MACH_EXCEPTIONS
@@ -147,7 +146,7 @@ BOOL EnsureSignalAlternateStack()
 {
     int st = 0;
 
-    if (ensure_alt_signal_stack)
+    if (registered_signal_handlers)
     {
         stack_t oss;
 
@@ -207,7 +206,7 @@ Return :
 --*/
 void FreeSignalAlternateStack()
 {
-    if (ensure_alt_signal_stack)
+    if (registered_signal_handlers)
     {
         stack_t ss, oss;
         // The man page for sigaltstack says that when the ss.ss_flags is set to SS_DISABLE,
@@ -245,6 +244,7 @@ BOOL SEHInitializeSignals(DWORD flags)
     if (flags & PAL_INITIALIZE_REGISTER_SIGNALS)
     {
         registered_signal_handlers = true;
+
         /* we call handle_signal for every possible signal, even
            if we don't provide a signal handler.
 
@@ -273,11 +273,6 @@ BOOL SEHInitializeSignals(DWORD flags)
 #ifdef INJECT_ACTIVATION_SIGNAL
         handle_signal(INJECT_ACTIVATION_SIGNAL, inject_activation_handler, &g_previous_activation);
 #endif
-    }
-
-    if (flags & PAL_INITIALIZE_ENSURE_ALT_SIGNAL_STACK)
-    {
-        ensure_alt_signal_stack = true;
         if (!EnsureSignalAlternateStack())
         {
             return FALSE;

diff --git a/src/pal/src/init/pal.cpp b/src/pal/src/init/pal.cpp
index 9d0be758b04f4d0683435482b509413b5779775a..5c75f93fabdd91d6acbb8d2c62776818d7d6128d 100644 (file)
--- a/src/pal/src/init/pal.cpp
+++ b/src/pal/src/init/pal.cpp
@@ -158,7 +158,7 @@ PAL_Initialize(
 
 /*++
 Function:
-  PAL_InitializeFlags
+  PAL_InitializeWithFlags
 
 Abstract:
   This function is the first function of the PAL to be called.
@@ -172,7 +172,7 @@ Return:
 --*/
 int
 PALAPI
-PAL_InitializeFlags(
+PAL_InitializeWithFlags(
     int argc,
     const char *const argv[],
     DWORD flags)

diff --git a/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp b/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp
index 8eb10c91ac74e531206276964c56e20e91788877..0b9d78fa8cc4b3d4939fdcfdda0ce519434f20ac 100644 (file)
--- a/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp
+++ b/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp
@@ -17,7 +17,7 @@
 extern "C"
 int InitializeDllTest1()
 {
-    PAL_SetInitializeDLLFlags(PAL_INITIALIZE_DLL | PAL_INITIALIZE_REGISTER_SIGNALS | PAL_INITIALIZE_ENSURE_ALT_SIGNAL_STACK);
+    PAL_SetInitializeDLLFlags(PAL_INITIALIZE_DLL | PAL_INITIALIZE_REGISTER_SIGNALS);
     return PAL_InitializeDLL();
 }
 

diff --git a/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp b/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp
index 5b1d2781d1d23b5f0825918a4daffdfbd2732301..1f17b81218487d810ae5ee225db77927346fb61e 100644 (file)
--- a/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp
+++ b/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp
@@ -17,7 +17,7 @@
 extern "C"
 int InitializeDllTest2()
 {
-    PAL_SetInitializeDLLFlags(PAL_INITIALIZE_DLL | PAL_INITIALIZE_REGISTER_SIGNALS | PAL_INITIALIZE_ENSURE_ALT_SIGNAL_STACK);
+    PAL_SetInitializeDLLFlags(PAL_INITIALIZE_DLL | PAL_INITIALIZE_REGISTER_SIGNALS);
     return PAL_InitializeDLL();
 }