Skip to content

[wasm] Endian fix for Webcil - #92274

Merged
lambdageek merged 3 commits into
dotnet:mainfrom
saitama951:fix-endian-webcil
Sep 22, 2023
Merged

lambdageek merged 3 commits into
dotnet:mainfrom
saitama951:fix-endian-webcil

Conversation

@saitama951

@saitama951 saitama951 commented Sep 19, 2023 •

Copy link
Copy Markdown
Contributor

'dotnet new blazorwasm' command failed on s390x and was throwing a not implemented exception

The issue was with with the WebCil writer and reader, specific endianness conversions relating to the webcil payload were not implemented for big endian machines.

We considered fixing the generic implementation, but there were only two structures in use: WebcilHeader and WebcilSectionHeader, so it was easier to handle them explicitly.

fixes #91893
cc: @omajid @iii-i @uweigand

@ghost ghost added area-Build-mono community-contribution Indicates that the PR has been added by a community member labels Sep 19, 2023
@saitama951
saitama951 force-pushed the fix-endian-webcil branch 3 times, most recently from 2671ac0 to ab6a917 Compare September 19, 2023 16:00
Comment on lines 198 to 201

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
virtualSize: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(sectionHeader.VirtualSize)),
virtualAddress: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(sectionHeader.VirtualAddress)),
sizeOfRawData: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(sectionHeader.SizeOfRawData)),
pointerToRawData: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(sectionHeader.PointerToRawData))
virtualSize: BinaryPrimitives.ReverseEndianess(sectionHeader.VirtualSize),
virtualAddress: BinaryPrimitives.ReverseEndianess(sectionHeader.VirtualAddress),
sizeOfRawData: BinaryPrimitives.ReverseEndianess(sectionHeader.SizeOfRawData),
pointerToRawData: BinaryPrimitives.ReverseEndianess(sectionHeader.PointerToRawData)

Comment on lines 230 to 236

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
webcilHeader.version_major = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes( webcilHeader.version_major));
webcilHeader.version_minor = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes( webcilHeader.version_minor));
webcilHeader.coff_sections = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes(webcilHeader.coff_sections));
webcilHeader.pe_cli_header_rva = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(webcilHeader.pe_cli_header_rva));
webcilHeader.pe_cli_header_size = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(webcilHeader.pe_cli_header_size));
webcilHeader.pe_debug_rva = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(webcilHeader.pe_debug_rva));
webcilHeader.pe_debug_size = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(webcilHeader.pe_debug_size));
webcilHeader.version_major = BinaryPrimitives.ReverseEndianess(webcilHeader.version_major);
webcilHeader.version_minor = BinaryPrimitives.ReverseEndianess(webcilHeader.version_minor);
webcilHeader.coff_sections = BinaryPrimitives.ReverseEndianess(webcilHeader.coff_sections);
webcilHeader.pe_cli_header_rva = BinaryPrimitives.ReverseEndianess(webcilHeader.pe_cli_header_rva);
webcilHeader.pe_cli_header_size = BinaryPrimitives.ReverseEndianess(webcilHeader.pe_cli_header_size);
webcilHeader.pe_debug_rva = BinaryPrimitives.ReverseEndianess(webcilHeader.pe_debug_rva);
webcilHeader.pe_debug_size = BinaryPrimitives.ReverseEndianess(webcilHeader.pe_debug_size);

Comment on lines 72 to 78

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
header.version_major = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes(header.version_major));
header.version_minor = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes(header.version_minor));
header.coff_sections = BinaryPrimitives.ReadUInt16LittleEndian(BitConverter.GetBytes(header.coff_sections));
header.pe_cli_header_rva = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(header.pe_cli_header_rva));
header.pe_cli_header_size = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(header.pe_cli_header_size));
header.pe_debug_rva = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(header.pe_debug_rva));
header.pe_debug_rva = BinaryPrimitives.ReadUInt32LittleEndian(BitConverter.GetBytes(header.pe_debug_size));
header.version_major = BinaryPrimitives.ReverseEndianess(header.version_major);
header.version_minor = BinaryPrimitives.ReverseEndianess(header.version_minor);
header.coff_sections = BinaryPrimitives.ReverseEndianess(header.coff_sections);
header.pe_cli_header_rva = BinaryPrimitives.ReverseEndianess(header.pe_cli_header_rva);
header.pe_cli_header_size = BinaryPrimitives.ReverseEndianess(header.pe_cli_header_size);
header.pe_debug_rva = BinaryPrimitives.ReverseEndianess(header.pe_debug_rva);
header.pe_debug_rva = BinaryPrimitives.ReverseEndianess(header.pe_debug_size);

Comment on lines 375 to 378

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
virtualSize: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(secheader.VirtualSize)),
virtualAddress: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(secheader.VirtualAddress)),
sizeOfRawData: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(secheader.SizeOfRawData)),
pointerToRawData: BinaryPrimitives.ReadInt32LittleEndian(BitConverter.GetBytes(secheader.PointerToRawData))
virtualSize: BinaryPrimitives.ReverseEndianess(secheader.VirtualSize),
virtualAddress: BinaryPrimitives.ReverseEndianess(secheader.VirtualAddress),
sizeOfRawData: BinaryPrimitives.ReverseEndianess(secheader.SizeOfRawData),
pointerToRawData: BinaryPrimitives.ReverseEndianess(secheader.PointerToRawData)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks. Can you try removing the #if if it compiles? This function might be available on all frameworks (unless I'm missing something).

