[release/6.0-rc2][wasm] Fixes, and improvements based on user feedback

eng/Versions.props

@@ -165,7 +165,7 @@
     <SQLitePCLRawbundle_greenVersion>2.0.4</SQLitePCLRawbundle_greenVersion>
     <MoqVersion>4.12.0</MoqVersion>
     <FsCheckVersion>2.14.3</FsCheckVersion>
-    <SdkVersionForWorkloadTesting>6.0.100-rc.2.21463.12</SdkVersionForWorkloadTesting>
+    <SdkVersionForWorkloadTesting>6.0.100-rc.2.21473.29</SdkVersionForWorkloadTesting>
     <!-- Docs -->
     <MicrosoftPrivateIntellisenseVersion>6.0.0-preview-20210916.1</MicrosoftPrivateIntellisenseVersion>
     <!-- ILLink -->

eng/testing/scenarios/BuildWasmAppsJobsList.txt

@@ -11,6 +11,7 @@ Wasm.Build.Tests.LocalEMSDKTests
 Wasm.Build.Tests.MainWithArgsTests
 Wasm.Build.Tests.NativeBuildTests
 Wasm.Build.Tests.NativeLibraryTests
+Wasm.Build.Tests.PInvokeTableGeneratorTests
 Wasm.Build.Tests.RebuildTests
 Wasm.Build.Tests.SatelliteAssembliesTests
 Wasm.Build.Tests.WasmBuildAppTest

src/libraries/workloads-testing.targets

@@ -16,7 +16,6 @@
     </ItemGroup>
 
     <Copy SourceFiles="@(_SourceFiles)" DestinationFolder="$(SdkWithWorkloadForTestingPath)\%(_SourceFiles.RecursiveDir)" />
-    <Copy SourceFiles="$(MonoProjectRoot)\wasm\BlazorOverwrite.targets" DestinationFiles="$(SdkWithWorkloadForTestingPath)\sdk\$(SdkVersionForWorkloadTesting)\Sdks\Microsoft.NET.Sdk.BlazorWebAssembly\targets\Microsoft.NET.Sdk.BlazorWebAssembly.6_0.targets" />
 
     <WriteLinesToFile File="$(SdkWithWorkloadStampPath)" Lines="" Overwrite="true" />
   </Target>
@@ -41,7 +40,6 @@
     <Exec Condition="$([MSBuild]::IsOSPlatform('windows'))"
           Command='powershell -ExecutionPolicy ByPass -NoProfile -command "&amp; $(_DotNetInstallScriptPath) -InstallDir $(SdkWithNoWorkloadForTestingPath) -Version $(SdkVersionForWorkloadTesting)"' />
 
-    <Copy SourceFiles="$(MonoProjectRoot)\wasm\BlazorOverwrite.targets" DestinationFiles="$(SdkWithNoWorkloadForTestingPath)\sdk\$(SdkVersionForWorkloadTesting)\Sdks\Microsoft.NET.Sdk.BlazorWebAssembly\targets\Microsoft.NET.Sdk.BlazorWebAssembly.6_0.targets" />
     <WriteLinesToFile File="$(SdkWithNoWorkloadStampPath)" Lines="" Overwrite="true" />
   </Target>
 

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

@@ -143,6 +143,7 @@
     <PropertyGroup>
       <_MonoAotCrossCompilerPath>@(MonoAotCrossCompiler->WithMetadataValue('RuntimeIdentifier','browser-wasm'))</_MonoAotCrossCompilerPath>
       <_EmccDefaultFlagsRsp>$([MSBuild]::NormalizePath($(_WasmRuntimePackSrcDir), 'emcc-default.rsp'))</_EmccDefaultFlagsRsp>
+      <WasmNativeStrip Condition="'$(WasmNativeStrip)' == '' and '$(Configuration)' == 'Debug' and '$(WasmBuildingForNestedPublish)' != 'true'">false</WasmNativeStrip>
       <WasmNativeStrip Condition="'$(WasmNativeStrip)' == ''">true</WasmNativeStrip>
       <WasmNativeDebugSymbols Condition="'$(WasmNativeDebugSymbols)' == ''">true</WasmNativeDebugSymbols>
       <WasmLinkIcalls Condition="'$(WasmLinkIcalls)' == ''">$(WasmBuildNative)</WasmLinkIcalls>
