-
Notifications
You must be signed in to change notification settings - Fork 5k
[wasm] Endian fix for Webcil #92274
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] Endian fix for Webcil #92274
Conversation
2671ac0
to
ab6a917
Compare
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)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
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)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
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)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
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)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) |
fc4818f
to
894ce35
Compare
foreach (var sectionHeader in sectionsHeaders) | ||
{ | ||
WriteSectionHeader(s, sectionHeader); | ||
} | ||
} | ||
|
||
#if NETCOREAPP2_1_OR_GREATER |
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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 :)
There was a problem hiding this comment.
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
894ce35
to
5a5b543
Compare
Tagging subscribers to 'arch-wasm': @lewing 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
|
There was a problem hiding this comment.
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))
5a5b543
to
99df3e2
Compare
@lambdageek I have done the suggested changes, Thank you for the input. |
@@ -16,6 +16,7 @@ | |||
|
|||
<ItemGroup> | |||
<!-- we need to keep the version of System.Reflection.Metadata in sync with dotnet/msbuild and dotnet/sdk --> | |||
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" /> | |
<PackageReference Include="System.Memory" Version="$(SystemMemoryVersion)" /> |
'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.
99df3e2
to
72f310a
Compare
@lambdageek @radical @teo-tsirpanis can this be merged? |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
WasmBuildTests has relevant failures, like:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops
Moved the endianess fixup for |
/azp run runtime-wasm |
Azure Pipelines successfully started running 1 pipeline(s). |
This is failing in .NET 8 - can this be merged there? |
/backport to release/8.0 |
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6278015202 |
'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