Access Violation through reflection not caught as NRE on .NET 6 #61486
Assignees
Labels
area-CodeGen-coreclr
CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
bug
Milestone
Comments
dotnet-issue-labeler
bot
added
area-System.Reflection
untriaged
|
Tagging subscribers to this area: @buyaa-n Issue DetailsDescriptionFor some reason, reading a simple tiny property via reflection that should NRE instead crashes the process with an AV (seems the runtime isn't turning it into an NRE correctly?). This start happening with .NET 6 (we migrated two days ago, from .NET 5) and only seems to happen through reflection We have a debugging tool in our game called VV (view variables) which allows us to inspect and modify the fields/properties on objects. Outside of being networked (server connection) it just uses basic reflection stuff to find all properties/fields tagged with The problematic code (that's being invoked and causes the crash) is this tiny property. Yeah, it's that simple. There's no IL rewriting, unsafe optimizations, anything going on. Rickbrew on Discord suggested setting Reproduction StepsI tried reproducing this in a tiny test project and couldn't get it to happen. It is consistent in our game. I understand these aren't very lightweight instructions but anyways:
var c = new TagComponent(); // Literally any component instance works here. These components have no constructors, nothing funny here.
c.GetType().GetProperty("OwnerUid").GetValue(c); // Will cause AV.I tried debugging this in SOS but didn't really know what I was doing all that much. Tell me if I need to do something specific). Expected behaviorThe runtime should simply throw an NRE which our code will handle (admittedly our VV code doesn't handle it nicely but the packet handler catches it so it won't crash the whole process like it does now). Actual behaviorThe runtime seems to fail to detect the AV as an NRE and crashes the process. Regression?Yes, I rolled back to .NET 5 (we switched .NET 6 two days ago) and it stopped happening. The exception is correctly thrown as an NRE and caught. Known WorkaroundsNo response Configuration.NET Runtime: 6.0.0 Other informationNo response
|
|
@PJB3005 did you happen to get a stack trace when debugging? including the native frames. |
reporting in on GitHub |
From windbg/SOS: The stack from Also this appears to be opt tier related, if I run the csi code snippet ( I guess I should also mention we use |
|
|
This looks like the first chance exception. There are likely subsequent exceptions that are actually causing the crash. Can you try continuing in the debugger ( |
|
Second exception: |
|
Is there third or fourth exception after that? |
|
No, that's the last of it. |
|
Here is a small repro - compile and run with optimizations on: var i = new My(new My(null));
var m = i.GetType().GetMethod("M");
for (;;)
{
try
{
m.Invoke(i, null);
}
catch
{
}
}
public interface IFace
{
void M();
}
public class My : IFace
{
IFace _face;
public My(IFace face)
{
_face = face;
}
public void M() => _face.M();
}Expected behavior: Runs forever |
|
@janvorli Could you please take a look? It looks like a bug in stackwalking from tail-called VSD stub. |
|
@jkotas thank you for the simple repro. I am taking a look now. |
|
The culprit is quite nasty, as it breaks our assumption that when a NRE happens in the VSD stub, we can simply manually unwind to the caller to get to a managed frame. In this problematic case, the direct caller based on the stack trace is the |
|
I was thinking about it more and I have come to a conclusion that we should rather fix this in the JIT and let it generate null check instruction before tail calling the VSD. The reason is diagnosability of such exceptions. If I made it work only by runtime changes, then the real location of the NullReferenceException would be lost, as when the VSD stub is entered via tail call and we try to access the null reference, the frame of the managed function that invoked it is not on the stack anymore. |
I agree. |
jkotas
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsDescriptionFor some reason, reading a simple tiny property via reflection that should NRE instead crashes the process with an AV (seems the runtime isn't turning it into an NRE correctly?). This start happening with .NET 6 (we migrated two days ago, from .NET 5) and only seems to happen through reflection We have a debugging tool in our game called VV (view variables) which allows us to inspect and modify the fields/properties on objects. Outside of being networked (server connection) it just uses basic reflection stuff to find all properties/fields tagged with The problematic code (that's being invoked and causes the crash) is this tiny property. The objects in question being looked at do have a Rickbrew on Discord suggested setting Reproduction StepsI tried reproducing this in a tiny test project and couldn't get it to happen, in fact a separate test game project on our own (same) engine doesn't even trigger it. It is consistent in our game however. I understand these aren't very lightweight instructions but anyways:
var c = new TransformComponent(); // Literally any component instance works here. These components have no constructors, nothing funny here.
c.GetType().GetProperty("OwnerUid").GetValue(c); // Will cause AV.I tried debugging this in SOS but didn't really know what I was doing all that much. Tell me if I need to do something specific). I can supply a dump if necessary. Expected behaviorThe runtime should simply throw an NRE which our code will handle (admittedly our VV code doesn't handle it nicely but the packet handler catches it so it won't crash the whole process like it does now). Actual behaviorThe runtime seems to fail to detect the AV as an NRE and crashes the process. Regression?Yes, I rolled back to .NET 5 (we switched .NET 6 two days ago) and it stopped happening. The exception is correctly thrown as an NRE and caught. Known WorkaroundsNo response Configuration.NET Runtime: 6.0.0 Other informationNo response
|
|
@jakobbotsch Could you please take a look? It is in the area you have been working on. |
|
I'm OOF next week, but I will take a look once I am back. Removed the regression badge since the reduced example does seem to repro on .NET 5 as well. |
jakobbotsch
added a commit
to jakobbotsch/runtime
that referenced
this issue
Dec 13, 2021
There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. Fix dotnet#61486
ghost
added
the
in-pr
jkotas
pushed a commit
that referenced
this issue
Dec 14, 2021
There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. Fix #61486
ghost
removed
the
in-pr
github-actions bot
pushed a commit
that referenced
this issue
Dec 14, 2021
There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. Fix #61486
jeffschwMSFT
pushed a commit
that referenced
this issue
Jan 7, 2022
* Add explicit null-check for tailcalls to VSD There is already a comment that this is necessary, but it is only being done for x86 tailcalls via jit helper. Do it for normal tailcalls to VSD as well. Fix #61486 * Revert cleanup to make potential backport easier Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Description
For some reason, reading a simple tiny property via reflection that should NRE instead crashes the process with an AV (seems the runtime isn't turning it into an NRE correctly?). This start happening with .NET 6 (we migrated two days ago, from .NET 5) and only seems to happen through reflection
GetValue()/Invoke(). It's also specific to release mode (debug works fine) and happens consistently on both Linux/Windows.We have a debugging tool in our game called VV (view variables) which allows us to inspect and modify the fields/properties on objects. Outside of being networked (server connection) it just uses basic reflection stuff to find all properties/fields tagged with
[ViewVariables]and uses.GetValue()to view their value, show them nicely, etc... We noticed that this hard crashed the server when looking at certain objects. Accessing the property directly (at least via C# interactive) works fine. Accessing it via reflection crashes (it's not specific to our VV tool).The problematic code (that's being invoked and causes the crash) is this tiny property. The objects in question being looked at do have a
nullOwner, so an NRE is expected. Yeah, it's that simple. There's no IL rewriting, unsafe optimizations, anything going on.Rickbrew on Discord suggested setting
COMPlus_legacyCorruptedStateExceptionsPolicy=1. If I do that the AV gets thrown asAccessViolationExceptionbut can at least be caught with a try catch (it's wrapped in anInvocationTargetExceptionof course). I don't know how relevant that is.Ræin on Discord suggested using a function pointer like so:
((delegate*<[YourObjectType], EntityUid>)property.GetMethod.Handle.GetFunctionPointer())(Object). This worked fine (NRE thrown and caught). Other things they suggested likeproperty.GetMethod.Invoke(...)exhibit the same AV behavior.Reproduction Steps
I tried reproducing this in a tiny test project and couldn't get it to happen, in fact a separate test game project on our own (same) engine doesn't even trigger it. It is consistent in our game however. I understand these aren't very lightweight instructions but anyways:
git submodule update --init --recursivedotnet run -c Releasein theContent.Clientprojectdotnet run -c Releasein theContent.Serverproject to start the servercsifor C# interactive console.I tried debugging this in SOS but didn't really know what I was doing all that much. Tell me if I need to do something specific). I can supply a dump if necessary.
Expected behavior
The runtime should simply throw an NRE which our code will handle (admittedly our VV code doesn't handle it nicely but the packet handler catches it so it won't crash the whole process like it does now).
Actual behavior
The runtime seems to fail to detect the AV as an NRE and crashes the process.
Regression?
Yes, I rolled back to .NET 5 (we switched .NET 6 two days ago) and it stopped happening. The exception is correctly thrown as an NRE and caught.
Known Workarounds
No response
Configuration
.NET Runtime: 6.0.0
OS: Windows 10.0.19042 for my dev machine, but it also happens on our Linux servers.
Architecture: x64
Specific: Happens on Linux and Windows, does not happen on .NET 5, only happens on
Releasebuild configuration (not Debug).Other information
No response
The text was updated successfully, but these errors were encountered: