[mono][wasm] marshal-ilgen is dropped when not required by jandupej · Pull Request #86035 · dotnet/runtime

jandupej · 2023-05-10T12:58:25Z

A new analyzer MarshlingPInvokeScanner is added, that scans all assemblies for constructs incompatible with the lightweight marshaller. This is essentially every P/Invoke that uses a nonblittable type as argument or returns it. The analyzer uses System.Reflection.Metadata to remain compatible with .NET Framework.

Additionally, the wasm toolchain uses this analyzer to decide whether marshal-ilgen is needed. If it is not, the full marshaller is dropped in favor of the lightweight one.

EDIT: The reduction in dotnet.native.wasm is approx. 74 KB when marshal-ilgen is dropped in favor of its stub.

…ies.

…ded.

src/tasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

src/mono/wasm/build/WasmApp.Native.targets

lambdageek

I wonder if the stub emit method can assert in a few places.

src/mono/mono/component/marshal-ilgen-stub.c

lambdageek

native code changes lgtm. I have a couple of suggestions for the build task - the biggest one being whether we can use the type system from NativeAOT instead of building our own signature provider. if we use our own, then I think we need some comments to explain what is going on. It is not obvious.

Also check the weird ECMA edge-case of "generic" enum types that arise from nested types.

src/tasks/Common/PInvokeCollector.cs

src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

src/mono/wasm/build/WasmApp.Native.targets

…tibility enum.

radical · 2023-06-06T02:57:58Z

src/mono/wasm/build/WasmApp.Native.targets


  <UsingTask TaskName="Microsoft.WebAssembly.Build.Tasks.ManagedToNativeGenerator" AssemblyFile="$(WasmAppBuilderTasksAssemblyPath)" />
  <UsingTask TaskName="Microsoft.WebAssembly.Build.Tasks.EmccCompile" AssemblyFile="$(WasmAppBuilderTasksAssemblyPath)" />
+  <UsingTask TaskName="MarshalingPInvokeScanner" AssemblyFile="$(MonoTargetsTasksAssemblyPath)" />


Add namespace for the task

radical · 2023-06-06T02:59:32Z

src/tasks/Common/PInvokeCollector.cs

+
+

Suggested change

radical · 2023-06-06T02:59:39Z

src/tasks/Common/PInvokeCollector.cs

+
+
+}
+


Suggested change

radical · 2023-06-06T03:00:41Z

src/tasks/Common/PInvokeCollector.cs

+
+
+#pragma warning disable CS0649
+internal sealed class PInvokeCallback


This could be a record like:

internal sealed record PInvokeCallback(MethodInfo method, string? EntryName = null);

radical · 2023-06-06T03:01:30Z

src/tasks/Common/PInvokeCollector.cs

+
+
+


Suggested change

radical · 2023-06-06T03:01:56Z

src/tasks/Common/PInvokeCollector.cs

+
+internal sealed class PInvokeCollector {
+    private readonly Dictionary<Assembly, bool> _assemblyDisableRuntimeMarshallingAttributeCache = new();
+    private TaskLoggingHelper Log { get; set; }


Suggested change

private TaskLoggingHelper Log { get; set; }

private readonly TaskLoggingHelper Log { get; init; }

radical · 2023-06-06T03:24:14Z

src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

+                if(mdtReader.GetString(td.Name) == "DisableRuntimeMarshallingAttribute")
+                    return false;
+            }
+            catch(InvalidCastException)


Which part can throw this exception?

radical · 2023-06-06T03:24:31Z

src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

+                TypeDefinitionHandle tdh = md.GetDeclaringType();
+                TypeDefinition td = mdtReader.GetTypeDefinition(tdh);
+
+                if(mdtReader.GetString(td.Name) == "DisableRuntimeMarshallingAttribute")


.. and check namespace

radical · 2023-06-06T03:25:26Z

src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

+                SignatureDecoder<Compatibility, object> decoder = new SignatureDecoder<Compatibility, object>(
+                    mmtcp, mdtReader, null!);


Suggested change

SignatureDecoder<Compatibility, object> decoder = new SignatureDecoder<Compatibility, object>(

mmtcp, mdtReader, null!);

SignatureDecoder<Compatibility, object> decoder = new(mmtcp, mdtReader, null!);

radical · 2023-06-06T03:26:02Z

src/tasks/MonoTargetsTasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs

