Make all entries in WASM tables SymbolDefinitions

[jtschuster]

In the WasmObjectWriter, instead of tracking symbols in different sections using adhoc fields, extract the tracking of symbols and their corresponding indices in physical sections or logical wasm index spaces into a separate class, WasmSymbolManager.

This also allows Wasm index relocations to share a case in ResolveRelocations.

Adds additional testing to ensure relocations are resolved such that the module's imports are first in each logical index space. This ordering is why the SymbolManager "freezes" the imports sections once index space has assigned indices.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
12 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

This PR updates the WASM object writer to assign and resolve WASM structural indices (function/type/table) via a shared symbol-index table, and extends SuperPMI recording support for getWasmWellKnownGlobals alongside a JIT/EE interface GUID bump.

Changes:

• Replace ad-hoc _uniqueSymbols/_uniqueSignatures bookkeeping with per-section symbol index assignment and use it to self-resolve WASM index relocations.
• Update well-known WASM global symbol names for READYTORUN and wire them into default import creation.
• Add SuperPMI record support for GetWasmWellKnownGlobals and update JITEEVersionIdentifier.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h	Adds a recGetWasmWellKnownGlobals declaration (currently duplicated in the header).
src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs	Centralizes symbol index assignment and uses it for WASM index relocations; updates exports/elements counts to use derived method count.
src/coreclr/tools/Common/Compiler/ObjectWriter/ObjectWriter.cs	Makes EmitSymbolDefinition virtual to allow format-specific overrides.
src/coreclr/tools/Common/Compiler/DependencyAnalysis/WasmWellKnownGlobalSymbolNode.cs	Switches well-known global import names based on READYTORUN.
src/coreclr/inc/jiteeversionguid.h	Updates the JIT/EE interface version GUID.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs:931

• Throwing NotImplementedException for a missing WASM symbol registration is misleading here: this is an internal invariant violation (the writer produced a relocation referencing an unregistered symbol), not an unimplemented feature. Using InvalidOperationException (or InvalidDataException if you consider this a malformed image) will make failures easier to triage and avoids implying feature gaps.

                            if (!_wasmSymbolManager.TryGetSymbol(reloc.SymbolName, out WasmSymbol symbol))
                            {
                                throw new NotImplementedException($"No wasm index registered for symbol '{reloc.SymbolName}' (relocation {reloc.Type}).");
                            }

[Copilot]

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

src/coreclr/tools/Common/Compiler/ObjectWriter/WasmObjectWriter.cs:1105

• WriteExports orders by symbol.Name.ToString() and then calls symbol.Name.ToString() again when writing the export. Utf8String.ToString() allocates, so this does extra allocations (and uses culture-dependent string ordering). You can order by Utf8String directly and materialize the string once per symbol.

            // TODO-WASM: Handle exports better (e.g., only export public methods, etc.)
            IEnumerable<WasmSymbol> functionSymbols = _wasmSymbolManager.GetDefinitions(WasmIndexSpace.Function);
            foreach (WasmSymbol symbol in functionSymbols.OrderBy(symbol => symbol.Name.ToString()))
            {
                WriteFunctionExport(symbol.Name.ToString(), symbol.Index);
            }

[Copilot]

Pull request overview

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

[dotnet-policy-service]

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

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[MichalStrehovsky]

All of these exceptions and messages are unnecessary; this is not a public API. We let the compiler crash with whatever the Dictionary throws for duplicate keys. Extra ContainsKey checks are just more code to maintain and strings to audit for missed localization. Also, small perf hit.

ObjectWriter has many pre-existing checks like this because the original source of it was a public API but we don't need to maintain that and in fact at some point we should do a pass deleting these validations or replacing them with asserts if needed (this one doesn't need an assert; we're going to throw).

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (2)

src/coreclr/tools/Common/Compiler/ObjectWriter/WasmSymbolManager.cs:114

• GetDefinitions currently yields both imports and definitions for the requested index space (it doesn’t filter out Entry.IsImport). Given the method name and the presence of GetImportCount/GetDefinitionCount, this is easy to misread and could lead to subtle bugs if a future caller expects only definitions (e.g., Global/Tag spaces always have imports first). Consider filtering out imports here (or renaming the API / adding a separate GetAllSymbols).

            foreach (var entry in _entries.Values)
            {
                if (entry.IndexSpace == indexSpace)
                {
                    yield return Resolve(entry);

src/coreclr/tools/aot/ILCompiler.ReadyToRun.Tests/TestCasesRunner/WasmR2RAssert.cs:219

• CheckFunctionExports assumes every function-section entry is exported (it requires function export count == definedFunctionCount and then enforces contiguous indices). That makes this test brittle against the existing TODO in WasmObjectWriter about changing export policy (e.g., exporting only public methods) even if index-space ordering remains correct. Suggest validating only that exported function indices are in the defined-function range (>= importedFunctionCount and < importedFunctionCount + definedFunctionCount) and are unique.

        if (functionIndices.Count != definedFunctionCount)
        {
            failures.Add(
                $"Found {functionIndices.Count} function exports for {definedFunctionCount} " +
                "function-section entries.");