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

[mono] Add direct pinvoke module variations #81548

Closed
wants to merge 1 commit into from

Conversation

mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Feb 2, 2023

Follow up to #79721

When specifying DirectPInvokes/DirectPInvokeLists and DllImport modules in the Mono aot compiler, there are no strict limitations on what values can be passed in. Any exact mismatch between module(!entrypoint) values passed into DirectPInvokes/DirectPInvokeLists in the .csproj and module/entrypoint values specified in DllImport declarations on the managed side will result in undesired behavior.

MATCH

<ItemGroup>
    <DirectPInvokes Include="module!entrypoint" />
</ItemGroup>
--------
[DllImport("module")]
public static extern <ret> entrypoint (<args>);

NO MATCH

<ItemGroup>
    <DirectPInvokes Include="libmodule.so!entrypoint" />
</ItemGroup>
--------
[DllImport("module")]
public static extern <ret> entrypoint (<args>);

Currently, the mono aot compiler views the above as a mismatch, because the literal values are not equal even though its understood that the module of libmodule.so is module.

Across all supported platforms and architectures, the shared library prefixes and suffixes vary greatly, leading to a situation where the managed side DllImport invocation would need to vary for different platform/architecture.


This PR looks to expand the module/entrypoint pairs added to the aot compiler's known direct_pinvokes hashtable that is responsible for filtering which direct pinvokes to generate direct calls for.
More specifically:

  • modules passed into DirectPInvokes and DirectPInvokeLists will be stripped of known shared library prefixes/suffixes, and both the module/entrypoint and stripped module/entrypoint (if different) will be added to the hashtable for matching.
  • modules passed into DllImport will no longer by stripped of extensions

Specific cases that may cause problems:
libc.so, libz.so

@lateralusX
Copy link
Member

lateralusX commented Feb 3, 2023

@mdh1418 So thinking some more about this, I'm not sure this is the right approach. What we really would like to handle is the case where unix have dllimport of libMyLibrary and on windows it would be MyLibrary and avoid having to handle duplicate entries in the configurations to support both, but still give the user the power to do so if needed. Maybe a better way would be to support a very basic regexp in module names passed to AOT compiler, like (lib)?MyLibrary ,that would add both libMyLibrary and MyLibrary to the hash table. Parsing the simple regexp could be done very simple, look for ( if found look for )?, if found , create the two strings in hash table. You might need to support some escaping as well \(lib\)\?, won't match regexp and need a pass after to get rid of the escape characters but should be straightforward. If user have collisions with two libraries named libMyLibrary and MyLibrary, then he would specify them out like libMyLibrary and not use regexp meaning user is in more control compared to if we added the logic into the AOT compiler.

When we check if a module is part of the direct pinvoke hash table, we should first start using the literal from dllimport, without path (like we currently do) but with any prefix/suffix, if that is not a match, then we strip the suffix and make a search again, if that is not a match, the module is not defined and the dllimport should not be direct pinvoked.

Thoughts?

@lambdageek
Copy link
Member

What does NativeAOT do?

@lateralusX
Copy link
Member

lateralusX commented Feb 3, 2023

I had asked the same question to @mdh1418 offline, he will take a look to see, but I have not seen that they have any additional support in the specification of direct pinvokes than module!entrypoint. Right now I feel that we could stick to what we have for now and improve if needed later.

@mdh1418
Copy link
Member Author

mdh1418 commented Feb 3, 2023

What does NativeAOT do?

Just looking now at

private IEnumerable<string> ModuleNameVariations(string name)
{
yield return name;
if (_target.IsWindows)
{
string suffix = ".dll";
if (name.EndsWith(suffix, StringComparison.OrdinalIgnoreCase))
yield return name.Substring(0, name.Length - suffix.Length);
}
else
{
string suffix = _target.IsOSXLike ? ".dylib" : ".so";
if (name.EndsWith(suffix, StringComparison.Ordinal))
yield return name.Substring(0, name.Length - suffix.Length);
}
}
private IEnumerable<string> EntryPointNameVariations(string name, PInvokeFlags flags)
{
if (_target.IsWindows && !flags.ExactSpelling)
{
// Mirror CharSet normalization from Marshaller.CreateMarshaller
bool isAnsi = flags.CharSet switch
{
CharSet.Ansi => true,
CharSet.Unicode => false,
CharSet.Auto => false,
_ => true
};
if (isAnsi)
{
// For ANSI, look for the user-provided entry point name first.
// If that does not exist, try the charset suffix.
yield return name;
yield return name + "A";
}
else
{
// For Unicode, look for the entry point name with the charset suffix first.
// The 'W' API takes precedence over the undecorated one.
yield return name + "W";
yield return name;
}
}
else
{
yield return name;
}
}
, it looks like they strip the module suffix when generating their map/table of known module/entrypoint pairs. Doesn't seem like they do anything to check the prefix atleast from the side of building known map/table of module/etnrypoints. Maybe they have something before building the map/table, I'll have to keep looking, and I'm not exactly sure what they do from the perspective of compiling the managed code's [DllImport(module)] side yet, I'll need to find out where that is done for NativeAOT

They also have some sort of Entrypoint variation that we dont have. Is this something we need to implement as well? I'm not familiar with the entrypoint variations.

@lateralusX
Copy link
Member

What does NativeAOT do?

Just looking now at

private IEnumerable<string> ModuleNameVariations(string name)
{
yield return name;
if (_target.IsWindows)
{
string suffix = ".dll";
if (name.EndsWith(suffix, StringComparison.OrdinalIgnoreCase))
yield return name.Substring(0, name.Length - suffix.Length);
}
else
{
string suffix = _target.IsOSXLike ? ".dylib" : ".so";
if (name.EndsWith(suffix, StringComparison.Ordinal))
yield return name.Substring(0, name.Length - suffix.Length);
}
}
private IEnumerable<string> EntryPointNameVariations(string name, PInvokeFlags flags)
{
if (_target.IsWindows && !flags.ExactSpelling)
{
// Mirror CharSet normalization from Marshaller.CreateMarshaller
bool isAnsi = flags.CharSet switch
{
CharSet.Ansi => true,
CharSet.Unicode => false,
CharSet.Auto => false,
_ => true
};
if (isAnsi)
{
// For ANSI, look for the user-provided entry point name first.
// If that does not exist, try the charset suffix.
yield return name;
yield return name + "A";
}
else
{
// For Unicode, look for the entry point name with the charset suffix first.
// The 'W' API takes precedence over the undecorated one.
yield return name + "W";
yield return name;
}
}
else
{
yield return name;
}
}

, it looks like they strip the module suffix when generating their map/table of known module/entrypoint pairs. Doesn't seem like they do anything to check the prefix atleast from the side of building known map/table of module/etnrypoints. Maybe they have something before building the map/table, I'll have to keep looking, and I'm not exactly sure what they do from the perspective of compiling the managed code's [DllImport(module)] side yet, I'll need to find out where that is done for NativeAOT
They also have some sort of Entrypoint variation that we dont have. Is this something we need to implement as well? I'm not familiar with the entrypoint variations.

That is for windows where you can have A or W depending on char set. I think we can add that later, right now we target android/ios, so that is no variations happening there. The module variation seems to match what we currently implemented.

@mdh1418 mdh1418 closed this Mar 6, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants