Skip to content

[wasm][coreclr] Fix browser-wasm library tests broken by webcil V1 version bump #126698

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.

Sign up for GitHub

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

Closed
radekdoulik wants to merge 1 commit into from
+2 −2
Closed

[wasm][coreclr] Fix browser-wasm library tests broken by webcil V1 version bump #126698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilConverter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public unsafe void GatherInfo(PEReader peReader, out WCFileInfo wcInfo, out PEFi
var sections = headers.SectionHeaders;
WebcilHeader header = default;
header.Id = WebcilConstants.WEBCIL_MAGIC;
header.VersionMajor = WebcilConstants.WC_VERSION_MAJOR;
header.VersionMajor = 0; // WASM-TODO: add webcil V1 support
header.VersionMinor = WebcilConstants.WC_VERSION_MINOR;
header.CoffSections = (ushort)coffHeader.NumberOfSections;
Comment on lines 118 to 122
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

Hard-coding 0 for VersionMajor here works, but it introduces a magic number that’s now duplicated with WebcilReader. To keep this easier to update when v1 support is added, consider introducing a named constant in this project (e.g., const ushort SupportedWebcilVersionMajor = 0) and using it in both reader and converter.

Copilot uses AI. Check for mistakes.
header.Reserved0 = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ private unsafe bool ReadHeader()
header.PeDebugSize = BinaryPrimitives.ReverseEndianness(header.PeDebugSize);
}
if (header.Id != WebcilConstants.WEBCIL_MAGIC
|| header.VersionMajor != WebcilConstants.WC_VERSION_MAJOR
|| header.VersionMajor != 0 // WASM-TODO: add webcil V1 support
|| header.VersionMinor != WebcilConstants.WC_VERSION_MINOR)
Comment on lines 81 to 83
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

Hard-coding 0 for VersionMajor is intentional for now, but it creates a duplicated magic number (also in WebcilConverter). Consider using a shared named constant (within this project) for the supported major version to reduce the chance of future inconsistencies when v1 support is implemented.

Copilot uses AI. Check for mistakes.
{
return false;
Comment on lines 81 to 85
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

ReadHeader() returning false on a version mismatch causes the constructor to throw "Stream does not contain a valid Webcil file". With Webcil v1 now existing, a v1 image is valid but unsupported here, so the exception is misleading. Consider detecting the version mismatch explicitly and throwing a BadImageFormatException (or similar) that reports the unsupported version (e.g., major/minor) to make failures actionable.

Copilot uses AI. Check for mistakes.
Expand Down
Loading