@@ -156,8 +157,7 @@
 
       <_EmccAssertionLevelDefault>0</_EmccAssertionLevelDefault>
       <_EmccOptimizationFlagDefault Condition="'$(_WasmDevel)' == 'true'">-O0 -s ASSERTIONS=$(_EmccAssertionLevelDefault)</_EmccOptimizationFlagDefault>
-      <_EmccOptimizationFlagDefault Condition="'$(_EmccOptimizationFlagDefault)' == '' and '$(OS)' != 'Windows_NT' and '$(Configuration)' == 'Debug'">-Os</_EmccOptimizationFlagDefault>
-      <_EmccOptimizationFlagDefault Condition="'$(_EmccOptimizationFlagDefault)' == '' and '$(Configuration)' != 'Debug'">-Oz</_EmccOptimizationFlagDefault>
+      <_EmccOptimizationFlagDefault Condition="'$(_EmccOptimizationFlagDefault)' == '' and '$(Configuration)' == 'Debug' and '$(WasmBuildingForNestedPublish)' != 'true'">-O1</_EmccOptimizationFlagDefault>
       <_EmccOptimizationFlagDefault Condition="'$(_EmccOptimizationFlagDefault)' == ''">-Oz</_EmccOptimizationFlagDefault>
 
       <EmccCompileOptimizationFlag Condition="'$(EmccCompileOptimizationFlag)' == ''">$(_EmccOptimizationFlagDefault)</EmccCompileOptimizationFlag>
@@ -206,6 +206,9 @@
       <_EmccLDFlags Include="@(_EmccCommonFlags)" />
       <_EmccLDFlags Include="-s TOTAL_MEMORY=$(EmccTotalMemory)" />
 
+      <!-- ILLinker should have removed unused imports, so error for Publish -->
+      <_EmccLDFlags Include="-s ERROR_ON_UNDEFINED_SYMBOLS=0" Condition="'$(WasmBuildingForNestedPublish)' != 'true'" />
+
       <_DriverCDependencies Include="$(_WasmPInvokeHPath);$(_WasmICallTablePath)" />
       <_DriverCDependencies Include="$(_DriverGenCPath)" Condition="'$(_DriverGenCNeeded)' == 'true'" />
 
@@ -296,7 +299,8 @@
           Inputs="@(_BitcodeFile);$(_EmccDefaultFlagsRsp);$(_EmccCompileBitcodeRsp)"
           Outputs="@(_BitcodeFile->'%(ObjectFile)')"
           Condition="'$(_WasmShouldAOT)' == 'true' and @(_BitcodeFile->Count()) > 0"
-          DependsOnTargets="_WasmWriteRspForCompilingBitcode">
+          DependsOnTargets="_WasmWriteRspForCompilingBitcode"
+          Returns="@(FileWrites)">
 
     <ItemGroup>
       <_BitCodeFile Dependencies="%(_BitCodeFile.Dependencies);$(_EmccDefaultFlagsRsp);$(_EmccCompileBitcodeRsp)" />
@@ -362,7 +366,8 @@
   <Target Name="_WasmLinkDotNet"
           Inputs="@(_WasmLinkDependencies);$(_EmccDefaultFlagsRsp);$(_EmccLinkRsp)"
           Outputs="$(_WasmIntermediateOutputPath)dotnet.js;$(_WasmIntermediateOutputPath)dotnet.wasm"
-          DependsOnTargets="_WasmSelectRuntimeComponentsForLinking;_WasmCompileAssemblyBitCodeFilesForAOT;_WasmWriteRspFilesForLinking">
+          DependsOnTargets="_WasmSelectRuntimeComponentsForLinking;_WasmCompileAssemblyBitCodeFilesForAOT;_WasmWriteRspFilesForLinking"
+          Returns="@(FileWrites)" >
 
     <Message Text="Linking with emcc. This may take a while ..." Importance="High" />
     <Message Text="Running emcc with @(_EmccLinkStepArgs->'%(Identity)', ' ')" Importance="Low" />

