Skip to content

Commit 1fc3b9e

Browse files
Fix macOS floating point corruption (#75467)
There is a race condition between activation signal handling and returning from a hardware exception handler on macOS. It shows up intermittently in the Regression/coreclr/GitHub_16833 test in the CI and I am able to repro it on my local mac once in several thousands of iterations of the test when running with GC stress C. It turned out the issue is caused by the order in which we set parts of the context in the thread when returning from the hardware exception handler. MacOS can only set the floating point and control / integer portions separately. We were setting the control / integer portion first and the floating point portion after that. In the race condition, the signal handling code in the macOS extracts the context that contains the new control registers, but the old floating point ones (which is the state of those in the PAL_DispatchException). The signal handler for the activation injection then gets executed and when it returns later to our managed code, the floating point registers get restored to the wrong values. The fix is to change the context setting to first set the floating point registers and then the control / integer portion of the context. Close #66568 Co-authored-by: Jan Vorlicek <janvorli@microsoft.com>
1 parent abaf6f4 commit 1fc3b9e

File tree

1 file changed

+59
-82
lines changed

1 file changed

+59
-82
lines changed

src/coreclr/pal/src/thread/context.cpp

Lines changed: 59 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,68 +1398,6 @@ CONTEXT_SetThreadContextOnPort(
13981398
mach_msg_type_number_t StateCount;
13991399
thread_state_flavor_t StateFlavor;
14001400

1401-
if (lpContext->ContextFlags & (CONTEXT_CONTROL|CONTEXT_INTEGER) & CONTEXT_AREA_MASK)
1402-
{
1403-
#ifdef HOST_AMD64
1404-
x86_thread_state64_t State;
1405-
StateFlavor = x86_THREAD_STATE64;
1406-
1407-
State.__rax = lpContext->Rax;
1408-
State.__rbx = lpContext->Rbx;
1409-
State.__rcx = lpContext->Rcx;
1410-
State.__rdx = lpContext->Rdx;
1411-
State.__rdi = lpContext->Rdi;
1412-
State.__rsi = lpContext->Rsi;
1413-
State.__rbp = lpContext->Rbp;
1414-
State.__rsp = lpContext->Rsp;
1415-
State.__r8 = lpContext->R8;
1416-
State.__r9 = lpContext->R9;
1417-
State.__r10 = lpContext->R10;
1418-
State.__r11 = lpContext->R11;
1419-
State.__r12 = lpContext->R12;
1420-
State.__r13 = lpContext->R13;
1421-
State.__r14 = lpContext->R14;
1422-
State.__r15 = lpContext->R15;
1423-
// State.ss = lpContext->SegSs;
1424-
State.__rflags = lpContext->EFlags;
1425-
State.__rip = lpContext->Rip;
1426-
State.__cs = lpContext->SegCs;
1427-
// State.ds = lpContext->SegDs_PAL_Undefined;
1428-
// State.es = lpContext->SegEs_PAL_Undefined;
1429-
State.__fs = lpContext->SegFs;
1430-
State.__gs = lpContext->SegGs;
1431-
#elif defined(HOST_ARM64)
1432-
arm_thread_state64_t State;
1433-
StateFlavor = ARM_THREAD_STATE64;
1434-
1435-
memcpy(&State.__x[0], &lpContext->X0, 29 * 8);
1436-
State.__cpsr = lpContext->Cpsr;
1437-
arm_thread_state64_set_fp(State, lpContext->Fp);
1438-
arm_thread_state64_set_sp(State, lpContext->Sp);
1439-
arm_thread_state64_set_lr_fptr(State, lpContext->Lr);
1440-
arm_thread_state64_set_pc_fptr(State, lpContext->Pc);
1441-
#else
1442-
#error Unexpected architecture.
1443-
#endif
1444-
1445-
StateCount = sizeof(State) / sizeof(natural_t);
1446-
1447-
do
1448-
{
1449-
MachRet = thread_set_state(Port,
1450-
StateFlavor,
1451-
(thread_state_t)&State,
1452-
StateCount);
1453-
}
1454-
while (MachRet == KERN_ABORTED);
1455-
1456-
if (MachRet != KERN_SUCCESS)
1457-
{
1458-
ASSERT("thread_set_state(THREAD_STATE) failed: %d\n", MachRet);
1459-
goto EXIT;
1460-
}
1461-
}
1462-
14631401
if (lpContext->ContextFlags & CONTEXT_ALL_FLOATING & CONTEXT_AREA_MASK)
14641402
{
14651403

@@ -1496,26 +1434,6 @@ CONTEXT_SetThreadContextOnPort(
14961434
#error Unexpected architecture.
14971435
#endif
14981436

1499-
// If we're setting only one of the floating point or extended registers (of which Mach supports only
1500-
// the xmm values) then we don't have values for the other set. This is a problem since Mach only
1501-
// supports setting both groups as a single unit. So in this case we'll need to fetch the current
1502-
// values first.
1503-
if ((lpContext->ContextFlags & CONTEXT_ALL_FLOATING) !=
1504-
CONTEXT_ALL_FLOATING)
1505-
{
1506-
mach_msg_type_number_t StateCountGet = StateCount;
1507-
MachRet = thread_get_state(Port,
1508-
StateFlavor,
1509-
(thread_state_t)&State,
1510-
&StateCountGet);
1511-
if (MachRet != KERN_SUCCESS)
1512-
{
1513-
ASSERT("thread_get_state(FLOAT_STATE) failed: %d\n", MachRet);
1514-
goto EXIT;
1515-
}
1516-
_ASSERTE(StateCountGet == StateCount);
1517-
}
1518-
15191437
if (lpContext->ContextFlags & CONTEXT_FLOATING_POINT & CONTEXT_AREA_MASK)
15201438
{
15211439
#ifdef HOST_AMD64
@@ -1569,6 +1487,65 @@ CONTEXT_SetThreadContextOnPort(
15691487
}
15701488
}
15711489

1490+
if (lpContext->ContextFlags & (CONTEXT_CONTROL|CONTEXT_INTEGER) & CONTEXT_AREA_MASK)
1491+
{
1492+
#ifdef HOST_AMD64
1493+
x86_thread_state64_t State;
1494+
StateFlavor = x86_THREAD_STATE64;
1495+
1496+
State.__rax = lpContext->Rax;
1497+
State.__rbx = lpContext->Rbx;
1498+
State.__rcx = lpContext->Rcx;
1499+
State.__rdx = lpContext->Rdx;
1500+
State.__rdi = lpContext->Rdi;
1501+
State.__rsi = lpContext->Rsi;
1502+
State.__rbp = lpContext->Rbp;
1503+
State.__rsp = lpContext->Rsp;
1504+
State.__r8 = lpContext->R8;
1505+
State.__r9 = lpContext->R9;
1506+
State.__r10 = lpContext->R10;
1507+
State.__r11 = lpContext->R11;
1508+
State.__r12 = lpContext->R12;
1509+
State.__r13 = lpContext->R13;
1510+
State.__r14 = lpContext->R14;
1511+
State.__r15 = lpContext->R15;
1512+
State.__rflags = lpContext->EFlags;
1513+
State.__rip = lpContext->Rip;
1514+
State.__cs = lpContext->SegCs;
1515+
State.__fs = lpContext->SegFs;
1516+
State.__gs = lpContext->SegGs;
1517+
#elif defined(HOST_ARM64)
1518+
arm_thread_state64_t State;
1519+
StateFlavor = ARM_THREAD_STATE64;
1520+
1521+
memcpy(&State.__x[0], &lpContext->X0, 29 * 8);
1522+
State.__cpsr = lpContext->Cpsr;
1523+
arm_thread_state64_set_fp(State, lpContext->Fp);
1524+
arm_thread_state64_set_sp(State, lpContext->Sp);
1525+
arm_thread_state64_set_lr_fptr(State, lpContext->Lr);
1526+
arm_thread_state64_set_pc_fptr(State, lpContext->Pc);
1527+
#else
1528+
#error Unexpected architecture.
1529+
#endif
1530+
1531+
StateCount = sizeof(State) / sizeof(natural_t);
1532+
1533+
do
1534+
{
1535+
MachRet = thread_set_state(Port,
1536+
StateFlavor,
1537+
(thread_state_t)&State,
1538+
StateCount);
1539+
}
1540+
while (MachRet == KERN_ABORTED);
1541+
1542+
if (MachRet != KERN_SUCCESS)
1543+
{
1544+
ASSERT("thread_set_state(THREAD_STATE) failed: %d\n", MachRet);
1545+
goto EXIT;
1546+
}
1547+
}
1548+
15721549
EXIT:
15731550
return MachRet;
15741551
}

0 commit comments

Comments
 (0)