@saitama951 saitama951 Sep 19, 2023 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. I have done the same for the WriteStructure as well and hence removing the endian checks for net472 as well. making it generic :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's a little more subtle than just having the API on net472. Because this code is used in an MSBuild task that might be called from a build in Visual Studio, the API has to be in whatever old version of the out of band assembly is already loaded in to the MSBuild process that invokes the task.

@saitama951 Hmm... actually we might also need to add this:

    <PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />

to src/tasks/Microsoft.NET.WebAssembly.Webcil/Microsoft.NET.WebAssembly.Webcil.csproj

@ghost

ghost commented Sep 20, 2023

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

'dotnet new blazorwasm' command failed on s390x and was throwing a not implemented exception

The issue was with with the WebCil writer and reader, specific endianness conversions relating to the webcil payload were not implemented for big endian machines.

We considered fixing the generic implementation, but there were only two structures in use: WebcilHeader and WebcilSectionHeader, so it was easier to handle them explicitly.

fixes #91893
cc: @omajid @iii-i @uweigand

Author: saitama951
Assignees: -
Labels:

arch-wasm, area-Build-mono, community-contribution

Milestone: -

@maraf
maraf requested a review from lambdageek September 20, 2023 07:56
@maraf maraf added this to the 9.0.0 milestone Sep 20, 2023

@lambdageek lambdageek left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks!