src/tasks/WasmAppBuilder/EmccCompile.cs

@@ -95,6 +95,7 @@ private bool ExecuteActual()
                     if (!ShouldCompile(srcFile, objFile, depFiles, out string reason))
                     {
                         Log.LogMessage(MessageImportance.Low, $"Skipping {srcFile} because {reason}.");
+                        outputItems.Add(CreateOutputItemFor(srcFile, objFile));
                     }
                     else
                     {
@@ -107,7 +108,8 @@ private bool ExecuteActual()
                 if (_numCompiled == _totalFiles)
                 {
                     // nothing to do!
-                    return true;
+                    OutputFiles = outputItems.ToArray();
+                    return !Log.HasLoggedErrors;
                 }
 
                 if (_numCompiled > 0)
@@ -200,9 +202,7 @@ bool ProcessSourceFile(string srcFile, string objFile)
                     else
                         Log.LogMessage(MessageImportance.Low, $"Copied {tmpObjFile} to {objFile}");
 
-                    ITaskItem newItem = new TaskItem(objFile);
-                    newItem.SetMetadata("SourceFile", srcFile);
-                    outputItems.Add(newItem);
+                    outputItems.Add(CreateOutputItemFor(srcFile, objFile));
 
                     int count = Interlocked.Increment(ref _numCompiled);
                     Log.LogMessage(MessageImportance.High, $"[{count}/{_totalFiles}] {Path.GetFileName(srcFile)} -> {Path.GetFileName(objFile)} [took {elapsedSecs:F}s]");
@@ -219,6 +219,13 @@ bool ProcessSourceFile(string srcFile, string objFile)
                     File.Delete(tmpObjFile);
                 }
             }
+
+            ITaskItem CreateOutputItemFor(string srcFile, string objFile)
+            {
+                ITaskItem newItem = new TaskItem(objFile);
+                newItem.SetMetadata("SourceFile", srcFile);
+                return newItem;
+            }
         }
 
         private bool ShouldCompile(string srcFile, string objFile, string[] depFiles, out string reason)

src/tasks/WasmAppBuilder/PInvokeTableGenerator.cs

@@ -3,13 +3,10 @@
 
 using System;
 using System.Collections.Generic;
-using System.Collections.Immutable;
-using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
 using System.Text;
-using System.Text.Json;
 using System.Reflection;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
@@ -121,25 +118,42 @@ private void EmitPInvokeTable(StreamWriter w, Dictionary<string, string> modules
         w.WriteLine("// GENERATED FILE, DO NOT MODIFY");
         w.WriteLine();
 
-        var decls = new HashSet<string>();
-        foreach (var pinvoke in pinvokes.OrderBy(l => l.EntryPoint))
+        var pinvokesGroupedByEntryPoint = pinvokes.Where(l => modules.ContainsKey(l.Module))
+                             .OrderBy(l => l.EntryPoint)
+                             .GroupBy(l => l.EntryPoint);
+
+        var comparer = new PInvokeComparer();
+        foreach (IGrouping<string, PInvoke> group in pinvokesGroupedByEntryPoint)
         {
-            if (modules.ContainsKey(pinvoke.Module)) {
-                try
+            var candidates = group.Distinct(comparer).ToArray();
+            PInvoke first = candidates[0];
+            if (ShouldTreatAsVariadic(candidates))
+            {
+                Log.LogWarning($"Found a native function ({first.EntryPoint}) with varargs, which is not supported. Calling it will fail at runtime. Module: {first.Module}." +
+                                $" Managed DllImports: {Environment.NewLine}{CandidatesToString(candidates)}");
+
+                string? decl = GenPInvokeDecl(first, treatAsVariadic: true);
+                if (decl != null)
                 {
-                    var decl = GenPInvokeDecl(pinvoke);
-                    if (decls.Contains(decl))
-                        continue;
+                    w.WriteLine($"// Variadic signature created for");
+                    foreach (PInvoke pinvoke in candidates)
+                        w.WriteLine($"//   {pinvoke.Method}");
 
                     w.WriteLine(decl);
-                    decls.Add(decl);
-                }
-                catch (NotSupportedException)
-                {
-                    // See the FIXME in GenPInvokeDecl
-                    Log.LogWarning($"Cannot handle function pointer arguments/return value in pinvoke method '{pinvoke.Method}' in type '{pinvoke.Method.DeclaringType}'.");
-                    pinvoke.Skip = true;
                 }
+
+                continue;
+            }
+
+            var decls = new HashSet<string>();
+            foreach (var candidate in candidates)
+            {
+                var decl = GenPInvokeDecl(candidate, treatAsVariadic: false);
+                if (decl == null || decls.Contains(decl))
+                    continue;
+
+                w.WriteLine(decl);
+                decls.Add(decl);
             }
         }
 
@@ -186,6 +200,25 @@ static string ModuleNameToId(string name)
 
             return fixedName;
         }
+
+        static bool ShouldTreatAsVariadic(PInvoke[] candidates)
+        {
+            if (candidates.Length < 2)
+                return false;
+
+            PInvoke first = candidates[0];
+            if (TryIsMethodGetParametersUnsupported(first.Method, out _))
+                return false;
+
+            int firstNumArgs = first.Method.GetParameters().Length;
+            return candidates
+                        .Skip(1)
+                        .Any(c => !TryIsMethodGetParametersUnsupported(c.Method, out _) &&
+                                    c.Method.GetParameters().Length != firstNumArgs);
+        }
+
+        static string CandidatesToString(IEnumerable<PInvoke> group)
+            => string.Join(Environment.NewLine, group);
     }
 
     private string MapType (Type t)
