Code review feedback for the alternate stack changes (PR dotnet/coreclr#19309). ...

Fixed SOS plugin for core dumps.

Commit migrated from https://github.com/dotnet/coreclr/commit/c509903171fedf0a4a6f9fffeb6e2b9820f537c2

diff --git a/src/coreclr/src/ToolBox/SOS/lldbplugin/services.cpp b/src/coreclr/src/ToolBox/SOS/lldbplugin/services.cpp
index a0914ba..f5ec14d 100644 (file)
--- a/src/coreclr/src/ToolBox/SOS/lldbplugin/services.cpp
+++ b/src/coreclr/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/coreclr/src/pal/inc/pal.h b/src/coreclr/src/pal/inc/pal.h
index d474a7c..97fee60 100644 (file)
--- a/src/coreclr/src/pal/inc/pal.h
+++ b/src/coreclr/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/coreclr/src/pal/src/exception/signal.cpp b/src/coreclr/src/pal/src/exception/signal.cpp
index ff3096a..d0c0d69 100644 (file)
--- a/src/coreclr/src/pal/src/exception/signal.cpp
+++ b/src/coreclr/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/coreclr/src/pal/src/init/pal.cpp b/src/coreclr/src/pal/src/init/pal.cpp
index 9d0be75..5c75f93 100644 (file)
--- a/src/coreclr/src/pal/src/init/pal.cpp
+++ b/src/coreclr/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/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp b/src/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp
index 8eb10c9..0b9d78f 100644 (file)
--- a/src/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp
+++ b/src/coreclr/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/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp b/src/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp
index 5b1d278..1f17b81 100644 (file)
--- a/src/coreclr/src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp
+++ b/src/coreclr/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();
 }