Update Please add the PackageReference to the csproj (see #92274 (comment))

@saitama951

Copy link
Copy Markdown
Contributor Author

thanks!

Update Please add the PackageReference to the csproj (see #92274 (comment))

@lambdageek I have done the suggested changes, Thank you for the input.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" />

Comment thread src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilConverter.cs Outdated
'dotnet new blazorwasm' command failed on s390x and was throwing a not implemented exception

The issue was with with the WebCil writer and reader, specific endianness conversions relating to the webcil payload were not implemented for big endian machines.

We considered fixing the generic implementation, but there were only two structures in use: WebcilHeader and WebcilSectionHeader, so it was easier to handle them explicitly.
@saitama951

Copy link
Copy Markdown
Contributor Author

@lambdageek @radical @teo-tsirpanis can this be merged?

@radical

radical commented Sep 21, 2023

Copy link
Copy Markdown
Member

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@radical

radical commented Sep 21, 2023

Copy link
Copy Markdown
Member

WasmBuildTests has relevant failures, like:

      [] Stack overflow.
      [] Repeat 74781 times:
      [] --------------------------------
      []    at Microsoft.NET.WebAssembly.Webcil.WebcilConverter.WriteStructure(System.IO.Stream, Microsoft.NET.WebAssembly.Webcil.WebcilHeader)
      [] --------------------------------
      []    at Microsoft.NET.WebAssembly.Webcil.WebcilConverter.WriteHeader(System.IO.Stream, Microsoft.NET.WebAssembly.Webcil.WebcilHeader)
      []    at Microsoft.NET.WebAssembly.Webcil.WebcilConverter.WriteConversionTo(System.IO.Stream, System.IO.FileStream, PEFileInfo, WCFileInfo)
      []    at Microsoft.NET.WebAssembly.Webcil.WebcilConverter.ConvertToWebcil()
      []    at Microsoft.WebAssembly.Build.Tasks.WebcilConverter.ConvertToWebcil()
      []    at Microsoft.WebAssembly.Build.Tasks.WasmAppBuilder.ExecuteInternal()
      []    at Microsoft.WebAssembly.Build.Tasks.WasmAppBuilderBaseTask.Execute()
      []    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
      []    at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__25.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TaskBuilder+<ExecuteInstantiatedTask>d__25, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ExecuteInstantiatedTask>d__25 ByRef)
      []    at Microsoft.Build.BackEnd.TaskBuilder+<InitializeAndExecuteTask>d__23.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TaskBuilder+<InitializeAndExecuteTask>d__23, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<InitializeAndExecuteTask>d__23 ByRef)
      []    at Microsoft.Build.BackEnd.TaskBuilder.InitializeAndExecuteTask(Microsoft.Build.BackEnd.Logging.TaskLoggingContext, Microsoft.Build.BackEnd.ItemBucket, System.Collections.Generic.IDictionary`2<System.String,System.String>, Microsoft.Build.BackEnd.TaskHost, Microsoft.Build.BackEnd.TaskExecutionMode)
      []    at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteBucket>d__19.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TaskBuilder+<ExecuteBucket>d__19, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ExecuteBucket>d__19 ByRef)
      []    at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteTask>d__18.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TaskBuilder+<ExecuteTask>d__18, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ExecuteTask>d__18 ByRef)
      []    at Microsoft.Build.BackEnd.TaskBuilder+<ExecuteTask>d__13.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TaskBuilder+<ExecuteTask>d__13, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ExecuteTask>d__13 ByRef)
      []    at Microsoft.Build.BackEnd.TargetEntry+<ProcessBucket>d__51.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TargetEntry+<ProcessBucket>d__51, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ProcessBucket>d__51 ByRef)
      []    at Microsoft.Build.BackEnd.TargetEntry+<ExecuteTarget>d__44.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TargetEntry+<ExecuteTarget>d__44, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ExecuteTarget>d__44 ByRef)
      []    at Microsoft.Build.BackEnd.TargetEntry.ExecuteTarget(Microsoft.Build.BackEnd.ITaskBuilder, Microsoft.Build.BackEnd.BuildRequestEntry, Microsoft.Build.BackEnd.Logging.ProjectLoggingContext, System.Threading.CancellationToken)
      []    at Microsoft.Build.BackEnd.TargetBuilder+<ProcessTargetStack>d__23.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TargetBuilder+<ProcessTargetStack>d__23, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ProcessTargetStack>d__23 ByRef)
      []    at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[Microsoft.Build.BackEnd.TargetBuilder+<ProcessTargetStack>d__23, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<ProcessTargetStack>d__23 ByRef)
      []    at Microsoft.Build.BackEnd.TargetBuilder.ProcessTargetStack(Microsoft.Build.BackEnd.ITaskBuilder)
      []    at Microsoft.Build.BackEnd.TargetBuilder+<BuildTargets>d__10.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.TargetBuilder+<BuildTargets>d__10, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildTargets>d__10 ByRef)
      []    at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Start[[Microsoft.Build.BackEnd.TargetBuilder+<BuildTargets>d__10, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildTargets>d__10 ByRef)
      []    at Microsoft.Build.BackEnd.TargetBuilder.BuildTargets(Microsoft.Build.BackEnd.Logging.ProjectLoggingContext, Microsoft.Build.BackEnd.BuildRequestEntry, Microsoft.Build.BackEnd.IRequestBuilderCallback, System.String[], Microsoft.Build.BackEnd.Lookup, System.Threading.CancellationToken)
      []    at Microsoft.Build.BackEnd.RequestBuilder+<BuildProject>d__68.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.RequestBuilder+<BuildProject>d__68, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildProject>d__68 ByRef)
      []    at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].Start[[Microsoft.Build.BackEnd.RequestBuilder+<BuildProject>d__68, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildProject>d__68 ByRef)
      []    at Microsoft.Build.BackEnd.RequestBuilder.BuildProject()
      []    at Microsoft.Build.BackEnd.RequestBuilder+<BuildAndReport>d__59.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.RequestBuilder+<BuildAndReport>d__59, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildAndReport>d__59 ByRef)
      []    at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[Microsoft.Build.BackEnd.RequestBuilder+<BuildAndReport>d__59, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<BuildAndReport>d__59 ByRef)
      []    at Microsoft.Build.BackEnd.RequestBuilder.BuildAndReport()
      []    at Microsoft.Build.BackEnd.RequestBuilder+<RequestThreadProc>d__58.MoveNext()
      []    at System.Runtime.CompilerServices.AsyncMethodBuilderCore.Start[[Microsoft.Build.BackEnd.RequestBuilder+<RequestThreadProc>d__58, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<RequestThreadProc>d__58 ByRef)
      []    at System.Runtime.CompilerServices.AsyncTaskMethodBuilder.Start[[Microsoft.Build.BackEnd.RequestBuilder+<RequestThreadProc>d__58, Microsoft.Build, Version=15.1.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a]](<RequestThreadProc>d__58 ByRef)
      []    at Microsoft.Build.BackEnd.RequestBuilder.RequestThreadProc(Boolean)
      []    at System.Threading.Tasks.Task`1[[System.__Canon, System.Private.CoreLib, Version=8.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]].InnerInvoke()
      []    at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
      []    at System.Threading.Tasks.Task.ExecuteWithThreadLocal(System.Threading.Tasks.Task ByRef, System.Threading.Thread)
      []    at System.Threading.Tasks.Task.ExecuteEntry()
      []    at Microsoft.Build.BackEnd.RequestBuilder+DedicatedThreadsTaskScheduler.<InjectThread>b__6_0()
      []    at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)

@lambdageek lambdageek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oops

@lambdageek

Copy link
Copy Markdown
Member

Moved the endianess fixup for WebcilHeader to WriteHeader instead of an overload of WriteStructure /cc @saitama951

@lambdageek

Copy link
Copy Markdown
Member

/azp run runtime-wasm

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@uweigand

Copy link
Copy Markdown
Contributor

This is failing in .NET 8 - can this be merged there?

@lambdageek

Copy link
Copy Markdown
Member

/backport to release/8.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6278015202

@ghost ghost locked as resolved and limited conversation to collaborators Oct 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Build-mono community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The "ConvertDllsToWebCil" task failed: System.NotImplementedException on s390x

6 participants