@@ -205,7 +238,29 @@ private string MapType (Type t)
             return "int";
     }
 
-    private string GenPInvokeDecl(PInvoke pinvoke)
+    // FIXME: System.Reflection.MetadataLoadContext can't decode function pointer types
+    // https://github.com/dotnet/runtime/issues/43791
+    private static bool TryIsMethodGetParametersUnsupported(MethodInfo method, [NotNullWhen(true)] out string? reason)
+    {
+        try
+        {
+            method.GetParameters();
+        }
+        catch (NotSupportedException nse)
+        {
+            reason = nse.Message;
+            return true;
+        }
+        catch
+        {
+            // not concerned with other exceptions
+        }
+
+        reason = null;
+        return false;
+    }
+
+    private string? GenPInvokeDecl(PInvoke pinvoke, bool treatAsVariadic)
     {
         var sb = new StringBuilder();
         var method = pinvoke.Method;
@@ -215,15 +270,33 @@ private string GenPInvokeDecl(PInvoke pinvoke)
             sb.Append($"int {pinvoke.EntryPoint} (int, int, int, int, int);");
             return sb.ToString();
         }
+
+        if (TryIsMethodGetParametersUnsupported(pinvoke.Method, out string? reason))
+        {
+            Log.LogWarning($"Skipping the following DllImport because '{reason}'. {Environment.NewLine}  {pinvoke.Method}");
+            pinvoke.Skip = true;
+            return null;
+        }
+
         sb.Append(MapType(method.ReturnType));
         sb.Append($" {pinvoke.EntryPoint} (");
-        int pindex = 0;
-        var pars = method.GetParameters();
-        foreach (var p in pars) {
-            if (pindex > 0)
-                sb.Append(',');
-            sb.Append(MapType(pars[pindex].ParameterType));
-            pindex++;
+        if (!treatAsVariadic)
+        {
+            int pindex = 0;
+            var pars = method.GetParameters();
+            foreach (var p in pars) {
+                if (pindex > 0)
+                    sb.Append(',');
+                sb.Append(MapType(pars[pindex].ParameterType));
+                pindex++;
+            }
+        }
+        else
+        {
+            // FIXME: handle sigs with different first args
+            ParameterInfo firstParam = method.GetParameters()[0];
+            sb.Append(MapType(firstParam.ParameterType));
+            sb.Append(", ...");
         }
         sb.Append(");");
         return sb.ToString();
@@ -366,7 +439,7 @@ private static bool IsBlittable (Type type)
     private static void Error (string msg) => throw new LogAsErrorException(msg);
 }
 