+                MethodSignature<Compatibility> sgn = decoder.DecodeMethodSignature(ref sgnBlobReader);
+                if(sgn.ReturnType == Compatibility.Incompatible || sgn.ParameterTypes.Any(p => p == Compatibility.Incompatible))
+                {
+                    Log.LogMessage(MessageImportance.Low, string.Format("Assebly {0} requires marhsal-ilgen for method {1}.{2}:{3} (first pass).",


Suggested change

Log.LogMessage(MessageImportance.Low, string.Format("Assebly {0} requires marhsal-ilgen for method {1}.{2}:{3} (first pass).",

Log.LogMessage(MessageImportance.Low, string.Format("Assembly {0} requires marhsal-ilgen for method {1}.{2}:{3} (first pass).",

radical · 2023-06-06T03:26:48Z

src/tasks/WasmAppBuilder/PInvokeTableGenerator.cs

        var pinvokes = new List<PInvoke>();
        var callbacks = new List<PInvokeCallback>();

+        PInvokeCollector pinvokeCollector = new PInvokeCollector(Log);


Suggested change

PInvokeCollector pinvokeCollector = new PInvokeCollector(Log);

PInvokeCollector pinvokeCollector = new(Log);

jandupej added 10 commits March 28, 2023 16:31

Replicating naricc's PInvokeScanner.

58fc204

MarshalingPInvokeScanner now detects and outputs incompatible assembl…

538ed14

…ies.

Allowing void return type.

927ad6b

PInvoke

cfa8ded

Merge branch 'main' into marshal-analyzer-metadata

3954e44

Two-pass searching in progress.

2309940

Second pass resolves inconclusive types.

a1e9294

Cleanup.

a70d7b6

Merge branch 'main' into marshal-analyzer-metadata

6afe913

Modifying the wasm toolchain to omit marshal-ilgen when possible.

1f7c84b

jandupej added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label May 10, 2023

jandupej requested a review from lambdageek May 10, 2023 12:58

jandupej self-assigned this May 10, 2023

ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label May 10, 2023

build-analysis bot mentioned this pull request May 10, 2023

Checkout failure: "Git fetch failed with exit code 128" dotnet/arcade#9009

Open

2 tasks

jandupej added 3 commits May 11, 2023 12:24

Hopefully fix incorrect app dir.

dff107a

Added definitions to MarshalingPInvokeScannerPath hopefully where nee…

5e9dabe

…ded.

Adding definitions of MarshalingPInvokeScannerPath to more locations.

fd6a283

lambdageek reviewed May 11, 2023

View reviewed changes

src/tasks/MarshalingPInvokeScanner/MarshalingPInvokeScanner.cs Outdated Show resolved Hide resolved

teo-tsirpanis added area-Build-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels May 13, 2023

Adding missing references to PInvoke scanner.

082cf44

build-analysis bot mentioned this pull request May 16, 2023

[mono][wasm] System.Numerics.Vectors.Tests assert equal failure #84775

Closed

radekdoulik reviewed May 17, 2023

View reviewed changes

src/mono/wasm/build/WasmApp.Native.targets Show resolved Hide resolved

radekdoulik reviewed May 17, 2023

View reviewed changes

src/mono/wasm/build/WasmApp.Native.targets Outdated Show resolved Hide resolved

jandupej added 3 commits May 23, 2023 15:40

Changed task ordering, assemblies list.

2c7d9c1

Removed metadata load context.

d08e012

Fixed code analyzer issues.

c2e83a9

This was referenced May 23, 2023

Tracking issue for CI build timeouts #76454

Closed

[wasm] Random blazor build failures in WBT - error CS1001: Identifier expected #86533

Closed

Fixed P/Invoke return value in marshal-lightweight.

1156958

This was referenced May 25, 2023

IOException running NuGet-Migrations during tests in dotnet CLI first run #80619

Closed

Failed USB connection via port 54050, error 61, in tvOS arm64 Release AllSubsets_Mono #82637

Closed

jandupej added 4 commits May 26, 2023 14:37

Merge branch 'main' into marshal-analyzer-metadata

0205cee

Merge branch 'main' into marshal-analyzer-metadata

46ddaea

Removed incompatible assemblies listing.

f4b60c7

Restoring minimal functionality to marshal-ilgen-stub.

a54d680

lambdageek reviewed Jun 1, 2023

View reviewed changes

Addressed feedback.

cb1197b

jandupej marked this pull request as ready for review June 2, 2023 09:22

jandupej requested review from marek-safar, radical, thaystg and vargaz as code owners June 2, 2023 09:23

jandupej removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jun 2, 2023

jandupej mentioned this pull request Jun 2, 2023

[mono][dbg] EvaluateConstantValueUsingRuntimeEvaluate test crash on Chrome #87053

Closed

lambdageek approved these changes Jun 2, 2023

View reviewed changes

This comment was marked as duplicate.

Sign in to view

jandupej added 2 commits June 5, 2023 14:49

Tweaked identification of blittable types. Added explanation to Compa…

ad226d4

…tibility enum.

Moved PInvokeCollector.cs and SignatureMapper.cs back to WasmAppBuilder.

8fc0bbe

radical reviewed Jun 6, 2023

View reviewed changes

Addressed feedback.

c20beef

jandupej merged commit e1afb22 into dotnet:main Jun 14, 2023

lewing mentioned this pull request Jun 15, 2023

[wasm] Microbenchmarks failing with assertion Assertion at /__w/1/s/src/mono/mono/component/marshal-ilgen-stub.c:28, condition !m_type_is_byref(t)' not met` #87623

Closed

ghost locked as resolved and limited conversation to collaborators Jul 14, 2023



		#pragma warning disable CS0649
		internal sealed class PInvokeCallback

	private TaskLoggingHelper Log { get; set; }
	private readonly TaskLoggingHelper Log { get; init; }

		SignatureDecoder<Compatibility, object> decoder = new SignatureDecoder<Compatibility, object>(
		mmtcp, mdtReader, null!);

	Log.LogMessage(MessageImportance.Low, string.Format("Assebly {0} requires marhsal-ilgen for method {1}.{2}:{3} (first pass).",
	Log.LogMessage(MessageImportance.Low, string.Format("Assembly {0} requires marhsal-ilgen for method {1}.{2}:{3} (first pass).",

	PInvokeCollector pinvokeCollector = new PInvokeCollector(Log);
	PInvokeCollector pinvokeCollector = new(Log);

Conversation

jandupej commented May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lambdageek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lambdageek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as duplicate.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jandupej commented May 10, 2023 •

edited

Loading