Skip to content

Commit

Permalink
Work around a Linux arm32 unaligned data issue in SuperPMI
Browse files Browse the repository at this point in the history
impImportStaticReadOnlyField reads the data at the address returned by
getFieldAddress. SuperPMI saves and replays this data, but it doesn't
store it at a naturally aligned address. For int64/double (at least),
this is a problem on Linux arm32, which raises a SIGBUS exception on
such unaligned access.

This works around the problem by copying the potentially unaligned data
to a known aligned spot before reading it. This is only done under DEBUG
and when we know we are replaying under SuperPMI.

A proper fix would be to teach SuperPMI about alignment, but that would be
a large and risky change, compared to this small and isolated workaround.

The fixes the non-determinism of dotnet#56156.
  • Loading branch information
BruceForstall committed Jul 29, 2021
1 parent 10f38d2 commit 947d268
Showing 1 changed file with 57 additions and 0 deletions.
57 changes: 57 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7531,6 +7531,63 @@ GenTree* Compiler::impImportStaticReadOnlyField(void* fldAddr, var_types lclTyp)
{
GenTree* op1 = nullptr;

#if defined(DEBUG)
// If we're replaying under SuperPMI, we're going to read the data stored by SuperPMI and use it
// for optimization. Unfortunately, SuperPMI doesn't implement a guarantee on the alignment of
// this data, so for some platforms which don't allow unaligned access (e.g., Linux arm32),
// this can fault. We should fix SuperPMI to guarantee alignment, but that is a big change.
// Instead, simply fix up the data here for future use.

// This variable should be the largest size element, with the largest alignment requirement,
// and the native C++ compiler should guarantee sufficient alignment.
double aligned_data = 0.0;
void* p_aligned_data = &aligned_data;
if (info.compMethodSuperPMIIndex != -1)
{
switch (lclTyp)
{
case TYP_BOOL:
case TYP_BYTE:
case TYP_UBYTE:
static_assert_no_msg(sizeof(unsigned __int8) == sizeof(bool));
static_assert_no_msg(sizeof(unsigned __int8) == sizeof(signed char));
static_assert_no_msg(sizeof(unsigned __int8) == sizeof(unsigned char));
*(unsigned __int8*)p_aligned_data = *((unsigned __int8*)fldAddr);
break;

case TYP_SHORT:
case TYP_USHORT:
static_assert_no_msg(sizeof(unsigned __int16) == sizeof(short));
static_assert_no_msg(sizeof(unsigned __int16) == sizeof(unsigned short));
*(unsigned __int16*)p_aligned_data = GET_UNALIGNED_16(fldAddr);
break;

case TYP_INT:
case TYP_UINT:
case TYP_FLOAT:
static_assert_no_msg(sizeof(unsigned __int32) == sizeof(int));
static_assert_no_msg(sizeof(unsigned __int32) == sizeof(unsigned int));
static_assert_no_msg(sizeof(unsigned __int32) == sizeof(float));
*(unsigned __int32*)p_aligned_data = GET_UNALIGNED_32(fldAddr);
break;

case TYP_LONG:
case TYP_ULONG:
case TYP_DOUBLE:
static_assert_no_msg(sizeof(unsigned __int64) == sizeof(unsigned __int64));
static_assert_no_msg(sizeof(unsigned __int64) == sizeof(double));
*(unsigned __int64*)p_aligned_data = GET_UNALIGNED_64(fldAddr);
break;

default:
assert(!"Unexpected lclTyp");
break;
}

fldAddr = p_aligned_data;
}
#endif // DEBUG

switch (lclTyp)
{
int ival;
Expand Down

0 comments on commit 947d268

Please sign in to comment.