Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wasm] Fix regression in compiling .bc -> .o files #56063

Merged
merged 8 commits into from
Jul 21, 2021
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@
<SQLitePCLRawbundle_greenVersion>2.0.4</SQLitePCLRawbundle_greenVersion>
<MoqVersion>4.12.0</MoqVersion>
<FsCheckVersion>2.14.3</FsCheckVersion>
<SdkVersionForWorkloadTesting>6.0.100-preview.7.21362.5</SdkVersionForWorkloadTesting>
<SdkVersionForWorkloadTesting>6.0.100-rc.1.21370.2</SdkVersionForWorkloadTesting>
<!-- Docs -->
<MicrosoftPrivateIntellisenseVersion>5.0.0-preview-20201009.2</MicrosoftPrivateIntellisenseVersion>
<!-- ILLink -->
Expand Down
14 changes: 11 additions & 3 deletions src/mono/wasm/build/WasmApp.Native.targets
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@
<EmccLinkOptimizationFlag Condition="'$(EmccLinkOptimizationFlag)' == ''" >$(EmccCompileOptimizationFlag)</EmccLinkOptimizationFlag>

<_EmccCompileRsp>$(_WasmIntermediateOutputPath)emcc-compile.rsp</_EmccCompileRsp>
<_EmccCompileOutputMessageImportance Condition="'$(EmccVerbose)' == 'true'">Normal</_EmccCompileOutputMessageImportance>
<_EmccCompileOutputMessageImportance Condition="'$(EmccVerbose)' != 'true'">Low</_EmccCompileOutputMessageImportance>
</PropertyGroup>

<ItemGroup>
Expand All @@ -181,6 +183,7 @@
<_EmccCFlags Include="-DLINK_ICALLS=1" Condition="'$(WasmLinkIcalls)' == 'true'" />
<_EmccCFlags Include="-DCORE_BINDINGS" />
<_EmccCFlags Include="-DGEN_PINVOKE=1" />
<_EmccCFlags Include="-emit-llvm" />

Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this be passed to the .bc -> .o builds as well ?

Copy link
Member Author

Choose a reason for hiding this comment

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

So, looks like I broke this in the last few days! Adding here is fine, because it will get used only to compile the .c files. And the bitcode files will use the LDFlags.

I've added a crude test to check for this, so we don't regress again.

<_EmccCFlags Include="&quot;-I%(_EmccIncludePaths.Identity)&quot;" />
<_EmccCFlags Include="-g" Condition="'$(WasmNativeDebugSymbols)' == 'true'" />
Expand Down Expand Up @@ -240,7 +243,11 @@
<Exec Command="$(_EmBuilder) build MINIMAL" EnvironmentVariables="@(EmscriptenEnvVars)" StandardOutputImportance="Low" StandardErrorImportance="Low" />

<Message Text="Compiling native assets with emcc. This may take a while ..." Importance="High" />
<EmccCompile SourceFiles="@(_WasmSourceFileToCompile)" Arguments='"@$(_EmccDefaultFlagsRsp)" "@$(_EmccCompileRsp)"' EnvironmentVariables="@(EmscriptenEnvVars)" />
<EmccCompile
SourceFiles="@(_WasmSourceFileToCompile)"
Arguments='"@$(_EmccDefaultFlagsRsp)" "@$(_EmccCompileRsp)"'
EnvironmentVariables="@(EmscriptenEnvVars)"
OutputMessageImportance="$(_EmccCompileOutputMessageImportance)" />

<ItemGroup>
<WasmNativeAsset Include="%(_WasmSourceFileToCompile.ObjectFile)" />
Expand Down Expand Up @@ -269,8 +276,9 @@
<EmccCompile
Condition="@(_BitCodeFile->Count()) > 0"
SourceFiles="@(_BitCodeFile)"
Arguments="&quot;@$(_EmccDefaultFlagsRsp)&quot; &quot;@$(_EmccCompileRsp)&quot;"
EnvironmentVariables="@(EmscriptenEnvVars)" />
Arguments="&quot;@$(_EmccDefaultFlagsRsp)&quot; @(_EmccLDFlags, ' ')"
EnvironmentVariables="@(EmscriptenEnvVars)"
OutputMessageImportance="$(_EmccCompileOutputMessageImportance)" />

<ItemGroup>
<!-- order seems to matter -->
Expand Down
11 changes: 9 additions & 2 deletions src/tasks/WasmAppBuilder/EmccCompile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class EmccCompile : Microsoft.Build.Utilities.Task
public bool DisableParallelCompile { get; set; }
public string Arguments { get; set; } = string.Empty;
public string? WorkingDirectory { get; set; }
public string OutputMessageImportance{ get; set; } = "Low";

[Output]
public ITaskItem[]? OutputFiles { get; private set; }
Expand All @@ -54,6 +55,12 @@ public override bool Execute()
return false;
}

if (!Enum.TryParse(OutputMessageImportance, ignoreCase: true, out MessageImportance messageImportance))
{
Log.LogError($"Invalid value for OutputMessageImportance={OutputMessageImportance}. Valid values: {string.Join(", ", Enum.GetNames(typeof(MessageImportance)))}");
return false;
}

IDictionary<string, string> envVarsDict = GetEnvironmentVariablesDict();
ConcurrentBag<ITaskItem> outputItems = new();
try
Expand Down Expand Up @@ -112,7 +119,7 @@ bool ProcessSourceFile(ITaskItem srcItem)

