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

[release/7.0-staging] [mono][aot] Generate 'native-indirect' wrappers in full-aot mode. #86934

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 31, 2023

Backport of #85923 to release/7.0-staging

/cc @kotlarmilos @vargaz

A user encountered an issue with the Avalonia UI framework on iOS in Mono AOT mode. The expected behavior was for the app to run, but instead, an exception is raised during app startup. The exception encountered was in the GlBasicInfoInterface constructor:

Attempting to JIT compile method '(wrapper managed-to-native) intptr Avalonia.OpenGL.GlBasicInfoInterface:wrapper_native_indirect_0x283931e40 (intptr&,int)' while running in aot-only mode

When a function pointer is returned from native code and later called using calli is not supported by full-aot prior to this PR. The reason is that the runtime needs to generate a managed-to-native wrapper to call the function pointer, and the wrapper needs to embed the address which is called which is a runtime constant.

The fix for this issue involves generating 'native-indirect' wrappers in full-aot mode in addition to the existing wrappers. This is achieved by adding an extra argument, which the caller will use to pass in the function pointer to calli.

Customer Impact

The issue was discovered by a community member.

This PR fixes #80853.

Testing

Manual testing and an automated testing on the CI were performed, utilizing the following test case. The customer's issue has been reproduced locally, and the issue has been fixed.

using System;

class Tests
{
	public static void Main () {
        unsafe {
            delegate* unmanaged<int, int> callbackProc = (delegate* unmanaged<int, int>)IntPtr.Zero;
            callbackProc (5);
        }
    }
}

Risk

Low risk. The change generates additional 'native-indirect' wrappers in full-aot mode and shouldn't change the existing code path. The fix has been in .NET 8 main branch since May 9th, 2023.

IMPORTANT: If this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

The change doesn't affect code that ships in a NuGet package.

@kotlarmilos kotlarmilos added this to the 7.0.x milestone May 31, 2023
@kotlarmilos kotlarmilos self-assigned this May 31, 2023
@kotlarmilos kotlarmilos added the Servicing-consider Issue for next servicing release review label May 31, 2023
@kotlarmilos
Copy link
Member

/azp run runtime-ioslike

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@steveisok
Copy link
Member

Approved over email by tactics.

@kotlarmilos
Copy link
Member

kotlarmilos commented Jun 2, 2023

  • Tactics approval obtained over email.
  • Please confirm the CI failures are unrelated.
  • Get a sign-off from an area owner.
  • No OOB changes needed (not affecting managed code in libraries code).

/cc: @SamMonoRT

@SamMonoRT
Copy link
Member

/cc @carlossanlop - this is ready to be merged in the next 7.0.x build.

@kotlarmilos
Copy link
Member

The failures are related to WASM and should not have been caused by the change. However, there is a failure https://helixre107v0xd1eu3ibi6ka.blob.core.windows.net/dotnet-runtime-refs-pull-86934-merge-0e902ea140f64022bf/WasmTestOnBrowser-System.Runtime.Tests/1/console.daee57b6.log?helixlogtype=result that might be relevant, as the reported message is "Parsing function pointer types in signatures is not supported."

@vargaz please take a look before we proceed.

@carlossanlop
Copy link
Member

/cc @carlossanlop - this is ready to be merged in the next 7.0.x build.

Thanks. PR owners can merge their PRs when ready.

…rapper and append_mangled_wrapper_subtype functions
@kotlarmilos
Copy link
Member

The failures include build timeouts and an incompatible Xcode version on testing machines. These issues have occurred in previous CI runs and shouldn't be related to this change. The PR is ready to be merged after the tactics re-approval.

@kotlarmilos kotlarmilos merged commit afb41da into release/7.0-staging Jun 9, 2023
@kotlarmilos kotlarmilos deleted the backport/pr-85923-to-release/7.0-staging branch June 9, 2023 06:33
@ghost ghost locked as resolved and limited conversation to collaborators Jul 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-VM-meta-mono Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants