[wasm] Set WasmEnableWebcil to true by default (#86330)

* [wasm] Set WasmEnableWebcil to true by default * WebcilReader: copy updated struct for BlobReader.ReadUtf8NullTerminated When using reflection to call a method on BlobReader, make sure to copy the struct after the call since the internal state is modified Fixes PdbChecksum extraction from Webcil files (This was a problem even before webcil-in-wasm)
dotnet · May 18, 2023 · 7b0d8fe · 7b0d8fe
1 parent aef327f
commit 7b0d8fe
Show file tree

Hide file tree

Showing 10 changed files with 32 additions and 24 deletions.
diff --git a/src/libraries/Microsoft.NET.WebAssembly.Webcil/src/Webcil/WebcilReader.Reflection.cs b/src/libraries/Microsoft.NET.WebAssembly.Webcil/src/Webcil/WebcilReader.Reflection.cs
@@ -29,7 +29,13 @@ internal static class Reflection
             return mi;
         });
 
-        internal static string? ReadUtf8NullTerminated(BlobReader reader) => (string?)s_readUtf8NullTerminated.Value.Invoke(reader, null);
+        internal static string? ReadUtf8NullTerminated(ref BlobReader reader)
+        {
+            object boxedReader = reader;
+            string? result = (string?)s_readUtf8NullTerminated.Value.Invoke(boxedReader, null);
+            reader = (BlobReader) boxedReader; // the call modifies the struct state, make sure to copy it back.
+            return result;
+        }
 
         private static readonly Lazy<ConstructorInfo> s_codeViewDebugDirectoryDataCtor = new Lazy<ConstructorInfo>(() =>
         {

diff --git a/src/libraries/Microsoft.NET.WebAssembly.Webcil/src/Webcil/WebcilReader.cs b/src/libraries/Microsoft.NET.WebAssembly.Webcil/src/Webcil/WebcilReader.cs
@@ -220,12 +220,12 @@ private static CodeViewDebugDirectoryData DecodeCodeViewDebugDirectoryData(BlobR
 
         Guid guid = reader.ReadGuid();
         int age = reader.ReadInt32();
-        string path = ReadUtf8NullTerminated(reader)!;
+        string path = ReadUtf8NullTerminated(ref reader)!;
 
         return MakeCodeViewDebugDirectoryData(guid, age, path);
     }
 
-    private static string? ReadUtf8NullTerminated(BlobReader reader) => Reflection.ReadUtf8NullTerminated(reader);
+    private static string? ReadUtf8NullTerminated(ref BlobReader reader) => Reflection.ReadUtf8NullTerminated(ref reader);
 
     private static CodeViewDebugDirectoryData MakeCodeViewDebugDirectoryData(Guid guid, int age, string path) => Reflection.MakeCodeViewDebugDirectoryData(guid, age, path);
 
@@ -316,7 +316,7 @@ public PdbChecksumDebugDirectoryData ReadPdbChecksumDebugDirectoryData(DebugDire
 
     private static PdbChecksumDebugDirectoryData DecodePdbChecksumDebugDirectoryData(BlobReader reader)
     {
-        var algorithmName = ReadUtf8NullTerminated(reader);
+        var algorithmName = ReadUtf8NullTerminated(ref reader);
         byte[]? checksum = reader.ReadBytes(reader.RemainingBytes);
         if (string.IsNullOrEmpty(algorithmName) || checksum == null || checksum.Length == 0)
         {

diff --git a/src/libraries/sendtohelix-wasm.targets b/src/libraries/sendtohelix-wasm.targets
@@ -276,7 +276,7 @@
       <BuildWasmAppsJobsList>$(RepositoryEngineeringDir)testing\scenarios\BuildWasmAppsJobsList.txt</BuildWasmAppsJobsList>
       <WorkItemPrefix Condition="'$(TestUsingWorkloads)' == 'true'">Workloads-</WorkItemPrefix>
       <WorkItemPrefix Condition="'$(TestUsingWorkloads)' != 'true'">NoWorkload-</WorkItemPrefix>
-      <WorkItemPrefix Condition="'$(TestUsingWebcil)' == 'true'">$(WorkItemPrefix)Webcil-</WorkItemPrefix>
+      <WorkItemPrefix Condition="'$(TestUsingWebcil)' == 'false'">$(WorkItemPrefix)NoWebcil-</WorkItemPrefix>
     </PropertyGroup>
 
     <ItemGroup>

diff --git a/src/libraries/sendtohelixhelp.proj b/src/libraries/sendtohelixhelp.proj
@@ -146,7 +146,7 @@
 
     <HelixCommandPrefixItem Include="DOTNET_CLI_TELEMETRY_OPTOUT=1" />
     <HelixCommandPrefixItem Condition="'$(TestUsingWorkloads)' == 'true'" Include="TEST_USING_WORKLOADS=true" />
-    <HelixCommandPrefixItem Condition="'$(TestUsingWebcil)' == 'true'" Include="TEST_USING_WEBCIL=true" />
+    <HelixCommandPrefixItem Condition="'$(TestUsingWebcil)' == 'false'" Include="TEST_USING_WEBCIL=false" />
   </ItemGroup>
 
   <PropertyGroup Condition="$(TargetRuntimeIdentifier.ToLowerInvariant().StartsWith('linux-bionic'))">

diff --git a/...et/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets b/...et/Microsoft.NET.Sdk.WebAssembly.Pack/build/Microsoft.NET.Sdk.WebAssembly.Browser.targets
@@ -171,7 +171,7 @@ Copyright (c) .NET Foundation. All rights reserved.
       <_WasmEnableThreads>$(WasmEnableThreads)</_WasmEnableThreads>
       <_WasmEnableThreads Condition="'$(_WasmEnableThreads)' == ''">false</_WasmEnableThreads>
       <_WasmEnableWebcil>$(WasmEnableWebcil)</_WasmEnableWebcil>
-      <_WasmEnableWebcil Condition="'$(_WasmEnableWebcil)' == ''">false</_WasmEnableWebcil>
+      <_WasmEnableWebcil Condition="'$(_WasmEnableWebcil)' == ''">true</_WasmEnableWebcil>
       <_BlazorWebAssemblyStartupMemoryCache>$(BlazorWebAssemblyStartupMemoryCache)</_BlazorWebAssemblyStartupMemoryCache>
       <_BlazorWebAssemblyJiterpreter>$(BlazorWebAssemblyJiterpreter)</_BlazorWebAssemblyJiterpreter>
       <_BlazorWebAssemblyRuntimeOptions>$(BlazorWebAssemblyRuntimeOptions)</_BlazorWebAssemblyRuntimeOptions>

diff --git a/src/mono/sample/wasm/browser-advanced/Wasm.Advanced.Sample.csproj b/src/mono/sample/wasm/browser-advanced/Wasm.Advanced.Sample.csproj
@@ -4,7 +4,7 @@
     <EnableAggressiveTrimming>true</EnableAggressiveTrimming>
     <PublishTrimmed>true</PublishTrimmed>
     <WasmEnableLegacyJsInterop>false</WasmEnableLegacyJsInterop>
-    <WasmEnableWebcil>true</WasmEnableWebcil>
+    <WasmEnableWebcil>false</WasmEnableWebcil>
     <WasmEmitSymbolMap>true</WasmEmitSymbolMap>
     <EmccEnableAssertions>true</EmccEnableAssertions>
     <EmccEnvironment>web,worker</EmccEnvironment>

diff --git a/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs b/src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs
@@ -83,6 +83,8 @@ static BuildTestBase()
                 Console.WriteLine($"=============== Running with {(s_buildEnv.IsWorkload ? "Workloads" : "No workloads")} ===============");
                 if (UseWebcil)
                     Console.WriteLine($"=============== Using webcil-in-wasm ===============");
+                else
+                    Console.WriteLine($"=============== Webcil disabled ===============");
                 Console.WriteLine($"==============================================================================================");
                 Console.WriteLine("");
             }
@@ -343,9 +345,9 @@ protected static BuildArgs ExpandBuildArgs(BuildArgs buildArgs, string extraProp
                 extraProperties += $"\n<EmccVerbose>{s_isWindows}</EmccVerbose>\n";
             }
 
-            if (UseWebcil)
+            if (!UseWebcil)
             {
-                extraProperties += "<WasmEnableWebcil>true</WasmEnableWebcil>\n";
+                extraProperties += "<WasmEnableWebcil>false</WasmEnableWebcil>\n";
             }
 
             extraItems += "<WasmExtraFilesToDeploy Include='index.html' />";
@@ -508,8 +510,8 @@ public string CreateWasmTemplateProject(string id, string template = "wasmbrowse
             extraProperties += "<TreatWarningsAsErrors>true</TreatWarningsAsErrors>";
             if (runAnalyzers)
                 extraProperties += "<RunAnalyzers>true</RunAnalyzers>";
-            if (UseWebcil)
-                extraProperties += "<WasmEnableWebcil>true</WasmEnableWebcil>";
+            if (!UseWebcil)
+                extraProperties += "<WasmEnableWebcil>false</WasmEnableWebcil>";
 
             // TODO: Can be removed after updated templates propagate in.
             string extraItems = string.Empty;
@@ -533,8 +535,8 @@ public string CreateBlazorWasmTemplateProject(string id)
                     .EnsureSuccessful();
 
             string projectFile = Path.Combine(_projectDir!, $"{id}.csproj");
-            if (UseWebcil)
-                AddItemsPropertiesToProject(projectFile, "<WasmEnableWebcil>true</WasmEnableWebcil>");
+            if (!UseWebcil)
+                AddItemsPropertiesToProject(projectFile, "<WasmEnableWebcil>false</WasmEnableWebcil>");
             return projectFile;
         }
 
@@ -595,7 +597,7 @@ public string CreateBlazorWasmTemplateProject(string id)
                 label, // same as the command name
                 $"-bl:{logPath}",
                 $"-p:Configuration={config}",
-                UseWebcil ? "-p:WasmEnableWebcil=true" : string.Empty,
+                !UseWebcil ? "-p:WasmEnableWebcil=false" : string.Empty,
                 "-p:BlazorEnableCompression=false",
                 "-nr:false",
                 setWasmDevel ? "-p:_WasmDevel=true" : string.Empty

diff --git a/src/mono/wasm/Wasm.Build.Tests/data/RunScriptTemplate.cmd b/src/mono/wasm/Wasm.Build.Tests/data/RunScriptTemplate.cmd
@@ -51,10 +51,10 @@ if [%TEST_USING_WORKLOADS%] == [true] (
 ) else (
     set SDK_HAS_WORKLOAD_INSTALLED=false
 )
-if [%TEST_USING_WEBCIL%] == [true] (
-   set USE_WEBCIL_FOR_TESTS=true
-) else (
+if [%TEST_USING_WEBCIL%] == [false] (
    set USE_WEBCIL_FOR_TESTS=false
+) else (
+   set USE_WEBCIL_FOR_TESTS=true
 )
 
 if [%HELIX_CORRELATION_PAYLOAD%] NEQ [] (

diff --git a/src/mono/wasm/Wasm.Build.Tests/data/RunScriptTemplate.sh b/src/mono/wasm/Wasm.Build.Tests/data/RunScriptTemplate.sh
@@ -33,10 +33,10 @@ function set_env_vars()
         export SDK_HAS_WORKLOAD_INSTALLED=false
     fi
 
-    if [ "x$TEST_USING_WEBCIL" = "xtrue" ]; then
-        export USE_WEBCIL_FOR_TESTS=true
-    else
+    if [ "x$TEST_USING_WEBCIL" = "xfalse" ]; then
         export USE_WEBCIL_FOR_TESTS=false
+    else
+        export USE_WEBCIL_FOR_TESTS=true
     fi
 
     local _SDK_DIR=

diff --git a/src/mono/wasm/build/WasmApp.targets b/src/mono/wasm/build/WasmApp.targets
@@ -71,7 +71,7 @@
       - $(WasmEnableLegacyJsInterop)        - Include support for legacy JS interop. Defaults to true.
       - $(WasmEnableExceptionHandling)      - Enable support for the WASM post MVP Exception Handling runtime extension.
       - $(WasmEnableSIMD)                   - Enable support for the WASM post MVP SIMD runtime extension.
-      - $(WasmEnableWebcil)                 - Enable conversion of assembly .dlls to Webcil wrapped in .wasm
+      - $(WasmEnableWebcil)                 - Enable conversion of assembly .dlls to Webcil wrapped in .wasm (default: true)
       - $(WasmIncludeFullIcuData)           - Loads full ICU data (icudt.dat). Defaults to false. Only applicable when InvariantGlobalization=false.
       - $(WasmIcuDataFileName)              - Name/path of ICU globalization file loaded to app. Only when InvariantGloblization=false and WasmIncludeFullIcuData=false.
       - $(WasmAllowUndefinedSymbols)        - Controls whether undefined symbols are allowed or not,
@@ -133,8 +133,8 @@
     <!-- if DebuggerSupport==true, then ensure that WasmDebugLevel isn't disabling debugging -->
     <WasmDebugLevel Condition="('$(WasmDebugLevel)' == '' or '$(WasmDebugLevel)' == '0') and ('$(DebuggerSupport)' == 'true' or '$(Configuration)' == 'Debug')">-1</WasmDebugLevel>
 
-    <!-- by default, don't package assemblies as webcil -->
-    <WasmEnableWebcil Condition="'$(WasmEnableWebcil)' == ''">false</WasmEnableWebcil>
+    <!-- by default, package assemblies as webcil -->
+    <WasmEnableWebcil Condition="'$(WasmEnableWebcil)' == ''">true</WasmEnableWebcil>
   </PropertyGroup>
 
   <ItemGroup>