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

iOS (and other Apple platforms): System.Private.CoreLib.dll has P/Invokes to a libhostpolicy library that isn't shipped #38543

Closed
rolfbjarne opened this issue Jun 29, 2020 · 7 comments
Assignees
Labels
os-ios Apple iOS
Milestone

Comments

@rolfbjarne
Copy link
Member

Description

The System.Private.CoreLib.dll has P/Invokes to libhostpolicy:

$ monodis microsoft.netcore.app.runtime.ios-x64/5.0.0-preview.7.20317.2/runtimes/ios-x64/native/System.Private.CoreLib.dll | grep libhostpolicy
.module extern 'libhostpolicy'
    .method assembly static hidebysig pinvokeimpl ("libhostpolicy" as "corehost_resolve_component_dependencies" autochar cdecl )
    .method assembly static hidebysig pinvokeimpl ("libhostpolicy" as "corehost_set_error_writer" autochar cdecl )

But there's no libhostpolicy library in the Microsoft.NETCore.Runtime.ios-x64 nuget (nor are these native functions provided by any other library that I can find).

Other information

It's highly preferable to not have any P/Invokes to native functions that don't exist, because it allows the AOT compiler to generate more efficient code. It also makes other performance improvements possible.

@ghost
Copy link

ghost commented Jun 29, 2020

Tagging subscribers to this area: @CoffeeFlux
Notify danmosemsft if you want to be subscribed.

@AaronRobinsonMSFT
Copy link
Member

/cc @vitek-karas

@marek-safar marek-safar added area-CoreLib-mono and removed area-AssemblyLoader-mono untriaged New issue has not been triaged by the area owner labels Jun 30, 2020
@marek-safar marek-safar added this to the 6.0.0 milestone Jun 30, 2020
@marek-safar
Copy link
Contributor

@rolfbjarne this should not matter a lot as they will be removed by illinker

@CoffeeFlux would be nice to clean this up

@rolfbjarne
Copy link
Member Author

@rolfbjarne this should not matter a lot as they will be removed by illinker

The linker won't always be enabled, in particular for simulator builds.

There's another complication here, because sometimes we have to ask the native linker to keep the native symbols P/Invokes point to (because otherwise the native linker may remove them, if those symbols come from static libraries). The problem is that if there are P/Invokes to native functions that don't exist, we can't ask the native linker to keep them, because then the native linker will fail. This means we'll have to add code to inspect all the static libraries we link with, compute a list of all the native symbols, and match with the symbols P/Invokes point to, to avoid asking the native linker to keep native symbols that don't exist.

rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Jun 30, 2020
…ibrary.

System.Private.CoreLib.dll contains P/Invokes to a libhostpolicy library,
which isn't shipped.

An isssue has been filed to fix this in the runtime: dotnet/runtime#38543
@marek-safar marek-safar added the os-ios Apple iOS label Oct 10, 2020
@spouliot
Copy link
Contributor

spouliot commented Apr 7, 2021

Still present with P3

$ monodis tests//dotnet/packages/microsoft.netcore.app.runtime.iossimulator-x64/6.0.0-preview.3.21201.4/runtimes/iossimulator-x64/native/System.Private.CoreLib.dll | grep libhostpolicy
.module extern 'libhostpolicy'
    .method assembly static hidebysig pinvokeimpl ("libhostpolicy" as "corehost_resolve_component_dependencies" autochar cdecl )
    .method assembly static hidebysig pinvokeimpl ("libhostpolicy" as "corehost_set_error_writer" autochar cdecl )

@vitek-karas
Copy link
Member

The existing implementation of AssemblyDependencyResolver is intended only for cases where the runtime is loaded via hostpolicy (so even things like corerun will not work here - and that is currently by design).

On platforms where we don't use hostpolicy the implementation of this class should be different - depending on the platform/deployment behavior. The functional definition of the class is pretty simple: "Here's a path to plugin.dll - figure out where its dependencies are located" - it's meant as a good way to implement AssemblyLoadContext.Load method logic basically.

I would imagine that on mobile, we might just throw - as dynamically loaded plugins with dependency resolution may not be a thing (or are not common enough to invest into implementing some standard behavior).

On CoreCLR proper - the class parses the .deps.json next to the plugin.dll and resolves dependencies based on that. The simplest way to implement that was to call into hostpolicy which already has all the logic to do so. It was a deliberate decision to not support anything else at that time. (For example this class doesn't work from corerun which doesn't use hostpolicy)

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 12, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 9, 2021
@SamMonoRT SamMonoRT assigned steveisok and MaximLipnin and unassigned CoffeeFlux Jun 22, 2021
@MaximLipnin
Copy link
Contributor

Addressed in #54601

@ghost ghost locked as resolved and limited conversation to collaborators Jul 25, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
os-ios Apple iOS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants