Skip to content

[browser] Webcil alignment#124905

Merged
pavelsavara merged 2 commits intodotnet:mainfrom
pavelsavara:webcil_alignment
Feb 27, 2026
Merged

[browser] Webcil alignment#124905
pavelsavara merged 2 commits intodotnet:mainfrom
pavelsavara:webcil_alignment

Conversation

@pavelsavara
Copy link
Member

Webcil: naming cleanup, section alignment, and bug fixes

Summary

This PR improves the Webcil converter/reader with naming convention fixes, 16-byte section alignment, robust debug directory fixup, and a big-endian correctness bug fix.

Changes

Section alignment

  • Added 16-byte alignment for section data in webcil output files.
  • Sections are now padded with zeros to aligned positions, improving compatibility with loaders that expect aligned data.

Debug directory fixup improvements

  • Replaced the old constant-offset dataPointerAdjustment approach with a per-section TranslateFileOffset method that correctly maps PE file offsets to webcil file offsets even when sections have different relative positions (due to alignment).
  • Removed the now-unnecessary GetSectionFromFileOffset methods.
  • Made FixupDebugDirectoryEntries static since it no longer uses instance state.
  • Added debug directory size verification (previously a TODO).

Bug fixes

  • Big-endian byte-swap bug: Fixed WebcilReader.ReadHeader where pe_debug_size was incorrectly assigned to pe_debug_rva instead of pe_debug_size on big-endian platforms.
  • Added bounds checking in TranslateRVA and GetPositionOfRelativeVirtualAddress to detect RVAs that map beyond a section's raw data.

Naming cleanup

  • Renamed WebcilHeader fields from C-style snake_case (id, version_major, pe_cli_header_rva, etc.) to .NET PascalCase (Id, VersionMajor, PeCliHeaderRva, etc.).
  • Fixed filename typo: WebciHeader.csWebcilHeader.cs (was missing the 'l').
  • Normalized casing: ConvertDllsToWebCilConvertDllsToWebcil, WebCilCandidatesWebcilCandidates, IsWebCilEnabledIsWebcilEnabled.
  • Updated MSBuild targets to match the new names.

@pavelsavara pavelsavara added this to the 11.0.0 milestone Feb 26, 2026
@pavelsavara pavelsavara self-assigned this Feb 26, 2026
Copilot AI review requested due to automatic review settings February 26, 2026 15:38
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-Host os-browser Browser variant of arch-wasm labels Feb 26, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Webcil toolchain (converter/reader + build integration) to standardize naming, introduce 16-byte section alignment in Webcil payloads, and improve correctness around debug directory fixups and big-endian parsing.

Changes:

  • Align Webcil section raw data to 16-byte boundaries (with zero padding) and update debug-directory fixups to translate file offsets per-section rather than via a single constant adjustment.
  • Fix a big-endian header parsing bug in WebcilReader and add stricter RVA/bounds validation when translating RVAs to file offsets.
  • Rename Webcil-related APIs/targets/properties (e.g., WebCil*Webcil*, ConvertDllsToWebCilConvertDllsToWebcil) and update docs accordingly.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilWasmWrapper.cs Improves internal alignment assertion message for wasm-wrapped payload emission.
src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilReader.cs Fixes big-endian header parsing and adds bounds checking during RVA translation.
src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilHeader.cs Renames header fields to PascalCase while preserving layout.
src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilConverter.cs Adds 16-byte section alignment + padding, reworks debug directory pointer translation, and validates debug dir overwrite size.
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ConvertDllsToWebCil.cs Renames MSBuild task/class and output item list for consistent “Webcil” casing.
src/tasks/Microsoft.NET.Sdk.WebAssembly.Pack.Tasks/ComputeWasmPublishAssets.cs Renames IsWebCilEnabledIsWebcilEnabled and updates related logic.
src/mono/wasm/features.md Updates documentation casing to “Webcil”.
src/mono/wasm/Wasm.Build.Tests/Common/BuildEnvironment.cs Updates env var casing to match WasmEnableWebcil.
src/mono/nuget/.../Microsoft.NET.Sdk.WebAssembly.Browser.targets Updates UsingTask/task invocation/property names to the new “Webcil” casing.
src/mono/mono/metadata/webcil-loader.c Renames internal struct typedef to “Webcil” casing.
src/mono/cmake/options.cmake Updates option description casing.
src/mono/cmake/config.h.in Updates config comment casing.
src/mono/browser/debugger/BrowserDebugProxy/DebugStore.cs Updates comment casing and minor whitespace tweak.
docs/design/mono/webcil.md Updates spec text and documents 16-byte section alignment and rationale.
Comments suppressed due to low confidence (3)

src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilConverter.cs:260

  • CopySections computes padding using (int)outStream.Position and silently skips padding when the current position is already past the expected PointerToRawData (negative paddingNeeded). This can hide layout bugs and can also overflow/truncate for outputs >2GB. Consider doing the math in long, and throw if outStream.Position is greater than the expected section start (or if the padding would be unexpectedly large), so corrupted Webcil output is detected early.
            // Write zero padding to reach the aligned section position
            int paddingNeeded = wcSections[i].PointerToRawData - (int)outStream.Position;
            if (paddingNeeded > 0)
            {
                outStream.Write(new byte[paddingNeeded], 0, paddingNeeded);
            }
            var buffer = new byte[peSections[i].SizeOfRawData];

src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilConverter.cs:342

  • TranslateFileOffset indexes wcSections[i] based on the PE section loop without verifying that wcSections has the same length/order as peSections. If they ever diverge (corrupt input, future format change), this will throw IndexOutOfRangeException or produce incorrect offsets. Add an upfront length check and a clearer exception (or translate by matching on VirtualAddress/PointerToRawData rather than relying on identical indexing).
    private static int TranslateFileOffset(ImmutableArray<SectionHeader> peSections, ImmutableArray<WebcilSectionHeader> wcSections, int peFileOffset)
    {
        for (int i = 0; i < peSections.Length; i++)
        {
            var peSection = peSections[i];
            if (peFileOffset >= peSection.PointerToRawData && peFileOffset < peSection.PointerToRawData + peSection.SizeOfRawData)
            {
                int offsetInSection = peFileOffset - peSection.PointerToRawData;
                return wcSections[i].PointerToRawData + offsetInSection;
            }

src/tasks/Microsoft.NET.WebAssembly.Webcil/WebcilWasmWrapper.cs:189

  • This uses throw new Exception(...) for a deterministic internal consistency check. Elsewhere in this file similar failures use InvalidOperationException; using the base Exception makes the error harder to catch/diagnose consistently. Consider throwing InvalidOperationException (or InvalidDataException) here as well.
        if (writer.BaseStream.Position % WebcilPayloadInternalAlignment != 0) {
            throw new Exception ($"Expected offset {payloadOffset}, actual position {writer.BaseStream.Position}");
        }

@pavelsavara
Copy link
Member Author

/ba-g unrelated apple infra

@pavelsavara pavelsavara merged commit 7b67779 into dotnet:main Feb 27, 2026
82 of 86 checks passed
@pavelsavara pavelsavara deleted the webcil_alignment branch February 27, 2026 09:00
lewing added a commit to lewing/runtime that referenced this pull request Mar 2, 2026
PR dotnet#124905 renamed IsWebCilEnabled to IsWebcilEnabled on main.
The merge auto-resolved the parameter name but the error message
string interpolation still referenced the old casing. Also improved
the error message to include asset details and used 'is null' pattern.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-Host os-browser Browser variant of arch-wasm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants