Add Get/SetLastPInvokeError and Get/SetLastSystemError APIs

[elinor-fung]

Implements #46843

cc @AaronRobinsonMSFT @jkoritzinsky

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[CoffeeFlux]

Mono changes look fine, but like Jan said I think it would be preferable to just move the identical functions to the shared portion. Thanks!

[stephentoub]

If GetLastPInvokeError is the recommended API to use moving forward, should we change all of our netcoreapp usage of GetLastWin32Error to instead use GetLastPInvokeError?

[elinor-fung]

Yes, I think so. I didn't try to do it with this change for adding the new API just because it would touch so much, but the plan would be to do so for 6.0.

[elinor-fung]

@CoffeeFlux what is the expected behaviour on mono for the stored last error when calling a P/Invoke with SetLastError=true that doesn't actually change the error?

I added a test to check that GetLastPInvokeError returns 0 after calling a P/Invoke with SetLastError=true that doesn't change the system error. On coreclr, if SetLastError=true, the last system error is cleared (set to 0) before calling the target function, so if the target function doesn't update the system error, the stored p/invoke error will be 0. From a quick look at mono, I had expected the same behaviour, but it seems to return 1 instead? (I don't think anything I've done in this change would have affected this behaviour)

[lambdageek]

what is the expected behaviour on mono for the stored last error when calling a P/Invoke with SetLastError=true that doesn't actually change the error?

Should work the same way as CoreCLR - errno should be set to 0 before the call, and then after the call mono_marshal_set_last_error should get called to push the current errno value to the last pinvoke error thread-local variable.

It's possible that it isn't working right (maybe while JITing the wrapper method for the pinvoke, the JIT sets errno to 1 for some reason).

[elinor-fung]

I added the SetLastError test to issues.targets for mono and filed #51600.

[AaronRobinsonMSFT]

Are we going to have a follow up to change all existing usage? Might be something we can file and mark "up-for-grabs" as it would be a relatively easy issue if someone in the community wanted to do the work.

[AaronRobinsonMSFT]

I don't recall for sure, but can you add a comment on these P/Invokes if they need to be blittable in order to make this work.

[elinor-fung]

They don't need to be blittable. I can just change this to bool.

[elinor-fung]

Are we going to have a follow up to change all existing usage?

Yes for existing usage for netcoreapp. As I mentioned in another comment, I didn't do it here since it would touch so much, but it would be a follow-up change for 6.0.

Might be something we can file and mark "up-for-grabs"

I like that idea. Filed: #51648