[release/8.0] JIT: Disallow mismatched GC-ness for physical promotions

src/coreclr/jit/layout.cpp

@@ -421,6 +421,7 @@ void ClassLayout::InitializeGCPtrs(Compiler* compiler)
 //
 // Return value:
 //    true if at least one GC ByRef, false otherwise.
+//
 bool ClassLayout::HasGCByRef() const
 {
     unsigned slots = GetSlotCount();
@@ -435,6 +436,39 @@ bool ClassLayout::HasGCByRef() const
     return false;
 }
 
+//------------------------------------------------------------------------
+// IntersectsGCPtr: check if the specified interval intersects with a GC
+// pointer.
+//
+// Parameters:
+//   offset - The start offset of the interval
+//   size   - The size of the interval
+//
+// Return value:
+//   True if it does.
+//
+bool ClassLayout::IntersectsGCPtr(unsigned offset, unsigned size) const
+{
+    if (!HasGCPtr())
+    {
+        return false;
+    }
+
+    unsigned startSlot = offset / TARGET_POINTER_SIZE;
+    unsigned endSlot   = (offset + size - 1) / TARGET_POINTER_SIZE;
+    assert((startSlot < GetSlotCount()) && (endSlot < GetSlotCount()));
+
+    for (unsigned i = startSlot; i <= endSlot; i++)
+    {
+        if (IsGCPtr(i))
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 //------------------------------------------------------------------------
 // AreCompatible: check if 2 layouts are the same for copying.
 //

src/coreclr/jit/layout.h

@@ -216,6 +216,8 @@ class ClassLayout
         }
     }
 
+    bool IntersectsGCPtr(unsigned offset, unsigned size) const;
+
     static bool AreCompatible(const ClassLayout* layout1, const ClassLayout* layout2);
 
 private:

src/coreclr/jit/promotion.cpp

@@ -621,6 +621,38 @@ class LocalUses
     bool EvaluateReplacement(
         Compiler* comp, unsigned lclNum, const Access& access, unsigned inducedCount, weight_t inducedCountWtd)
     {
+        // Verify that this replacement has proper GC ness compared to the
+        // layout. While reinterpreting GC fields to integers can be considered
+        // UB, there are scenarios where it can happen safely:
+        //
+        // * The user code could have guarded the access with a dynamic check
+        //   that it doesn't contain a GC pointer, so that the access is actually
+        //   in dead code. This happens e.g. in span functions in SPC.
+        //
+        // * For byrefs, reinterpreting as an integer could be ok in a
+        //   restricted scope due to pinning.
+        //
+        // In theory we could allow these promotions in the restricted scope,
+        // but currently physical promotion works on a function-wide basis.
+
+        LclVarDsc*   lcl    = comp->lvaGetDesc(lclNum);
+        ClassLayout* layout = lcl->GetLayout();
+        if (layout->IntersectsGCPtr(access.Offset, genTypeSize(access.AccessType)))
+        {
+            if (((access.Offset % TARGET_POINTER_SIZE) != 0) ||
+                (layout->GetGCPtrType(access.Offset / TARGET_POINTER_SIZE) != access.AccessType))
+            {
+                return false;
+            }
+        }
+        else
+        {
+            if (varTypeIsGC(access.AccessType))
+            {
+                return false;
+            }
+        }
+
         unsigned countOverlappedCallArg        = 0;
         unsigned countOverlappedStoredFromCall = 0;
 
@@ -678,9 +710,8 @@ class LocalUses
 
         // Now look at the overlapping struct uses that promotion will make more expensive.
 
-        unsigned   countReadBacks    = 0;
-        weight_t   countReadBacksWtd = 0;
-        LclVarDsc* lcl               = comp->lvaGetDesc(lclNum);
+        unsigned countReadBacks    = 0;
+        weight_t countReadBacksWtd = 0;
         // For parameters or OSR locals we always need one read back.
         if (lcl->lvIsParam || lcl->lvIsOSRLocal)
         {