Merge branch 'main' into wasm-fix-methodimpl-interpreter-crash

Author: radekdoulik

.github/skills/code-review/SKILL.md

@@ -19,7 +19,11 @@ Use this skill when:
 
 ## Review Process
 
-### Step 0: Gather Code Context (No PR Narrative Yet)
+### Step 0: Load Relevant Instructions
+
+Before analyzing anything, load any and all instructions under `.github/instructions` that are relevant to the code changes, as indicated by the frontmatter. If conflict arises between said custom instructions and the instructions in this skill, the custom instructions supersede instructions in this skill.
+
+### Step 1: Gather Code Context (No PR Narrative Yet)
 
 Before analyzing anything, collect as much relevant **code** context as you can. **Critically, do NOT read the PR description, linked issues, or existing review comments yet.** You must form your own independent assessment of what the code does, why it might be needed, what problems it has, and whether the approach is sound — before being exposed to the author's framing. Reading the author's narrative first anchors your judgment and makes you less likely to find real problems.
 
@@ -32,14 +36,14 @@ Before analyzing anything, collect as much relevant **code** context as you can.
 7. **Detect new public API surface**: Check whether the PR introduces new public API surface. Look for:
    - Changes to `ref/` assembly source files (the strongest signal — these define the public API contract)
    - New `public` members (methods, properties, types, enum values) in `src/` files
-   - Note whether new public API was detected. If it was, you **MUST** load and execute the API approval verification procedure during Step 3. Read the file `.github/skills/code-review/api-approval-check.md` (relative to the repository root) and follow its instructions. Do not skip this step — it is blocking.
+   - Note whether new public API was detected. If it was, you **MUST** load and execute the API approval verification procedure during Step 4. Read the file `.github/skills/code-review/api-approval-check.md` (relative to the repository root) and follow its instructions. Do not skip this step — it is blocking.
 
-### Step 1: Discover Area-Specific Agents
+### Step 2: Discover Area-Specific Agents
 - Study **review** agents available in  `.github/agents` folder that are capable of reviewing specific areas of the codebase. Their yaml frontmatter description tells when they apply.
 - When performing the review, invoke sub-agents to perform those area-specific reviews as subtasks during all subsequent steps, integrating those results.
 - Depending on the PR, more subagents might be launched. Launch them in parallel. Always continue regular review described here as well - the subagents are addons, not replacements.
 
-### Step 2: Form an Independent Assessment
+### Step 3: Form an Independent Assessment
 
 Based **only** on the code context gathered above (without the PR description or issue), answer these questions:
 