-internal class PInvoke
+internal class PInvoke : IEquatable<PInvoke>
 {
     public PInvoke(string entryPoint, string module, MethodInfo method)
     {
@@ -379,6 +452,30 @@ public PInvoke(string entryPoint, string module, MethodInfo method)
     public string Module;
     public MethodInfo Method;
     public bool Skip;
+
+    public bool Equals(PInvoke? other)
+        => other != null &&
+            string.Equals(EntryPoint, other.EntryPoint, StringComparison.Ordinal) &&
+            string.Equals(Module, other.Module, StringComparison.Ordinal) &&
+            string.Equals(Method.ToString(), other.Method.ToString(), StringComparison.Ordinal);
+
+    public override string ToString() => $"{{ EntryPoint: {EntryPoint}, Module: {Module}, Method: {Method}, Skip: {Skip} }}";
+}
+
+internal class PInvokeComparer : IEqualityComparer<PInvoke>
+{
+    public bool Equals(PInvoke? x, PInvoke? y)
+    {
+        if (x == null && y == null)
+            return true;
+        if (x == null || y == null)
+            return false;
+
+        return x.Equals(y);
+    }
+
+    public int GetHashCode(PInvoke pinvoke)
+        => $"{pinvoke.EntryPoint}{pinvoke.Module}{pinvoke.Method}".GetHashCode();
 }
 
 internal class PInvokeCallback

src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmBuildPublishTests.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
 using System.IO;
 using System.Linq;
 using Xunit;

src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs

@@ -79,6 +79,7 @@ private CommandResult PublishForRequiresWorkloadTest(string config, string extra
         [Theory]
         [InlineData("Debug")]
         [InlineData("Release")]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/59538")]
         public void Net50Projects_NativeReference(string config)
             => BuildNet50Project(config, aot: false, expectError: true, @"<NativeFileReference Include=""native-lib.o"" />");
 
@@ -92,6 +93,7 @@ public void Net50Projects_NativeReference(string config)
 
         [Theory]
         [MemberData(nameof(Net50TestData))]
+        [ActiveIssue("https://github.com/dotnet/runtime/issues/59538")]
         public void Net50Projects_AOT(string config, bool aot, bool expectError)
             => BuildNet50Project(config, aot: aot, expectError: expectError);
 

src/tests/BuildWasmApps/Wasm.Build.Tests/NativeLibraryTests.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System;
 using System.IO;
 using Xunit;
 using Xunit.Abstractions;

src/tests/BuildWasmApps/Wasm.Build.Tests/NativeRebuildTests/NativeRebuildTestsBase.cs

@@ -77,6 +77,7 @@ protected string Rebuild(bool nativeRelink, bool invariant, BuildArgs buildArgs,
             buildArgs = newBuildArgs;
 
             _testOutput.WriteLine($"{Environment.NewLine}Rebuilding with no changes ..{Environment.NewLine}");
+            Console.WriteLine($"{Environment.NewLine}Rebuilding with no changes ..{Environment.NewLine}");
             (_, string output) = BuildProject(buildArgs,
                                             id: id,
                                             dotnetWasmFromRuntimePack: false,
@@ -137,6 +138,18 @@ internal void CompareStat(IDictionary<string, FileStat> oldStat, IDictionary<str
                 throw new XunitException($"CompareStat failed:{Environment.NewLine}{msg}");
         }
 
+        internal IDictionary<string, (string fullPath, bool unchanged)> GetFilesTable(bool unchanged, params string[] baseDirs)
+        {
+            var dict = new Dictionary<string, (string fullPath, bool unchanged)>();
+            foreach (var baseDir in baseDirs)
+            {
+                foreach (var file in Directory.EnumerateFiles(baseDir, "*", new EnumerationOptions { RecurseSubdirectories = true }))
+                    dict[Path.GetFileName(file)] = (file, unchanged);
+            }
+
+            return dict;
+        }
+
         internal IDictionary<string, (string fullPath, bool unchanged)> GetFilesTable(BuildArgs buildArgs, BuildPaths paths, bool unchanged)
         {
             List<string> files = new()