Skip to content

Commit

Permalink
[dotnet] Add support for selecting whether to create P/Invoke wrapper…
Browse files Browse the repository at this point in the history
…s or not. Fixes #4940. (#14961)

* This is a potential mitigation for slower transition to native code when
  exception marshalling is enabled (#14812).
* A minor modification was required in the linker, to make sure any modified
  assemblies are saved.

Fixes #4940.
  • Loading branch information
rolfbjarne authored May 11, 2022
1 parent c4f3d65 commit 92eda7f
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 10 deletions.
1 change: 1 addition & 0 deletions dotnet/targets/Xamarin.Shared.Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,7 @@
PlatformAssembly=$(_PlatformAssemblyName).dll
RelativeAppBundlePath=$(_RelativeAppBundlePath)
Registrar=$(_BundlerRegistrar)
RequirePInvokeWrappers=$(_RequirePInvokeWrappers)
RuntimeConfigurationFile=$(_RuntimeConfigurationFile)
SdkDevPath=$(_SdkDevPath)
SdkRootDirectory=$(_XamarinSdkRootDirectory)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ public abstract class ParseBundlerArgumentsTaskBase : XamarinTask {
[Output]
public string Registrar { get; set; }

[Output]
public string RequirePInvokeWrappers { get; set; }

// This is input too
[Output]
public string NoStrip { get; set; }
Expand Down Expand Up @@ -148,6 +151,9 @@ public override bool Execute ()
case "package-debug-symbols":
PackageDebugSymbols = string.IsNullOrEmpty (value) ? "true" : value;
break;
case "require-pinvoke-wrappers":
RequirePInvokeWrappers = string.IsNullOrEmpty (value) ? "true" : value;
break;
case "registrar":
value = hasValue ? value : nextValue; // requires a value, which might be the next option
Registrar = value;
Expand Down
1 change: 1 addition & 0 deletions msbuild/Xamarin.Shared/Xamarin.Shared.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,7 @@ Copyright (C) 2018 Microsoft. All rights reserved.
<Output TaskParameter="Optimize" PropertyName="_BundlerOptimize"/>
<Output TaskParameter="PackageDebugSymbols" PropertyName="PackageDebugSymbols" />
<Output TaskParameter="Registrar" PropertyName="_BundlerRegistrar" />
<Output TaskParameter="RequirePInvokeWrappers" PropertyName="_RequirePInvokeWrappers" />
<Output TaskParameter="Verbosity" PropertyName="_BundlerVerbosity" />
<Output TaskParameter="XmlDefinitions" ItemName="_BundlerXmlDefinitions" />
<Output TaskParameter="NoStrip" PropertyName="EnableAssemblyILStripping" />
Expand Down
11 changes: 11 additions & 0 deletions tools/common/Application.cs
Original file line number Diff line number Diff line change
Expand Up @@ -611,8 +611,16 @@ public string Name {
get { return Path.GetFileNameWithoutExtension (AppDirectory); }
}

bool? requires_pinvoke_wrappers;
public bool RequiresPInvokeWrappers {
get {
if (requires_pinvoke_wrappers.HasValue)
return requires_pinvoke_wrappers.Value;

// By default this is disabled for .NET
if (Driver.IsDotNet)
return false;

if (Platform == ApplePlatform.MacOSX)
return false;

Expand All @@ -624,6 +632,9 @@ public bool RequiresPInvokeWrappers {

return MarshalObjectiveCExceptions == MarshalObjectiveCExceptionMode.ThrowManagedException || MarshalObjectiveCExceptions == MarshalObjectiveCExceptionMode.Abort;
}
set {
requires_pinvoke_wrappers = value;
}
}

public string PlatformName {
Expand Down
3 changes: 3 additions & 0 deletions tools/common/Driver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ static bool ParseOptions (Application app, Mono.Options.OptionSet options, strin
options.Add ("rid=", "The runtime identifier we're building for", v => {
app.RuntimeIdentifier = v;
}, true /* hidden - this is only for build-time --runregistrar support */);
options.Add ("require-pinvoke-wrappers:", v => {
app.RequiresPInvokeWrappers = ParseBool (v, "--require-pinvoke-wrappers");
});


// Keep the ResponseFileSource option at the end.
Expand Down
8 changes: 8 additions & 0 deletions tools/dotnet-linker/LinkerConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ public class LinkerConfiguration {

Dictionary<string, List<MSBuildItem>> msbuild_items = new Dictionary<string, List<MSBuildItem>> ();

internal PInvokeWrapperGenerator PInvokeWrapperGenerationState;

public static LinkerConfiguration GetInstance (LinkContext context, bool createIfNotFound = true)
{
if (!configurations.TryGetValue (context, out var instance) && createIfNotFound) {
Expand Down Expand Up @@ -231,6 +233,11 @@ public static LinkerConfiguration GetInstance (LinkContext context, bool createI
case "Registrar":
Application.ParseRegistrar (value);
break;
case "RequirePInvokeWrappers":
if (!TryParseOptionalBoolean (value, out var require_pinvoke_wrappers))
throw new InvalidOperationException ($"Unable to parse the {key} value: {value} in {linker_file}");
Application.RequiresPInvokeWrappers = require_pinvoke_wrappers.Value;
break;
case "RuntimeConfigurationFile":
Application.RuntimeConfigurationFile = value;
break;
Expand Down Expand Up @@ -401,6 +408,7 @@ public void Write ()
Console.WriteLine ($" RelativeAppBundlePath: {RelativeAppBundlePath}");
Console.WriteLine ($" Registrar: {Application.Registrar} (Options: {Application.RegistrarOptions})");
Console.WriteLine ($" RuntimeConfigurationFile: {Application.RuntimeConfigurationFile}");
Console.WriteLine ($" RequirePInvokeWrappers: {Application.RequiresPInvokeWrappers}");
Console.WriteLine ($" SdkDevPath: {Driver.SdkRoot}");
Console.WriteLine ($" SdkRootDirectory: {SdkRootDirectory}");
Console.WriteLine ($" SdkVersion: {SdkVersion}");
Expand Down
18 changes: 18 additions & 0 deletions tools/dotnet-linker/Steps/GenerateMainStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,24 @@ protected override void TryEndProcess ()
if (app.EnableDebug)
item.Metadata.Add ("Arguments", "-DDEBUG");
items.Add (item);

if (app.RequiresPInvokeWrappers) {
var state = Configuration.PInvokeWrapperGenerationState;
if (state.Started) {
// The generator is 'started' by the linker, which means it may not
// be started if the linker was not executed due to re-using cached results.
state.End ();
}
item = new MSBuildItem {
Include = state.SourcePath,
Metadata = {
{ "Arch", abi.AsArchString () },
},
};
if (app.EnableDebug)
item.Metadata.Add ("Arguments", "-DDEBUG");
items.Add (item);
}
}

Configuration.WriteOutputForMSBuild ("_MainFile", items);
Expand Down
65 changes: 55 additions & 10 deletions tools/linker/MonoTouch.Tuner/ListExportedSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,56 @@ namespace Xamarin.Linker.Steps
public class ListExportedSymbols : BaseStep
{
PInvokeWrapperGenerator state;
#if !NET
bool skip_sdk_assemblies;
#endif

PInvokeWrapperGenerator State {
get {
#if NET
if (state is null && DerivedLinkContext.App.RequiresPInvokeWrappers) {
Configuration.PInvokeWrapperGenerationState = new PInvokeWrapperGenerator () {
App = DerivedLinkContext.App,
SourcePath = Path.Combine (Configuration.CacheDirectory, "pinvokes.mm"),
HeaderPath = Path.Combine (Configuration.CacheDirectory, "pinvokes.h"),
Registrar = DerivedLinkContext.StaticRegistrar,
};
state = Configuration.PInvokeWrapperGenerationState;
}
#endif
return state;
}
}

#if NET
public LinkerConfiguration Configuration {
get {
return LinkerConfiguration.GetInstance (Context);
}
}
#endif

public DerivedLinkContext DerivedLinkContext {
get {
#if NET
return LinkerConfiguration.GetInstance (Context).DerivedLinkContext;
return Configuration.DerivedLinkContext;
#else
return (DerivedLinkContext) Context;
#endif
}
}

public ListExportedSymbols () : this (null)
#if NET
public ListExportedSymbols ()
{
}

#else
internal ListExportedSymbols (PInvokeWrapperGenerator state, bool skip_sdk_assemblies = false)
{
this.state = state;
this.skip_sdk_assemblies = skip_sdk_assemblies;
}
#endif

protected override void ProcessAssembly (AssemblyDefinition assembly)
{
Expand All @@ -64,23 +93,34 @@ protected override void ProcessAssembly (AssemblyDefinition assembly)
if (!hasSymbols)
return;

var modified = false;
foreach (var type in assembly.MainModule.Types)
ProcessType (type);
modified |= ProcessType (type);

// Make sure the linker saves any changes in the assembly.
if (modified) {
var action = Context.Annotations.GetAction (assembly);
if (action == AssemblyAction.Copy)
Context.Annotations.SetAction (assembly, AssemblyAction.Save);
}
}

void ProcessType (TypeDefinition type)
bool ProcessType (TypeDefinition type)
{
var modified = false;
if (type.HasNestedTypes) {
foreach (var nested in type.NestedTypes)
ProcessType (nested);
modified |= ProcessType (nested);
}

if (type.HasMethods) {
foreach (var method in type.Methods)
ProcessMethod (method);
modified |= ProcessMethod (method);
}

AddRequiredObjectiveCType (type);

return modified;
}

void AddRequiredObjectiveCType (TypeDefinition type)
Expand All @@ -106,20 +146,23 @@ void AddRequiredObjectiveCType (TypeDefinition type)
}
}

void ProcessMethod (MethodDefinition method)
bool ProcessMethod (MethodDefinition method)
{
var modified = false;

if (method.IsPInvokeImpl && method.HasPInvokeInfo && method.PInvokeInfo != null) {
var pinfo = method.PInvokeInfo;
bool addPInvokeSymbol = false;

if (state != null) {
if (State != null) {
switch (pinfo.EntryPoint) {
case "objc_msgSend":
case "objc_msgSendSuper":
case "objc_msgSend_stret":
case "objc_msgSendSuper_stret":
case "objc_msgSend_fpret":
state.ProcessMethod (method);
State.ProcessMethod (method);
modified = true;
break;
default:
break;
Expand Down Expand Up @@ -181,6 +224,8 @@ void ProcessMethod (MethodDefinition method)
DerivedLinkContext.RequiredSymbols.AddField ((string) symbol).AddMember (property);
}
}

return modified;
}
}
}

5 comments on commit 92eda7f

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💻 [CI Build] Tests on macOS Mac Catalina (10.15) passed 💻

All tests on macOS Mac Catalina (10.15) passed.

Pipeline on Agent
Hash: 92eda7f35399f442f8730aa539571ad8fd44d7da

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📋 [CI Build] API Diff 📋

API Current PR diff

ℹ️ API Diff (from PR only) (please review changes)

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

API diff

✅ API Diff from stable

View API diff
View dotnet API diff
View dotnet legacy API diff
View dotnet iOS-MacCatalayst API diff

Generator diff

Generator Diff (no change)

Pipeline on Agent XAMMINI-062.Monterey'
Hash: 92eda7f35399f442f8730aa539571ad8fd44d7da

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 [CI Build] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent XAMMINI-051.Monterey'
Hash: 92eda7f35399f442f8730aa539571ad8fd44d7da

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMMINI-051.Monterey'
Hash: 92eda7f35399f442f8730aa539571ad8fd44d7da

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 Tests failed catastrophically on VSTS: simulator tests iOS (no summary found). 🔥

Result file D:\a\1\s\Reports\TestSummary-simulator\TestSummary.md not found.

Pipeline on Agent XAMBOT-1101.Monterey'
[dotnet] Add support for selecting whether to create P/Invoke wrappers or not. Fixes #4940. (#14961)

Please sign in to comment.