@@ -50,7 +54,7 @@ Based **only** on the code context gathered above (without the PR description or
 
 Write down your independent assessment before proceeding. You must produce a holistic assessment (see [Holistic PR Assessment](#holistic-pr-assessment)) at this stage.
 
-### Step 3: Incorporate PR Narrative and Reconcile
+### Step 4: Incorporate PR Narrative and Reconcile
 
 Now read the PR description, labels, linked issues (in full), author information, existing review comments, and any related open issues in the same area. Treat all of this as **claims to verify**, not facts to accept.
 
@@ -59,9 +63,9 @@ Now read the PR description, labels, linked issues (in full), author information
 3. **Existing review comments**: Check if there are already review comments on the PR to avoid duplicating feedback.
 4. **Reconcile your assessment with the author's claims.** Where your independent reading of the code disagrees with the PR description or issue, investigate further — but do not simply defer to the author's framing. If the PR claims a bug fix, a performance improvement, or a behavioral correction, verify those claims against the code and any provided evidence. If your independent assessment found problems the PR narrative doesn't acknowledge, those problems are more likely to be real, not less.
 5. **Update your holistic assessment** if the additional context reveals information that genuinely changes your evaluation (e.g., a linked issue proves the bug is real, or an existing review comment already identified the same concern). But do not soften findings just because the PR description sounds reasonable.
-6. **API Approval Verification.** If Step 0 detected new public API surface, you **MUST** now load the file `.github/skills/code-review/api-approval-check.md` (relative to the repository root) and execute the full procedure described there. Use the `view` tool, `cat`, or equivalent to read the file contents into your context, then follow every step. This is a **blocking** gate — if any check in that procedure fails, the review verdict must be ❌ Reject or ❌ Needs Changes regardless of other findings. Do not proceed without completing this step when new public API is detected. **If the file cannot be loaded for any reason**, report ❌ error — "Unable to load API approval verification procedure; cannot verify new public API surface" — and set the verdict to ❌ Needs Changes.
+6. **API Approval Verification.** If Step 1 detected new public API surface, you **MUST** now load the file `.github/skills/code-review/api-approval-check.md` (relative to the repository root) and execute the full procedure described there. Use the `view` tool, `cat`, or equivalent to read the file contents into your context, then follow every step. This is a **blocking** gate — if any check in that procedure fails, the review verdict must be ❌ Reject or ❌ Needs Changes regardless of other findings. Do not proceed without completing this step when new public API is detected. **If the file cannot be loaded for any reason**, report ❌ error — "Unable to load API approval verification procedure; cannot verify new public API surface" — and set the verdict to ❌ Needs Changes.
 
-### Step 4: Detailed Analysis
+### Step 5: Detailed Analysis
 
 1. **Focus on what matters.** Prioritize bugs, performance regressions, safety issues, race conditions, resource management problems, incorrect assumptions about data or state, and API design problems. Do not comment on trivial style issues unless they violate an explicit rule below.
 2. **Consider collateral damage.** For every changed code path, actively brainstorm: what other scenarios, callers, or inputs flow through this code? Could any of them break or behave differently after this change? If you identify any plausible risk — even one you can't fully confirm — surface it so the author can evaluate. Do not dismiss behavioral changes because you believe the fix justifies them. The tradeoff is the author's decision — your job is to make it visible.

docs/coding-guidelines/interop-guidelines.md

@@ -167,6 +167,10 @@ If implicit framework references are disabled (as is the case for most libraries
   - `System.Memory`
   - `System.Runtime.CompilerServices.Unsafe`
 
+### Search paths
+
+System.Private.CoreLib and libraries assemblies all set `DefaultDllImportSearchPaths` to `DllImportSearchPath.Assembly | DllImportSearchPath.System32` at the module level. This first looks in the assembly directory (application directory for single-file), then system directory on Windows or default search on non-Windows. For Windows P/Invokes that should only ever load from the system directory, this can be narrowed with `[DefaultDllImportSearchPaths(DllImportSearchPath.System32)]` on the P/Invoke.
+
 ## UNIX shims
 
 Often, various UNIX flavors offer the same API from the point-of-view of compatibility with C/C++ source code, but they do not have the same ABI. e.g. Fields can be laid out differently, constants can have different numeric values, exports can be named differently, etc. There are not only differences between operating systems (Mac OS X vs. Ubuntu vs. FreeBSD), but also differences related to the underlying processor architecture (x64 vs. x86 vs. ARM).

docs/design/datacontracts/CallingConvention.md

@@ -0,0 +1,41 @@
+# Contract CallingConvention
+
+This contract walks a method's argument signature using the runtime's
+calling-convention rules so consumers can locate each argument on the
+caller's transition frame and reason about which slots hold GC references.
+
+The actual ABI (which registers hold which arguments, what alignment and
+padding rules apply, how structs are promoted to registers vs spilled, how
+varargs are passed, etc.) is documented in the CLR ABI specs and is not
+re-described here:
+
+- [Common CLR ABI conventions](../coreclr/botr/clr-abi.md)
+
+This contract's responsibility is to surface the *result* of that walk in
+a form the cDAC can use, byte-for-byte compatible with what the runtime
+itself produces.
+
+## APIs of contract
+
+``` csharp
+// Encode the argument GCRefMap blob for `methodDesc` byte-for-byte
+// compatible with the runtime's ComputeCallRefMap (frames.cpp).
+// Returns false when this contract declines to encode the method
+// (e.g. an unported ABI path); callers should map false to E_NOTIMPL.
+// When false, the value of `blob` is unspecified.
+bool TryComputeArgGCRefMapBlob(MethodDescHandle methodDesc, out byte[] blob);
+```
+
+## Version 1
+
+The single API is implemented by walking the shared `ArgIterator`
+(`src/coreclr/tools/Common/CallingConvention/ArgIterator.cs`) and feeding
+the per-argument result into a GCRefMap encoder that mirrors
+`GCRefMapBuilder` (`src/coreclr/inc/gcrefmap.h`).
+
+`TryComputeArgGCRefMapBlob` returns `false` for any method whose
+signature, ABI path, or generic context the encoder hasn't been taught
+yet. The cdacstress harness (`src/coreclr/vm/cdacstress.cpp`,
+`ARGITER` sub-check) uses byte-for-byte comparison of the returned blob
+against the runtime's `ComputeCallRefMap` output as its correctness
+oracle.

docs/design/datacontracts/Object.md

@@ -221,6 +221,7 @@ DelegateInfo GetDelegateInfo(TargetPointer address)
     Data.Delegate del = new Data.Delegate(target, address);
 
     // Classify the delegate from its invocation count and auxiliary pointer.
+    // This does not handle open virtual delegates correctly.
     DelegateType delegateType = target.ReadNInt(address + /* Delegate::InvocationCount offset */) switch
     {
         0  => del.MethodPtrAux == TargetCodePointer.Null

docs/design/datacontracts/RuntimeTypeSystem.md

@@ -73,6 +73,8 @@ partial interface IRuntimeTypeSystem : IContract
     public virtual TargetPointer GetWellKnownMethodTable(WellKnownMethodTable kind);
     // True if the MethodTable represents a type that contains managed references
     public virtual bool ContainsGCPointers(TypeHandle typeHandle);
+    // True if the MethodTable represents a byref-like value type (Span<T>, ReadOnlySpan<T>, any ref struct).
+    public virtual bool IsByRefLike(TypeHandle typeHandle);
     // True if the type requires 8-byte alignment on platforms that don't 8-byte align by default (FEATURE_64BIT_ALIGNMENT)
     public virtual bool RequiresAlign8(TypeHandle typeHandle);
     // True if the MethodTable represents a continuation type used by the async continuation feature
@@ -290,6 +292,10 @@ partial interface IRuntimeTypeSystem : IContract
     // Return true if the method is a wrapper stub (unboxing or instantiating).
     public virtual bool IsWrapperStub(MethodDescHandle methodDesc);
 
+    // Return true if the method is an unboxing stub (a wrapper around a
+    // value-type instance method that unboxes `this` before forwarding).
+    public virtual bool IsUnboxingStub(MethodDescHandle methodDesc);
+
 }
 ```
 
@@ -302,6 +308,7 @@ bool IsFieldDescStatic(TargetPointer fieldDescPointer);
 bool IsFieldDescRVA(TargetPointer fieldDescPointer);
 uint GetFieldDescType(TargetPointer fieldDescPointer);
 uint GetFieldDescOffset(TargetPointer fieldDescPointer, FieldDefinition? fieldDef);
+TypeHandle GetFieldDescApproxTypeHandle(TargetPointer fieldDescPointer);
 TargetPointer GetFieldDescStaticAddress(TargetPointer fieldDescPointer, bool unboxValueTypes = true);
 TargetPointer GetFieldDescThreadStaticAddress(TargetPointer fieldDescPointer, TargetPointer thread, bool unboxValueTypes = true);
 ```
@@ -330,6 +337,8 @@ internal partial struct RuntimeTypeSystem_1
         GenericsMask_SharedInst = 0x00000020,            // shared instantiation, e.g. List<__Canon> or List<MyValueType<__Canon>>
         GenericsMask_TypicalInstantiation = 0x00000030,  // the type instantiated at its formal parameters, e.g. List<T>
 
+        IsByRefLike = 0x00001000,                        // value type that may contain managed pointers (e.g. Span<T>, ReadOnlySpan<T>)
+
         StringArrayValues = GenericsMask_NonGeneric,
     }
 
@@ -404,6 +413,7 @@ internal partial struct RuntimeTypeSystem_1
         public bool IsTrackedReferenceWithFinalizer => GetFlag(WFLAGS_HIGH.IsTrackedReferenceWithFinalizer) != 0;
         public bool IsGenericTypeDefinition => TestFlagWithMask(WFLAGS_LOW.GenericsMask, WFLAGS_LOW.GenericsMask_TypicalInstantiation);
         public bool IsSharedByGenericInstantiations => TestFlagWithMask(WFLAGS_LOW.GenericsMask, WFLAGS_LOW.GenericsMask_SharedInst);
+        public bool IsByRefLike => TestFlagWithMask(WFLAGS_LOW.IsByRefLike, WFLAGS_LOW.IsByRefLike);
     }
 
     [Flags]
@@ -668,6 +678,8 @@ Contracts used:
 
     public bool ContainsGCPointers(TypeHandle TypeHandle) => !typeHandle.IsMethodTable() ? false : _methodTables[TypeHandle.Address].Flags.ContainsGCPointers;
 
+    public bool IsByRefLike(TypeHandle typeHandle) => typeHandle.IsMethodTable() && _methodTables[typeHandle.Address].Flags.IsByRefLike;
+
     public bool RequiresAlign8(TypeHandle typeHandle) => !typeHandle.IsMethodTable() ? false : _methodTables[typeHandle.Address].Flags.RequiresAlign8;
 
     public bool IsCanonicalMethodTable(TypeHandle typeHandle)
@@ -1872,6 +1884,17 @@ Determining if a method is a wrapper stub (unboxing or instantiating):
     }
 ```
 
+Determining if a method is an unboxing stub. An unboxing stub is a wrapper
+around a value-type instance method whose `this` is a boxed object: the
+stub unboxes `this` and forwards to the real instance method. The bit is
+stored in `MethodDescFlags3` and surfaces as the `IsUnboxingStub` flag on
+`MethodDesc`:
+
+```csharp
+    public bool IsUnboxingStub(MethodDescHandle methodDescHandle)
+        => _methodDescs[methodDescHandle.Address].IsUnboxingStub;
+```
+
 Extracting a pointer to the `MethodDescVersioningState` data for a given method
 
 ```csharp
@@ -2232,6 +2255,15 @@ TargetPointer GetFieldDescThreadStaticAddress(TargetPointer fieldDescPointer, Ta
     // Uses GetGCThreadStaticsBasePointer / GetNonGCThreadStaticsBasePointer.
     // The unboxValueTypes parameter behaves the same as in GetFieldDescStaticAddress.
 }
+
+TypeHandle GetFieldDescApproxTypeHandle(TargetPointer fieldDescPointer)
+{
+    // Resolve enclosing MT -> Module -> MetadataReader, decode the field's
+    // signature using the SignatureDecoder contract with a SignatureTypeProvider
+    // bound to the enclosing class as generic context, and return the resulting
+    // TypeHandle. Returns TypeHandle.Null if any link in the chain is unavailable
+    // (e.g. uncached constructed instantiation).
+}
 ```
 
 ### Other APIs

docs/design/datacontracts/StackWalk.md

@@ -136,6 +136,7 @@ This contract depends on the following descriptors:
 | `ReadyToRunInfo` | `ImportSections` | Pointer to array of `READYTORUN_IMPORT_SECTION` structs for GCRefMap resolution |
 | `ReadyToRunInfo` | `NumImportSections` | Count of import sections in the array |
 | `FuncEvalFrame` | `DebuggerEvalPtr` | Pointer to the Frame's DebuggerEval object |
+| `FuncEvalFrame` | `ReturnAddress` | Return address of the frame |
 | `DebuggerEval` | `TargetContext` | Context saved inside DebuggerEval |
 | `DebuggerEval` | `EvalUsesHijack` | Flag used in processing FuncEvalFrame |
 | `DebuggerEval` | `MethodToken` | Metadata token of the method being evaluated |

docs/workflow/building/coreclr/README.md

@@ -50,7 +50,7 @@ As described in the [workflow README](/docs/workflow/README.md#building-the-repo
 Once you have both subsets built, you can generate the *Core_Root*, which as mentioned above, is the most flexible way of testing your changes. You can generate the *Core_Root* by running the following command, assuming a *Checked* clr build on an x64 machine:
 
 ```bash
-./src/tests/build.sh -x64 -checked -generatelayoutonly
+./src/tests/build.sh -arch x64 -checked -generatelayoutonly
 ```
 
 Since this is more related to testing, you can find the full details and instructions in the CoreCLR testing doc [over here](/docs/workflow/testing/coreclr/testing.md).

docs/workflow/building/coreclr/cross-building.md

@@ -129,15 +129,15 @@ The Crossgen2 JIT tools are used to run Crossgen2 on libraries built during the
 However, you might find yourself needing to (re)build them because either you made changes to them, or you built CoreCLR in a different way using `build-runtime.sh` instead of the usual default script at the root of the repo. To build these tools, you need to run the `src/coreclr/build-runtime.sh` script, and pass the `-hostarch` flag with the architecture of the host machine, alongside the `-component crosscomponents` flag to specify that you only want to build the cross-targeting tools. Retaking our previous example of building for ARM64 using an x64 Linux machine:
 
 ```bash
-./src/coreclr/build-runtime.sh -arm64 -hostarch x64 -component crosscomponents -cmakeargs "-DCLR_CROSS_COMPONENTS_BUILD=1"
+./src/coreclr/build-runtime.sh -arch arm64 -hostarch x64 -component crosscomponents -cmakeargs "-DCLR_CROSS_COMPONENTS_BUILD=1"
 ```
 
 The output of running this command is placed in `artifacts/bin/coreclr/linux.<target_arch>.<configuration>/<host_arch>`. For our example, it would be `artifacts/bin/coreclr/linux.arm64.Release/x64`.
 
 On Windows, you can build these cross-targeting diagnostic libraries with the `linuxdac` and `alpinedac` subsets from the root `build.cmd` script. That said, you can also use the `build-runtime.cmd` script, like with Linux. These builds also require you to pass the `-os` flag to specify the target OS. For example:
 
 ```cmd
-.\src\coreclr\build-runtime.cmd -arm64 -hostarch x64 -os linux -component crosscomponents -cmakeargs "-DCLR_CROSS_COMPONENTS_BUILD=1"
+.\src\coreclr\build-runtime.cmd -arch arm64 -hostarch x64 -os linux -component crosscomponents -cmakeargs "-DCLR_CROSS_COMPONENTS_BUILD=1"
 ```
 
 If you're building the cross-components in powershell, you'll need to wrap `"-DCLR_CROSS_COMPONENTS_BUILD=1"` with single quotes (`'`) to ensure things are escaped correctly for CMD.
@@ -218,7 +218,7 @@ arch=arm64
 ./build.sh clr+libs --cross --arch $arch --os $os --use-bootstrap
 
 # CoreCLR runtime tests.
-src/tests/build.sh -cross -$arch -$os -p:LibrariesConfiguration=Debug --use-bootstrap
+src/tests/build.sh --cross --arch $arch --os $os -p:LibrariesConfiguration=Debug --use-bootstrap
 
 # Libraries tests (produces zipped per-library test archives under artifacts/helix/tests/).
 ./build.sh libs.tests --cross --arch $arch --os $os --use-bootstrap -p:ArchiveTests=true

docs/workflow/building/coreclr/ios.md

@@ -107,7 +107,7 @@ open ./src/mono/sample/iOS/bin/<ios|iossimulator|tvossimulator|maccatalyst>-arm6
 To build the runtime tests for iOS with CoreCLR, run the following command from `<repo-root>`:
 
 ```bash
-./src/tests/build.sh -os <ios|iossimulator|tvossimulator|maccatalyst> arm64 <Release|Debug> -p:UseMonoRuntime=false
+./src/tests/build.sh -os <ios|iossimulator|tvossimulator|maccatalyst> -arch arm64 <Release|Debug> -p:UseMonoRuntime=false
 ```
 
 ## Debugging the runtime and the sample app

docs/workflow/testing/libraries/testing-apple.md

@@ -96,7 +96,7 @@ Currently, only the `tracing/eventpipe` subset of runtime tests is enabled on iO
 The subset of runtime tests can be built by executing the following shell script:
 ```sh
 ./build.sh -arch arm64 -os ios -s mono+libs -c Release
-./src/tests/build.sh os ios arm64 Release -mono tree tracing/eventpipe /p:LibrariesConfiguration=Release
+./src/tests/build.sh -os ios -arch arm64 Release -mono tree tracing/eventpipe /p:LibrariesConfiguration=Release
 ```
 
 The script generates an Apple bundle that can be executed using Xcode or XHarness.