try
{
string command = $"emcc {Arguments} -c -o {objFile} {srcFile}";
string command = $"emcc {Arguments} -c -o \"{objFile}\" \"{srcFile}\"";

// Log the command in a compact format which can be copy pasted
StringBuilder envStr = new StringBuilder(string.Empty);
Expand All @@ -125,7 +132,7 @@ bool ProcessSourceFile(ITaskItem srcItem)
envVarsDict,
workingDir: Environment.CurrentDirectory,
logStdErrAsMessage: true,
debugMessageImportance: MessageImportance.High,
debugMessageImportance: messageImportance,
label: Path.GetFileName(srcFile));

if (exitCode != 0)
Expand Down
18 changes: 13 additions & 5 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/BlazorWasmTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@ public BlazorWasmTests(ITestOutputHelper output, SharedBuildPerTestClassFixture
{
}

[ConditionalFact(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
public void PublishTemplateProject()
// TODO: invariant case?

[ConditionalTheory(typeof(BuildTestBase), nameof(IsUsingWorkloads))]
[InlineData("Debug", false)]
[InlineData("Debug", true)] // just aot
[InlineData("Release", false)] // should re-link
[InlineData("Release", true)]
public void PublishTemplateProject(string config, bool aot)
{
string id = "blazorwasm";
string id = $"blazorwasm_{config}_aot_{aot}";
InitPaths(id);
if (Directory.Exists(_projectDir))
Directory.Delete(_projectDir, recursive: true);
Expand All @@ -37,14 +43,16 @@ public void PublishTemplateProject()
.ExecuteWithCapturedOutput("new blazorwasm")
.EnsureSuccessful();

string publishLogPath = Path.Combine(logPath, $"{id}.publish.binlog");
string publishLogPath = Path.Combine(logPath, $"{id}.binlog");
new DotNetCommand(s_buildEnv)
.WithWorkingDirectory(_projectDir)
.ExecuteWithCapturedOutput("publish", $"-bl:{publishLogPath}", "-p:RunAOTCompilation=true")
.ExecuteWithCapturedOutput("publish", $"-bl:{publishLogPath}", aot ? "-p:RunAOTCompilation=true" : "", $"-p:Configuration={config}")
.EnsureSuccessful();

//TODO: validate the build somehow?
// compare dotnet.wasm?
// relinking - dotnet.wasm should be smaller
//
// playwright?
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/BuildTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,7 @@ protected static BuildArgs ExpandBuildArgs(BuildArgs buildArgs, string extraProp
}

if (useCache)
{
_buildContext.CacheBuild(buildArgs, new BuildProduct(_projectDir, logFilePath, true));
Console.WriteLine($"caching build for {buildArgs}");
}

return (_projectDir, result.buildOutput);
}
Expand Down
34 changes: 33 additions & 1 deletion src/tests/BuildWasmApps/Wasm.Build.Tests/NativeBuildTests.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
// 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;
using Xunit.Sdk;

#nullable enable

Expand Down Expand Up @@ -43,5 +43,37 @@ private void NativeBuild(string projectNamePrefix, string projectContents, Build
test: output => {},
host: host, id: id);
}

[Theory]
[BuildAndRun(host: RunHost.None, aot: true)]
public void IntermediateBitcodeToObjectFilesAreNotLLVMIR(BuildArgs buildArgs, string id)
{
string printFileTypeTarget = @"
<Target Name=""PrintIntermediateFileType"" AfterTargets=""WasmBuildApp"">
<Exec Command=""wasm-dis $(_WasmIntermediateOutputPath)System.Private.CoreLib.dll.o -o $(_WasmIntermediateOutputPath)wasm-dis-out.txt""
ConsoleToMSBuild=""true""
EnvironmentVariables=""@(EmscriptenEnvVars)""
IgnoreExitCode=""true"">

<Output TaskParameter=""ExitCode"" PropertyName=""ExitCode"" />
</Exec>

<Message Text=""wasm-dis exit code: $(ExitCode)"" Importance=""High"" />
</Target>
";
string projectName = $"bc_to_o_{buildArgs.Config}";

buildArgs = buildArgs with { ProjectName = projectName };
buildArgs = ExpandBuildArgs(buildArgs, insertAtEnd: printFileTypeTarget);

(_, string output) = BuildProject(buildArgs,
initProject: () => File.WriteAllText(Path.Combine(_projectDir!, "Program.cs"), s_mainReturns42),
dotnetWasmFromRuntimePack: false,
id: id);

if (!output.Contains("wasm-dis exit code: 0"))
throw new XunitException($"Expected to successfully run wasm-dis on System.Private.CoreLib.dll.o ."
+ " It might fail if it was incorrectly compiled to a bitcode file, instead of wasm.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ private void RemoveDirectory(string path)
{
try
{
Directory.Delete(path, recursive: true);
Directory.Delete(path, recursive: true);
}
catch (Exception ex)
{
Expand Down
2 changes: 2 additions & 0 deletions src/tests/BuildWasmApps/Wasm.Build.Tests/ToolCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
return;

output.Add($"[{_label}] {e.Data}");
Console.WriteLine($"[{_label}] {e.Data}");
ErrorDataReceived?.Invoke(s, e);
};

Expand All @@ -97,6 +98,7 @@ private async Task<CommandResult> ExecuteAsyncInternal(string executable, string
return;

output.Add($"[{_label}] {e.Data}");
Console.WriteLine($"[{_label}] {e.Data}");
OutputDataReceived?.Invoke(s, e);
};

Expand Down