summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMike McLaughlin <mikem@microsoft.com>2018-08-14 00:46:13 -0700
committerJan Vorlicek <janvorli@microsoft.com>2018-08-14 09:46:13 +0200
commitc509903171fedf0a4a6f9fffeb6e2b9820f537c2 (patch)
tree8f36e3b8d25b4089150b1a85df1f8e0af79ed485
parent821016dc0d94c08a40701f75a09a3d355c515579 (diff)
downloadcoreclr-c509903171fedf0a4a6f9fffeb6e2b9820f537c2.tar.gz
coreclr-c509903171fedf0a4a6f9fffeb6e2b9820f537c2.tar.bz2
coreclr-c509903171fedf0a4a6f9fffeb6e2b9820f537c2.zip
Code review feedback for the alternate stack changes (PR #19309). (#19476)
Fixed SOS plugin for core dumps.
-rw-r--r--src/ToolBox/SOS/lldbplugin/services.cpp4
-rw-r--r--src/pal/inc/pal.h6
-rw-r--r--src/pal/src/exception/signal.cpp13
-rw-r--r--src/pal/src/init/pal.cpp4
-rw-r--r--src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest1.cpp2
-rw-r--r--src/pal/tests/palsuite/exception_handling/pal_sxs/test1/dlltest2.cpp2
6 files changed, 12 insertions, 19 deletions
diff --git a/src/ToolBox/SOS/lldbplugin/services.cpp b/src/ToolBox/SOS/lldbplugin/services.cpp
index a0914baab3..f5ec14d08d 100644
--- 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 d474a7c978..97fee605e0 100644
--- 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 ff3096a48c..d0c0d696ec 100644
--- 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 9d0be758b0..5c75f93fab 100644
--- 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 8eb10c91ac..0b9d78fa8c 100644
--- 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 5b1d2781d1..1f17b81218 100644
--- 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();
}