Make GetRawData an intrinsic to avoid UB Unsafe.As

[MichalPetryka]

Replaces invalid Unsafe.As with a JIT intrinsic.

cc @jkotas does this make sense to you?

[azure-pipelines]

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

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[MichalPetryka]

@MihuBot

[MichalPetryka]

It seems like the JIT has trouble eliding the nullcheck in some cases for some reason...

[jkotas]

It seems like the JIT has trouble eliding the nullcheck in some cases for some reason...

It would be nice to fix these regressions.

LGTM otherwise

[EgorBo]

The null check isn't folded because optFindNullCheckToFold doesn't look through ARR_ADDR. E.g. for MemoryMarshal.GetArrayDataReference(a) we have:

NULLCHECK(V00)
IND(ARR_ADDR(ADD(V00, 16)))

Patch (also needs to clear GTF_ARR_ADDR_NONNULL on the ARR_ADDR once the null check is gone, otherwise the IND stays non-faulting):

diff --git a/src/coreclr/jit/earlyprop.cpp b/src/coreclr/jit/earlyprop.cpp
index 4a77fb0740f..84a79edd92b 100644
--- a/src/coreclr/jit/earlyprop.cpp
+++ b/src/coreclr/jit/earlyprop.cpp
@@ -337,6 +337,13 @@ bool Compiler::optFoldNullCheck(GenTree* tree, LocalNumberToNullCheckTreeMap* nu
         // The current indir is no longer non-faulting.
         tree->gtFlags &= ~GTF_IND_NONFAULTING;
 
+        GenTree* addr = tree->GetIndirOrArrMetaDataAddr()->gtEffectiveVal();
+        if (addr->OperIs(GT_ARR_ADDR))
+        {
+            // The array may be null now that the null check is gone.
+            addr->gtFlags &= ~GTF_ARR_ADDR_NONNULL;
+        }
+
         if (nullCheckParent != nullptr)
         {
             nullCheckParent->gtFlags &= ~GTF_DONT_CSE;
@@ -393,6 +400,12 @@ GenTree* Compiler::optFindNullCheckToFold(GenTree* tree, LocalNumberToNullCheckT
 
     GenTree* addr = tree->GetIndirOrArrMetaDataAddr()->gtEffectiveVal();
 
+    // ARR_ADDR is a transparent wrapper, look through it to get the actual address.
+    if (addr->OperIs(GT_ARR_ADDR))
+    {
+        addr = addr->AsArrAddr()->Addr();
+    }
+
     ssize_t offsetValue = 0;
 
     if (addr->OperIs(GT_ADD) && addr->gtGetOp2()->IsCnsIntOrI())

byte Test(byte[] a) => MemoryMarshal.GetArrayDataReference(a);:

; before
cmp      byte  ptr [rcx], cl
movzx    rax, byte  ptr [rcx+0x10]

; after
movzx    rax, byte  ptr [rcx+0x10]

SPMI (win-x64): benchmarks.run -94 bytes (39 improved, 0 regressed), libraries.pmi no diffs.

[MichalPetryka]

@MihuBot

[MichalPetryka]

@MihuBot

[MichalPetryka]

It seems like the JIT has trouble eliding the nullcheck in some cases for some reason...

It would be nice to fix these regressions.

LGTM otherwise

Seems mostly fixed now.

[jkotas]

The test failures look related. Many tests are crashing with AVs, e.g.:

  System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
     at System.Diagnostics.Tracing.EventListener.DisposeOnShutdown()

[Copilot]

Copilot review overview

🔵 Needs a closer look

The change crosses CoreLib, JIT, interpreter, VM-generated IL, and NativeAOT code paths, warranting final maintainer validation.

Review effort: Balanced
Findings: None

What changed in this PR

Replaces unsafe object-layout casting with an internal GetRawData intrinsic across CoreCLR and NativeAOT.

Changes:

• Adds JIT and interpreter expansion for GetRawData.
• Updates VM and AOT-generated IL to call the intrinsic.
• Removes the obsolete RawData helper and binder entries.

File	Description
src/​libraries/​System.Private.CoreLib/​src/​System/​Runtime/​InteropServices/​MemoryMarshal.cs	Uses the new intrinsic for array data.
src/​coreclr/​vm/​prestub.cpp	Updates unboxing stubs.
src/​coreclr/​vm/​ilmarshalers.cpp	Updates generated marshalling IL.
src/​coreclr/​vm/​corelib.h	Removes obsolete raw-data binder entries.
src/​coreclr/​vm/​array.cpp	Updates generated array-operation IL.
src/​coreclr/​tools/​Common/​TypeSystem/​Interop/​IL/​Marshaller.Aot.cs	Updates AOT pinning IL.
src/​coreclr/​tools/​Common/​TypeSystem/​IL/​Stubs/​GetFieldHelperMethodOverride.cs	Uses the intrinsic for field offsets.
src/​coreclr/​tools/​Common/​Compiler/​CompilerTypeSystemContext.BoxedTypes.cs	Updates generated unboxing thunks.
src/​coreclr/​System.Private.CoreLib/​src/​System/​Runtime/​CompilerServices/​RuntimeHelpers.CoreCLR.cs	Defines the CoreCLR intrinsic.
src/​coreclr/​nativeaot/​System.Private.CoreLib/​src/​System/​Runtime/​CompilerServices/​RuntimeHelpers.NativeAot.cs	Defines the NativeAOT intrinsic.
src/​coreclr/​jit/​namedintrinsiclist.h	Registers the intrinsic identifier.
src/​coreclr/​jit/​importercalls.cpp	Expands calls into a checked interior byref.
src/​coreclr/​jit/​fgprofile.cpp	Handles the intrinsic during instrumentation.
src/​coreclr/​interpreter/​intrinsics.cpp	Recognizes the intrinsic.
src/​coreclr/​interpreter/​compiler.cpp	Emits interpreter null-check and address arithmetic.
src/​coreclr/​inc/​corinfo.h	Defines the object-data offset.