[browser][coreCLR] Loading WebCIL 0.1#124268
[browser][coreCLR] Loading WebCIL 0.1#124268pavelsavara wants to merge 4 commits intodotnet:mainfrom
Conversation
|
Tagging subscribers to this area: @agocke, @jeffschwMSFT, @elinor-fung |
There was a problem hiding this comment.
Pull request overview
This PR adds support for loading WebCIL version 1.0 assemblies in CoreCLR on browser/WASM targets. WebCIL is an alternative container format for ECMA-335 assemblies that strips PE headers and packages assemblies as WebAssembly modules with a .wasm extension, helping avoid issues with firewalls/antivirus software that block .dll files.
Changes:
- Added
WebCILImageLayoutC++ class that extendsPEImageLayoutto handle WebCIL format, parsing WebCIL headers and mapping RVAs to file offsets - Implemented
instantiateWebCILModulein TypeScript that instantiates WebCIL .wasm modules and extracts assembly payloads into aligned memory - Enabled WebCIL support for CoreCLR by removing
WasmEnableWebcil=falserestrictions and updating test configurations
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/Common/JavaScript/types/public-api.ts | Added "webcil1" asset behavior type |
| src/native/libs/Common/JavaScript/types/exchange.ts | Added instantiateWebCILModule to BrowserHost exports |
| src/native/libs/Common/JavaScript/cross-module/index.ts | Updated export table to include instantiateWebCILModule |
| src/native/corehost/browserhost/loader/dotnet.d.ts | Added "webcil1" type definition |
| src/native/corehost/browserhost/loader/assets.ts | Added WebCIL detection and loading logic, mapping .wasm to .dll paths |
| src/native/corehost/browserhost/host/index.ts | Exported instantiateWebCILModule function |
| src/native/corehost/browserhost/host/host.ts | Updated assembly path mapping to replace .wasm with .dll |
| src/native/corehost/browserhost/host/assets.ts | Implemented instantiateWebCILModule to extract WebCIL payload and register assemblies |
| src/mono/wasm/Wasm.Build.Tests/Common/BuildEnvironment.cs | Removed CoreCLR restriction for WebCIL in tests |
| src/mono/browser/build/WasmApp.InTree.props | Removed WasmEnableWebcil=false for CoreCLR, updated TODO comment |
| src/coreclr/vm/webcildecoder.cpp | New file implementing WebCILImageLayout class for parsing and validating WebCIL format |
| src/coreclr/vm/peimagelayout.h | Added WebCIL header structures and WebCILImageLayout class declaration |
| src/coreclr/vm/peimagelayout.cpp | Added WebCIL format detection in LoadFlat |
| src/coreclr/vm/peassembly.cpp | Excluded 32-bit NT headers check for TARGET_BROWSER, added braces for consistency |
| src/coreclr/vm/coreassemblyspec.cpp | Added braces around single-statement if bodies |
| src/coreclr/vm/CMakeLists.txt | Added webcildecoder.cpp to browser build |
| src/coreclr/inc/pedecoder.h | Made specific PEDecoder methods virtual for browser target to enable WebCILImageLayout overrides |
| eng/testing/tests.browser.targets | Removed WasmEnableWebcil=false for CoreCLR, updated TODO comment |
|
Please have @elinor-fung look at this |
5adef6d to
102345a
Compare
|
The solution in this PR now works and the change looks better than the previous attempt with inheritance from The open question is DAC support, which I don't fully understand yet. |
|
/cc @dotnet/dotnet-diag |
| // in HasWebcilHeaders(), so RvaToSection()/RvaToOffset() work uniformly. | ||
| static constexpr uint16_t WEBCIL_MAX_SECTIONS = 8; | ||
| uint16_t m_webcilSectionCount = 0; | ||
| IMAGE_SECTION_HEADER m_webcilSectionHeaders[WEBCIL_MAX_SECTIONS]; |
There was a problem hiding this comment.
This is 320 bytes. It is not prohibitive, but it is not free either.
Would it be possible to say that the WebCIL has exactly one section to make things simpler? (Shrink all sections into one during the conversion to WebCIL.)
| mutableThis->m_webcilSectionCount = nSections; | ||
| for (uint16_t i = 0; i < nSections; i++) | ||
| { | ||
| memset(&mutableThis->m_webcilSectionHeaders[i], 0, sizeof(IMAGE_SECTION_HEADER)); |
There was a problem hiding this comment.
Instead of initializing each section one at a time, we should just do the lot of them prior to the loop.
I think the WASM world is single threaded right now so there is no race, but this has an inherent race condition with the initialization happening and the flag being set below. Basically, the first thread comes through and sets the flag and another comes in and reinitializes all of the sections while the first thread has already returned. I don't think we need to change anything here, just an observation.
| previousOffsetEnd = (UINT32)endInFile.Value(); | ||
| } | ||
|
|
||
| PEDecoder* mutableThis = const_cast<PEDecoder *>(this); |
There was a problem hiding this comment.
Remove the const from the function. const_cast is rarely needed.
| <NestedBuildProperty Include="WasmTestForwardConsole" /> | ||
| </ItemGroup> | ||
|
|
||
| <Target Name="CleanSampleBinObj" BeforeTargets="BuildSampleInTree"> |
There was a problem hiding this comment.
We should be using the MSBuild Clean item group instead of creating one off clean targets.
| if (HasWebcilHeaders()) | ||
| { | ||
| // WebCIL images are always IL-only by definition. | ||
| const_cast<PEDecoder *>(this)->m_flags |= FLAG_IL_ONLY_CHECKED; |
There was a problem hiding this comment.
Change the const if this is needed please.
| RETURN (TADDR)NULL; | ||
| } | ||
|
|
||
| // RvaToSection guarantees rva >= sectionBase, so the subtraction |
There was a problem hiding this comment.
An assert here is appropriate.
Enable WebCIL support for CoreCLR on Browser/WASM
Fixes #120248
WebCIL format v0.1 with 16-byte section alignment
Bumps WebCIL version from 0.0 to 0.1. The
WebcilConverternow emits section data at 16-byte-aligned offsets, ensuring RVA static fields backingReadOnlySpan<T>over types up toVector128<T>retain their natural alignment.CoreCLR PEDecoder: native WebCIL parsing
New
src/coreclr/inc/webcil.hdefinesWebcilHeaderandWebcilSectionHeaderC structs withstatic_assertsize checks.PEDecodergains:FLAG_WEBCILflag;HasWebcilHeaders()validates magic, version, section count/bounds/ordering and synthesizesIMAGE_SECTION_HEADER[]from WebCIL sections soRvaToSection()/RvaToOffset()work uniformly.HasHeaders()/CheckHeaders()as unified entry points that dispatch to WebCIL or NT paths.HasDirectoryEntry(),GetDirectoryEntryData()(handlesCOMHEADERandDEBUGentries viaWebcilHeaderfields with explicit range validation against section data).CheckCorHeader()andCheckILOnly()WebCIL branches (WebCIL images are always IL-only by definition).GetPEKindAndMachine()reportspeILonly/IMAGE_FILE_MACHINE_I386for WebCIL.GetNumberOfRvaAndSizes()moved from public to private API;GetNumberOfSections()/FindFirstSection()delegate to synthesized headers for WebCIL.CheckNTHeaders()toCheckHeaders()/HasHeaders().PEImage / PEAssembly integration
PEImageexposesHasHeaders()forwarding toPEDecoder::HasHeaders().PEAssemblyusesHasHeaders()instead ofHasNTHeaders()for metadata access.Browser host loader: WebCIL module instantiation
instantiateWebCILModule()in host assets:.wasmwrapper as a WebAssembly modulegetWebcilSize/getWebcilPayloadexports to extract the raw WebCIL.dllvirtual path.getWasmMemory/getWasmTableWasmEnableWebcil.DAC support gap
All WebCIL code paths in
PEDecoderare gated behind#ifdef TARGET_BROWSER, which is mutually exclusive withDACCESS_COMPILE.Should we drop
#ifdef TARGET_BROWSER? Or build target specific DAC binaries ?Follow up in #124467