Support serialized type names in R2R type maps - #133038
Conversation
Preserve serialized type names from custom attribute blobs and encode entries that cannot use R2R fixups as lazily resolved name pairs. Bump the R2R format version to 28 and update the VM reader and tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 4 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. |
There was a problem hiding this comment.
🟡 Changes recommended
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends ReadyToRun (R2R) ExternalTypeMaps and ProxyTypeMaps encoding to preserve serialized type names (from custom attribute blobs) when fixup-based encoding isn’t possible, by appending a lazily-resolved “named entries” sequence after each per-group native hashtable. It bumps the R2R major format version to 28 and updates the VM reader plus interop tests to validate the new behavior.
Changes:
- Extend crossgen2/ILCompiler type map emission to optionally append a native sequence of serialized name pairs for non-fixup-encodable entries.
- Update CoreCLR VM type map reader/QCall plumbing to surface the appended named entries to
TypeMapLazyDictionarycallbacks. - Bump R2R major version to 28 and update documentation/tests for the new encoding.
File summaries
| File | Description |
|---|---|
| src/tests/Interop/TypeMap/TypeMapBlobOnlyLib.il | Adds attributes that exercise duplicate full-name scenarios requiring lazy name-based resolution. |
| src/tests/Interop/TypeMap/TypeMapApp.cs | Extends validation to cover caching behavior and duplicate-name proxy mappings. |
| src/coreclr/vm/readytoruninfo.h | Adds new “TryGetPrecached*TypeMap” helpers that also return appended named-entry parsers. |
| src/coreclr/vm/readytoruninfo.cpp | Refactors type-map lookup and adds support for “named entries after hashtable” layout. |
| src/coreclr/vm/nativeformatreader.h | Adds NativeHashtable::GetParserAfterTable() to locate data appended after a hashtable. |
| src/coreclr/vm/assemblynative.cpp | Extends QCall processing to forward appended named entries to the existing callbacks. |
| src/coreclr/tools/Common/Internal/Runtime/ModuleHeaders.cs | Updates managed tool constants to R2R major version 28. |
| src/coreclr/tools/Common/Compiler/TypeMapMetadata.cs | Tracks serialized type names alongside type map entries extracted from attributes. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/TypeSystem/Mutable/MutableModule.cs | Adds helper to determine whether a reference to a type can be created across modules. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/ILCompiler.ReadyToRun.csproj | Includes new encoding helper source file. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunTypeMapEncoding.cs | Introduces shared constants/state and “encodable type” logic for type map emission. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunProxyTypeMapNode.cs | Emits fixup-based entries when possible, otherwise appends serialized name pairs. |
| src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunExternalTypeMapNode.cs | Emits fixup-based entries when possible, otherwise appends serialized (key, name) pairs. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ProxyTypeMapNode.cs | Adjusts dependency tracking to account for the richer proxy map entry type. |
| src/coreclr/tools/aot/ILCompiler.Compiler/Compiler/DependencyAnalysis/ExternalTypeMapNode.cs | Adjusts dependency tracking to account for the richer external map entry type. |
| src/coreclr/tools/aot/crossgen2.slnx | Minor solution configuration ordering adjustment. |
| src/coreclr/nativeaot/Runtime/inc/ModuleHeaders.h | Updates NativeAOT R2R header version constant to 28. |
| src/coreclr/inc/readytorun.h | Updates READYTORUN_MAJOR_VERSION and documents the v28 type map extension. |
| docs/design/coreclr/botr/readytorun-format.md | Documents the extended ExternalTypeMaps/ProxyTypeMaps layout with named entries. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Lite
|
Tagging subscribers to this area: @dotnet/interop-contrib |
There was a problem hiding this comment.
🔵 Needs a closer look
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
src/coreclr/vm/assemblynative.cpp:1796
- The result of TryGetPrecachedExternalTypeMap is only checked with _ASSERTE(found). In release builds this would silently continue on malformed images, potentially treating the map as precached while having no valid table/parser. Prefer throwing BadImageFormat when the table cannot be retrieved after HasPrecachedExternalTypeMap returned true.
This issue also appears on line 1813 of the same file.
src/coreclr/tools/aot/crossgen2.slnx:8
- This reorders the entries so x86 is listed first. Other .slnx files in this repo (e.g., src/coreclr/tools/aot/ilc.slnx) keep platforms in a stable order (Any CPU, x64, x86). Keeping the order consistent reduces churn and avoids changing any implicit/default platform selection behavior in tooling.
src/coreclr/vm/assemblynative.cpp:1817
- The result of TryGetPrecachedProxyTypeMap is only checked with _ASSERTE(found). In release builds this would silently continue on malformed images, potentially treating the map as precached while having no valid table/parser. Prefer throwing BadImageFormat when the table cannot be retrieved after HasPrecachedProxyTypeMap returned true.
NativeFormat::NativeHashtable typeMap;
NativeFormat::NativeParser namedEntries;
bool found = pR2RInfo->TryGetPrecachedProxyTypeMap(groupTypeMT, &typeMap, &namedEntries);
_ASSERTE(found);
if (!ProcessPrecachedTypeMapNamedEntries(namedEntries, newProxyTypeEntry, context))
- Files reviewed: 19/19 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
Can we either delay the re-serialization of the type name until it's definitely necessary (and reconstruct from the TypeDesc) or read the raw string out from the custom attribute blob directly instead of deserializing it just to reserialize it with our own logic? |
We do the latter in Or are you talking about something else? |
Preserve serialized type names from custom attribute blobs and encode entries that cannot use R2R fixups as lazily resolved name pairs. They are laid out in a 'native sequence' after the fixups hashtable.
The runtime treats these as copies of the attributes and uses the callsbacks provided by the TypeMapLazyDictionary to store them in the
_lazyDatato be resolved if they aren't found in the fixup hashtable.Bump the R2R format version to 28 and update the VM reader and tests.
Fixes #132811 in main