CoreCLR's debug/checked last error trashing leaks in non-P/Invoke scenarios

[jkoritzinsky]

CoreCLR has a macro named TRASH_LASTERROR in utilcode that is used to make it easier to detect when the "last system error code" is not preserved in P/Invoke scenarios.

However, it leaks out in a number of non-P/Invoke scenarios, which impacts some of the DllImportGenerator tests that use DNNE to make the logic of the native side of the generated code tests implemented in managed code. This causes test failures with a Debug/Checked runtime since the expected error code is not set since CoreCLR sets an error code value and leaves it set.

For example, it leaks out of coreclr_create_delegate's calls to StringToUnicode, as well as ComponentActivator's calls to Marshal.PtrToStringAuto.

[jkotas]

it leaks out of coreclr_create_delegate's calls to StringToUnicode, as well as ComponentActivator's calls to Marshal.PtrToStringAuto

Is there nothing else in these methods that would modify the last error on some paths in these cases? The last error is very volatile. You have to explicitly save it if you want to preserve it.

Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

---

CoreCLR has a macro named TRASH_LASTERROR in utilcode that is used to make it easier to detect when the "last system error code" is not preserved in P/Invoke scenarios.

However, it leaks out in a number of non-P/Invoke scenarios, which impacts some of the DllImportGenerator tests that use DNNE to make the logic of the native side of the generated code tests implemented in managed code. This causes test failures with a Debug/Checked runtime since the expected error code is not set since CoreCLR sets an error code value and leaves it set.

For example, it leaks out of coreclr_create_delegate's calls to StringToUnicode, as well as ComponentActivator's calls to Marshal.PtrToStringAuto.

Author:	jkoritzinsky
Assignees:	-
Labels:	area-Interop-coreclr, area-Host, untriaged
Milestone:	-

[AaronRobinsonMSFT]

The last error is very volatile. You have to explicitly save it if you want to preserve it.

Agree. We chatted about this issue offline. We thought it warranted an issue for tracking purposes when using the hosting APIs. The impetus for this was around using DNNE in a test scenario and is being addressed in AaronRobinsonMSFT/DNNE#87.

/cc @elinor-fung

[jkotas]

Is it really just a DNNE problem? I would expect you have to make fixes in the runtime as well to make this work 100%.

[AaronRobinsonMSFT]

Is it really just a DNNE problem?

I believe so. The issue here is DNNE discovers the export lazily so the initial call via a P/Invoke looks like:

Managed
    -> DNNE generated library
        -> Discover export
        -> Call export

Where as all subsequent ones look like the following:

Managed
    -> DNNE generated library
        -> Call export

For DNNE, preserving the current error code during "Discover export" should address the stated behavior. Are you thinking of another scenario where this could manifest?

[AaronRobinsonMSFT]

I think from the .NET Hosting API perspective, coreclr_create_delegate is doing the right thing. The "right thing" being it may impact the last error because it is a native call and the caller needs to handle that. That would mean callers like DNNE need to be sensitive to that behavior calls propagate from managed code.

[jkotas]

Do prologs and epilogs of UnmanagedCallersOnly methods preserve the last error in all situations?

[AaronRobinsonMSFT]

Do prologs and epilogs of UnmanagedCallersOnly methods preserve the last error in all situations?

I don't think it is and I believe we want that in this case. Or are you referring to the case where transitioning itself may cause last error to be impacted?

[jkotas]

I consider transitioning itself (e.g. the call to JIT_ReversePInvokeEnter helper) to be part of the prolog/epilog.

JIT_ReversePInvokeEnter/JIT_ReversePInvokeExit helpers do not seem to be guaranteed to preserve the last error on all paths. Also, there may be other helper calls generated by the JIT as part of the prolog/epilog that may be also altering the last error in some situations.