Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix reporting illegal instruction exception #107414

Merged
merged 4 commits into from
Sep 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,11 @@ internal static Exception GetClasslibException(ExceptionIDs id, IntPtr address)
ExceptionIDs.NullReference => new NullReferenceException(),
ExceptionIDs.OutOfMemory => new OutOfMemoryException(),
ExceptionIDs.Overflow => new OverflowException(),
#pragma warning disable CS0618 // ExecutionEngineException is obsolete
ExceptionIDs.IllegalInstruction => new ExecutionEngineException("Illegal instruction: Attempted to execute an instruction code not defined by the processor."),
ExceptionIDs.PrivilegedInstruction => new ExecutionEngineException("Privileged instruction: Attempted to execute an instruction code that cannot be executed in user mode."),
ExceptionIDs.InPageError => new ExecutionEngineException("In page error: Attempted to access a memory page that is not present, and the system is unable to load the page. For example, this exception might occur if a network connection is lost while running a program over a network."),
#pragma warning restore CS0618
_ => null
};
#endif
Expand Down Expand Up @@ -445,8 +450,11 @@ private enum HwExceptionCode : uint

STATUS_DATATYPE_MISALIGNMENT = 0x80000002u,
STATUS_ACCESS_VIOLATION = 0xC0000005u,
STATUS_IN_PAGE_ERROR = 0xC0000006u,
STATUS_ILLEGAL_INSTRUCTION = 0xC000001Du,
STATUS_INTEGER_DIVIDE_BY_ZERO = 0xC0000094u,
STATUS_INTEGER_OVERFLOW = 0xC0000095u,
STATUS_PRIVILEGED_INSTRUCTION = 0xC0000096u,
}
[StructLayout(LayoutKind.Explicit, Size = AsmOffsets.SIZEOF__PAL_LIMITED_CONTEXT)]
public struct PAL_LIMITED_CONTEXT
Expand Down Expand Up @@ -601,6 +609,18 @@ public static void RhThrowHwEx(uint exceptionCode, ref ExInfo exInfo)
exceptionId = ExceptionIDs.Overflow;
break;

case (uint)HwExceptionCode.STATUS_ILLEGAL_INSTRUCTION:
exceptionId = ExceptionIDs.IllegalInstruction;
break;

case (uint)HwExceptionCode.STATUS_IN_PAGE_ERROR:
exceptionId = ExceptionIDs.InPageError;
break;

case (uint)HwExceptionCode.STATUS_PRIVILEGED_INSTRUCTION:
exceptionId = ExceptionIDs.PrivilegedInstruction;
break;

default:
// We don't wrap SEH exceptions from foreign code like CLR does, so we believe that we
// know the complete set of HW faults generated by managed code and do not need to handle
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,8 @@ enum ExceptionIDs
DataMisaligned = 10,
EntrypointNotFound = 11,
AmbiguousImplementation = 12,
IllegalInstruction = 13,
PrivilegedInstruction = 14,
InPageError = 15,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@ internal enum ExceptionIDs
DataMisaligned = 10,
EntrypointNotFound = 11,
AmbiguousImplementation = 12,
IllegalInstruction = 13,
PrivilegedInstruction = 14,
InPageError = 15,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,18 @@ internal static class RuntimeExceptionHelpers
FailFast("Access Violation: Attempted to read or write protected memory. This is often an indication that other memory is corrupt. The application will be terminated since this platform does not support throwing an AccessViolationException.");
return null;

case ExceptionIDs.IllegalInstruction:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we want to create an exception on the NativeAOT or behave like in the case of Access violation and just failfast here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think PlarformNotSupportedException would be the best because it will allow us to have same behavior as CoreCLR for valid C#, but invalid use of hardware intrinsics.

For example:

using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.X86;

Sse41.Blend(Vector128<double>.One, default, default);

I'm not guarding this with Sse41.IsSupported and the generated code has an unguarded blendpd instruction. CoreCLR-JIT would generate a PNSE throw instead of the unguarded blendpd.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether we want these faults to be catchable exceptions. The JIT assumes that these instructions won't throw, and it may take shortcuts when generating GC info for this IP. Catching and handling this fault would produce a GC hole.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In coreclr, it is not catchable - the exception is created just for reporting the failure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether we want these faults to be catchable exceptions. The JIT assumes that these instructions won't throw, and it may take shortcuts when generating GC info for this IP. Catching and handling this fault would produce a GC hole.

👍

I think there's also a desire to differentiate between "the user called this API without checking, the JIT did the right thing" and "something in the JIT went wrong and an invalid instruction was emitted". In some cases it might even be invalid on all hardware, such as from a buggy encoding and not simply some encoding that would be valid on new enough hardware.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In coreclr, it is not catchable - the exception is created just for reporting the failure.

The exceptions should not be catchable in native aot either. I think it should look more like the access violation handling. I have tried the following with this change on Native AOT (my machine does not have AvxVnni support):
dotnet publish /p:PublishAot=true /p:IlcInstructionSet=avx2

using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.X86;

Console.WriteLine(AvxVnni.IsSupported);

try
{
    AvxVnni.MultiplyWideningAndAdd(default(Vector128<int>), default(Vector128<byte>), default(Vector128<sbyte>)).ToString();
}
catch (Exception e)
{
    Console.WriteLine("HANDLED!");
}

It prints:

False
HANDLED!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated it to fail fast on NativeAOT like we do for the access violation.

FailFast("Illegal instruction: Attempted to execute an instruction code not defined by the processor.");
return null;

case ExceptionIDs.PrivilegedInstruction:
FailFast("Privileged instruction: Attempted to execute an instruction code that cannot be executed in user mode.");
return null;

case ExceptionIDs.InPageError:
FailFast("In page error: Attempted to access a memory page that is not present, and the system is unable to load the page. For example, this exception might occur if a network connection is lost while running a program over a network.");
return null;

case ExceptionIDs.DataMisaligned:
return new DataMisalignedException();

Expand Down