From 568b3b228b1c501f089efe282c20dc178e2f918c Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 17 Jul 2024 02:18:28 +0200 Subject: [PATCH 1/3] Fix mach hardware exception forwarding A recent change to enable AVX512 register state processing in the mach exception handling on macOS x64 has broken a PAL test and also a case when a hardware exception occurs in 3rd party native code. In both cases runtime hangs, another exception occurs while forwarding the exception message and that ends up happenning infinitely. The problem is caused by the fact that the MachMessageInfo has grown, since we've changed a field containing float state to contain avx512 state instead. But the buffer that stores the message is of fixed size of 1500 bytes and it is not sufficient. I have grown the buffer size to the necessary size, but there was another issue lurking behind the first one. The MachExceptionInfo::RestoreState was trying to restore the float state always as x86_FLOAT_STATE instead of choosing x86_FLOAT_STATE, x86_AVX_STATE or x86_AVX512_STATE depending of which of them was stored. So the thread_set_state was failing. This change fixes both of the problems. --- src/coreclr/pal/src/exception/machexception.cpp | 17 +++++++++++++++-- src/coreclr/pal/src/exception/machmessage.h | 5 ++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/coreclr/pal/src/exception/machexception.cpp b/src/coreclr/pal/src/exception/machexception.cpp index eaf783a233117..9ea7649dd650f 100644 --- a/src/coreclr/pal/src/exception/machexception.cpp +++ b/src/coreclr/pal/src/exception/machexception.cpp @@ -1325,8 +1325,21 @@ void MachExceptionInfo::RestoreState(mach_port_t thread) kern_return_t machret = thread_set_state(thread, x86_THREAD_STATE, (thread_state_t)&ThreadState, x86_THREAD_STATE_COUNT); CHECK_MACH("thread_set_state(thread)", machret); - machret = thread_set_state(thread, x86_FLOAT_STATE, (thread_state_t)&FloatState, x86_FLOAT_STATE_COUNT); - CHECK_MACH("thread_set_state(float)", machret); + switch (FloatState.ash.flavor) + { + case x86_FLOAT_STATE: + machret = thread_set_state(thread, x86_FLOAT_STATE, (thread_state_t)&FloatState, x86_FLOAT_STATE_COUNT); + CHECK_MACH("thread_set_state(float)", machret); + break; + case x86_AVX_STATE: + machret = thread_set_state(thread, x86_AVX_STATE, (thread_state_t)&FloatState, x86_AVX_STATE_COUNT); + CHECK_MACH("thread_set_state(avx)", machret); + break; + case x86_AVX512_STATE: + machret = thread_set_state(thread, x86_AVX512_STATE, (thread_state_t)&FloatState, x86_AVX512_STATE_COUNT); + CHECK_MACH("thread_set_state(avx512)", machret); + break; + } machret = thread_set_state(thread, x86_DEBUG_STATE, (thread_state_t)&DebugState, x86_DEBUG_STATE_COUNT); CHECK_MACH("thread_set_state(debug)", machret); diff --git a/src/coreclr/pal/src/exception/machmessage.h b/src/coreclr/pal/src/exception/machmessage.h index 194f066dce4cd..033a686685b7c 100644 --- a/src/coreclr/pal/src/exception/machmessage.h +++ b/src/coreclr/pal/src/exception/machmessage.h @@ -189,7 +189,7 @@ class MachMessage // The maximum size in bytes of any Mach message we can send or receive. Calculating an exact size for // this is non trivial (basically because of the security trailers that Mach appends) but the current // value has proven to be more than enough so far. - static const size_t kcbMaxMessageSize = 1500; + static const size_t kcbMaxMessageSize = 0x1500; // The following are structures describing the formats of the Mach messages we understand. @@ -298,6 +298,9 @@ class MachMessage } data; } __attribute__((packed));; + + static_assert_no_msg(sizeof(mach_message_t) <= MachMessage::kcbMaxMessageSize); + // Re-initializes this data structure (to the same state as default construction, containing no message). void ResetMessage(); From fee489b03374f61f65e5b501502ab4cc73b0d996 Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 17 Jul 2024 08:35:58 +0200 Subject: [PATCH 2/3] Simplify the RestoreState --- src/coreclr/pal/src/exception/machexception.cpp | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/coreclr/pal/src/exception/machexception.cpp b/src/coreclr/pal/src/exception/machexception.cpp index 9ea7649dd650f..e3fd0de760ddb 100644 --- a/src/coreclr/pal/src/exception/machexception.cpp +++ b/src/coreclr/pal/src/exception/machexception.cpp @@ -1325,21 +1325,8 @@ void MachExceptionInfo::RestoreState(mach_port_t thread) kern_return_t machret = thread_set_state(thread, x86_THREAD_STATE, (thread_state_t)&ThreadState, x86_THREAD_STATE_COUNT); CHECK_MACH("thread_set_state(thread)", machret); - switch (FloatState.ash.flavor) - { - case x86_FLOAT_STATE: - machret = thread_set_state(thread, x86_FLOAT_STATE, (thread_state_t)&FloatState, x86_FLOAT_STATE_COUNT); - CHECK_MACH("thread_set_state(float)", machret); - break; - case x86_AVX_STATE: - machret = thread_set_state(thread, x86_AVX_STATE, (thread_state_t)&FloatState, x86_AVX_STATE_COUNT); - CHECK_MACH("thread_set_state(avx)", machret); - break; - case x86_AVX512_STATE: - machret = thread_set_state(thread, x86_AVX512_STATE, (thread_state_t)&FloatState, x86_AVX512_STATE_COUNT); - CHECK_MACH("thread_set_state(avx512)", machret); - break; - } + machret = thread_set_state(thread, FloatState.ash.flavor, (thread_state_t)&FloatState, FloatState.ash.count); + CHECK_MACH("thread_set_state(float)", machret); machret = thread_set_state(thread, x86_DEBUG_STATE, (thread_state_t)&DebugState, x86_DEBUG_STATE_COUNT); CHECK_MACH("thread_set_state(debug)", machret); From 9f38c1835c713dbccde17476ed4d75f4f615218f Mon Sep 17 00:00:00 2001 From: Jan Vorlicek Date: Wed, 17 Jul 2024 17:33:34 +0200 Subject: [PATCH 3/3] Set the message buffer to the size of the mach_msg_t plus trailer size --- src/coreclr/pal/src/exception/machmessage.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/pal/src/exception/machmessage.h b/src/coreclr/pal/src/exception/machmessage.h index 033a686685b7c..e82e1c808d88f 100644 --- a/src/coreclr/pal/src/exception/machmessage.h +++ b/src/coreclr/pal/src/exception/machmessage.h @@ -186,11 +186,6 @@ class MachMessage void ReplyToNotification(MachMessage& message, kern_return_t eResult); private: - // The maximum size in bytes of any Mach message we can send or receive. Calculating an exact size for - // this is non trivial (basically because of the security trailers that Mach appends) but the current - // value has proven to be more than enough so far. - static const size_t kcbMaxMessageSize = 0x1500; - // The following are structures describing the formats of the Mach messages we understand. // Request to set the register context on a particular thread. @@ -298,8 +293,8 @@ class MachMessage } data; } __attribute__((packed));; - - static_assert_no_msg(sizeof(mach_message_t) <= MachMessage::kcbMaxMessageSize); + // The maximum size in bytes of any Mach message we can send or receive including possible trailers + static const size_t kcbMaxMessageSize = sizeof(mach_message_t) + MAX_TRAILER_SIZE; // Re-initializes this data structure (to the same state as default construction, containing no message). void ResetMessage();