Skip to content

Commit

Permalink
[tools] Make sure to finish the P/Invoke generator output before runn…
Browse files Browse the repository at this point in the history
…ing the static registrar. Fixes #15190. (#15214)

Otherwise the P/Invoke generator leaves partial results in the static
registrar class, essentially saying things like "we've processed CoreMidi, no
need to add an #include for this framework", and then we'd generate the static
registrar code and that code would lack the #include for CoreMidi.

Finishing the P/Invoke generator output will clear out any state stored in the
static registrar.

Also fix a few other issues to make the generated P/Invoke wrapper code work,
and add a test.

Fixes #15190.
  • Loading branch information
rolfbjarne authored Jun 9, 2022
1 parent c68372d commit 77b8b61
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 14 deletions.
3 changes: 3 additions & 0 deletions src/Simd/MatrixFloat3x3RM.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@
using System.Numerics;
using System.Runtime.InteropServices;

using ObjCRuntime;

// This type does not come from the CoreGraphics framework
#if NET
namespace CoreGraphics
{
[NativeName ("GLKMatrix3")]
[StructLayout (LayoutKind.Sequential)]
public struct RMatrix3 : IEquatable<RMatrix3>
{
Expand Down
21 changes: 21 additions & 0 deletions tests/dotnet/UnitTests/ProjectTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -917,5 +917,26 @@ public void CatalystAppOptimizedForMacOS_InvalidMinOS (ApplePlatform platform, s
Assert.AreEqual (1, errors.Length, "Error count");
Assert.AreEqual ($"The UIDeviceFamily value '6' requires macOS 11.0. Please set the 'SupportedOSPlatformVersion' in the project file to at least 14.0 (the Mac Catalyst version equivalent of macOS 11.0). The current value is {minOS} (equivalent to macOS 10.15.2).", errors [0].Message, "Error message");
}

[Test]
[TestCase (ApplePlatform.iOS, "iossimulator-x64")]
// [TestCase (ApplePlatform.TVOS, "tvos-arm64")] // Currently doesn't work because we overwrite the required MtouchExtraArgs in tests/nunit.frameworks.target in this test.
// [TestCase (ApplePlatform.TVOS, "tvossimulator-x64")] // Currently doesn't work because we emit signatures with structs from the MetalPerformanceShaders framework, which isn't available in the tvOS simulator.
[TestCase (ApplePlatform.MacOSX, "osx-arm64")]
[TestCase (ApplePlatform.MacCatalyst, "maccatalyst-x64")]
public void PInvokeWrapperGenerator (ApplePlatform platform, string runtimeIdentifiers)
{
var project = "MySimpleApp";
Configuration.IgnoreIfIgnoredPlatform (platform);

var project_path = GetProjectPath (project, runtimeIdentifiers: runtimeIdentifiers, platform: platform, out var appPath);
Clean (project_path);
var properties = GetDefaultProperties (runtimeIdentifiers);
var extraArgs = "--require-pinvoke-wrappers:true --registrar:static"; // enable the static registrar too, see https://github.com/xamarin/xamarin-macios/issues/15190.
properties ["MonoBundlingExtraArgs"] = extraArgs;
properties ["MtouchExtraArgs"] = extraArgs;

DotNet.AssertBuild (project_path, properties);
}
}
}
7 changes: 5 additions & 2 deletions tools/common/StaticRegistrar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2329,6 +2329,7 @@ string CheckStructure (TypeDefinition structure, string descriptiveMethodName, M
n = "struct trampoline_struct_" + name.ToString ();
if (!structures.Contains (n)) {
structures.Add (n);
declarations.WriteLine ($"// {structure.FullName} (+other structs with same layout)");
declarations.WriteLine ("{0} {{\n{1}}};", n, body.ToString ());
}

Expand Down Expand Up @@ -2386,7 +2387,7 @@ void ProcessStructure (StringBuilder name, AutoIndentStringBuilder body, TypeDef
case "System.UIntPtr":
name.Append ('p');
body.AppendLine ("void *v{0};", size);
size += 4; // for now at least...
size += Is64Bits ? 8 : 4;
break;
default:
bool found = false;
Expand Down Expand Up @@ -2506,6 +2507,7 @@ string ToObjCParameterType (TypeReference type, string descriptiveMethodName, Li
case "System.Drawing.PointF": return App.Platform == ApplePlatform.MacOSX ? "NSPoint" : "CGPoint";
case "System.Drawing.SizeF": return App.Platform == ApplePlatform.MacOSX ? "NSSize" : "CGSize";
case "System.String": return "NSString *";
case "System.UIntPtr":
case "System.IntPtr": return "void *";
case "System.SByte": return "signed char";
case "System.Byte": return "unsigned char";
Expand All @@ -2530,6 +2532,7 @@ string ToObjCParameterType (TypeReference type, string descriptiveMethodName, Li
throw ErrorHelper.CreateError (4102, Errors.MT4102, "System.DateTime", "Foundation.NSDate", descriptiveMethodName);
case "ObjCRuntime.Selector": return "SEL";
case "ObjCRuntime.Class": return "Class";
case "ObjCRuntime.NativeHandle": return "void *";
default:
if (type.FullName == NFloatTypeName) {
CheckNamespace ("CoreGraphics", exceptions);
Expand Down Expand Up @@ -4982,7 +4985,7 @@ string TryGeneratePInvokeWrapper (PInvokeWrapperGenerator state, MethodDefinitio
sb.WriteLine ("{");
if (is_stret) {
sb.StringBuilder.AppendLine ("#if defined (__arm64__)");
sb.WriteLine ("xamarin_process_managed_exception (xamarin_create_system_entry_point_not_found_exception (\"{0}\"));", pinfo.EntryPoint);
sb.WriteLine ("xamarin_process_managed_exception ((MonoObject *) xamarin_create_system_entry_point_not_found_exception (\"{0}\"));", pinfo.EntryPoint);
sb.StringBuilder.AppendLine ("#else");
}
sb.WriteLine ("@try {");
Expand Down
5 changes: 0 additions & 5 deletions tools/dotnet-linker/Steps/GenerateMainStep.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,6 @@ protected override void TryEndProcess ()

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 = {
Expand Down
12 changes: 12 additions & 0 deletions tools/linker/MonoTouch.Tuner/ListExportedSymbols.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ PInvokeWrapperGenerator State {
}
}

#if NET
protected override void EndProcess ()
{
if (state?.Started == true) {
// 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 ();
}
base.EndProcess ();
}
#endif

#if NET
public LinkerConfiguration Configuration {
get {
Expand Down
6 changes: 5 additions & 1 deletion tools/mmp/driver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,11 @@ static void Compile ()

CheckSystemMonoVersion ();

if (App.RequiresPInvokeWrappers) {
var state = BuildTarget.LinkerOptions.MarshalNativeExceptionsState;
state.End ();
}

if (App.Registrar == RegistrarMode.Static) {
registrarPath = Path.Combine (App.Cache.Location, "registrar.m");
var registrarH = Path.Combine (App.Cache.Location, "registrar.h");
Expand Down Expand Up @@ -1002,7 +1007,6 @@ static void Compile ()

if (App.RequiresPInvokeWrappers) {
var state = BuildTarget.LinkerOptions.MarshalNativeExceptionsState;
state.End ();
args.Add (state.SourcePath);
}

Expand Down
13 changes: 7 additions & 6 deletions tools/mtouch/Target.mtouch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,13 @@ public void LinkAssemblies (out List<AssemblyDefinition> assemblies, string outp

MonoTouch.Tuner.Linker.Process (LinkerOptions, out LinkContext, out assemblies);

var state = MarshalNativeExceptionsState;
if (state?.Started == true) {
// 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 ();
}

ErrorHelper.Show (LinkContext.Exceptions);

Driver.Watch ("Link Assemblies", 1);
Expand Down Expand Up @@ -881,12 +888,6 @@ public void CompilePInvokeWrappers ()

// Write P/Invokes
var state = MarshalNativeExceptionsState;
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 ();
}

var ifile = state.SourcePath;
var mode = App.LibPInvokesLinkMode;
foreach (var abi in GetArchitectures (mode)) {
Expand Down

5 comments on commit 77b8b61

@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: 77b8b616396834e745e665130c40e4df947a36c1

@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 M1 - Mac Big Sur (11.5) passed 💻

All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: 77b8b616396834e745e665130c40e4df947a36c1

@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 diff (for current PR)

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

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

API diff (vs stable)

✅ API Diff from stable

API diff: vsdrops gist

Xamarin
.NET
Xamarin vs .NET
iOS vs Mac Catalyst (.NET)

Generator diff

Generator Diff (no change)

Pipeline on Agent XAMMINI-062.Monterey
Hash: 77b8b616396834e745e665130c40e4df947a36c1

@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-055.Monterey
Hash: 77b8b616396834e745e665130c40e4df947a36c1

@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 failed on VSTS: simulator tests iOS ❌

Tests failed on VSTS: simulator tests iOS.

Test results

45 tests failed, 189 tests passed.

Failed tests

  • monotouch-test/iOS Unified 64-bits - simulator/Debug [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (all optimizations) [dotnet]: Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (LinkSdk): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (static registrar): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Release (all optimizations): Failed
  • monotouch-test/iOS Unified 64-bits - simulator/Debug (all optimizations): Failed
  • monotouch-test/tvOS - simulator/Debug [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Debug: Failed
  • monotouch-test/tvOS - simulator/Debug (LinkSdk) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Debug (static registrar) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Release (all optimizations) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Debug (all optimizations) [dotnet]: Failed
  • monotouch-test/tvOS - simulator/Debug (LinkSdk): Failed
  • monotouch-test/tvOS - simulator/Debug (static registrar): Failed
  • monotouch-test/tvOS - simulator/Release (all optimizations): Failed
  • monotouch-test/tvOS - simulator/Debug (all optimizations): Failed
  • interdependent-binding-projects/watchOS 32-bits - simulator/Debug: Crashed
  • introspection/watchOS 32-bits - simulator/Debug: Crashed
  • link sdk/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemCoreTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemXmlTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemJsonTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemSecurityTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono SystemDataTests/watchOS 32-bits - simulator/Debug: Crashed
  • [NUnit] Mono CorlibTests/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemDataXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemJsonXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemSecurityXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemLinqXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemXmlXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemRuntimeCompilerServicesUnsafeXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemNumericsXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono MicrosoftCSharpXunit/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemNetHttpUnitTestsXunit/watchOS 32-bits - simulator/Debug: Crashed
  • mscorlib Part 1/watchOS 32-bits - simulator/Debug: Crashed
  • mscorlib Part 2/watchOS 32-bits - simulator/Debug: Crashed
  • mscorlib Part 3/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemCoreXunit Part 1/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemCoreXunit Part 2/watchOS 32-bits - simulator/Debug: Crashed
  • [xUnit] Mono SystemXunit/watchOS 32-bits - simulator/Debug: Crashed

Pipeline on Agent XAMBOT-1043.Monterey'
[tools] Make sure to finish the P/Invoke generator output before running the static registrar. Fixes #15190. (#15214)

Please sign in to comment.