Revert potential UB due to aliasing + more WB removals (#111733)

dotnet · Jan 26, 2025 · 0f0753d · 0f0753d
1 parent d5c8265
commit 0f0753d
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 32 deletions.
diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp
@@ -5258,30 +5258,6 @@ static GCInfo::WriteBarrierForm GetWriteBarrierForm(Compiler* comp, ValueNum vn)
                 return GetWriteBarrierForm(comp, funcApp.m_args[0]);
             }
         }
-        if (funcApp.m_func == VNF_InitVal)
-        {
-            unsigned lclNum = vnStore->CoercedConstantValue<unsigned>(funcApp.m_args[0]);
-            assert(lclNum != BAD_VAR_NUM);
-            CORINFO_CLASS_HANDLE srcCls = NO_CLASS_HANDLE;
-
-            if (comp->compMethodHasRetVal() && (lclNum == comp->info.compRetBuffArg))
-            {
-                // See if the address is in current method's return buffer
-                // while the return type is a byref-like type.
-                srcCls = comp->info.compMethodInfo->args.retTypeClass;
-            }
-            else if (lclNum == comp->info.compThisArg)
-            {
-                // Same for implicit "this" parameter
-                assert(!comp->info.compIsStatic);
-                srcCls = comp->info.compClassHnd;
-            }
-
-            if ((srcCls != NO_CLASS_HANDLE) && comp->eeIsByrefLike(srcCls))
-            {
-                return GCInfo::WriteBarrierForm::WBF_NoBarrier;
-            }
-        }
     }
     return GCInfo::WriteBarrierForm::WBF_BarrierUnknown;
 }

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
@@ -9548,6 +9548,21 @@ void Compiler::impImportBlockCode(BasicBlock* block)
                 {
                     indirFlags |= GTF_IND_TGT_HEAP;
                 }
+                else if ((lclTyp == TYP_STRUCT) && (fieldInfo.structType != NO_CLASS_HANDLE) &&
+                         eeIsByrefLike(fieldInfo.structType))
+                {
+                    // Field's type is a byref-like struct -> address is not on the heap.
+                    indirFlags |= GTF_IND_TGT_NOT_HEAP;
+                }
+                else
+                {
+                    // Field's owner is a byref-like struct -> address is not on the heap.
+                    CORINFO_CLASS_HANDLE fldOwner = info.compCompHnd->getFieldClass(resolvedToken.hField);
+                    if ((fldOwner != NO_CLASS_HANDLE) && eeIsByrefLike(fldOwner))
+                    {
+                        indirFlags |= GTF_IND_TGT_NOT_HEAP;
+                    }
+                }
 
                 assert(varTypeIsI(op1));
                 op1 = (lclTyp == TYP_STRUCT) ? gtNewStoreBlkNode(layout, op1, op2, indirFlags)->AsIndir()

diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp
@@ -7755,14 +7755,23 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA
             break;
 
         case GT_STOREIND:
-            if (op1->OperIs(GT_FIELD_ADDR) && varTypeIsGC(tree))
+            if (varTypeIsGC(tree))
             {
-                CORINFO_FIELD_HANDLE fieldHandle = op1->AsFieldAddr()->gtFldHnd;
-                if (eeIsByrefLike(info.compCompHnd->getFieldClass(fieldHandle)))
+                GenTree* addr = op1;
+                // If we're storing a reference to a field (GT_FIELD_ADDR), let's check if the field's owner is a
+                // byref-like struct.
+                while ((addr != nullptr) && addr->OperIs(GT_FIELD_ADDR))
                 {
-                    JITDUMP("Marking [%06u] STOREIND as GTF_IND_TGT_NOT_HEAP: field's owner is a byref-like struct\n",
+                    CORINFO_FIELD_HANDLE fieldHandle = addr->AsFieldAddr()->gtFldHnd;
+                    if (eeIsByrefLike(info.compCompHnd->getFieldClass(fieldHandle)))
+                    {
+                        JITDUMP(
+                            "Marking [%06u] STOREIND as GTF_IND_TGT_NOT_HEAP: field's owner is a byref-like struct\n",
                             dspTreeID(tree));
-                    tree->gtFlags |= GTF_IND_TGT_NOT_HEAP;
+                        tree->gtFlags |= GTF_IND_TGT_NOT_HEAP;
+                        break;
+                    }
+                    addr = addr->AsFieldAddr()->GetFldObj();
                 }
             }
             break;

diff --git a/src/coreclr/jit/morphblock.cpp b/src/coreclr/jit/morphblock.cpp
@@ -1363,6 +1363,10 @@ GenTree* MorphCopyBlockHelper::CopyFieldByField()
                     if (m_store->OperIs(GT_STORE_BLK, GT_STOREIND))
                     {
                         indirFlags = m_store->gtFlags & (GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP);
+                        if (m_store->OperIs(GT_STORE_BLK) && m_store->AsBlk()->GetLayout()->IsStackOnly(m_comp))
+                        {
+                            indirFlags |= GTF_IND_TGT_NOT_HEAP;
+                        }
                     }
                     dstFldStore = m_comp->gtNewStoreIndNode(srcType, fldAddr, srcFld, indirFlags);
                 }

diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp
@@ -523,16 +523,21 @@ class DecompositionPlan
         target_ssize_t addrBaseOffs       = 0;
         FieldSeq*      addrBaseOffsFldSeq = nullptr;
         GenTreeFlags   indirFlags         = GTF_EMPTY;
-
+        GenTreeFlags   flagsToPropagate   = GTF_IND_COPYABLE_FLAGS;
         if (m_store->OperIs(GT_STORE_BLK))
         {
+            flagsToPropagate |= GTF_IND_TGT_NOT_HEAP | GTF_IND_TGT_HEAP;
             addr       = m_store->AsIndir()->Addr();
-            indirFlags = m_store->gtFlags & GTF_IND_COPYABLE_FLAGS;
+            indirFlags = m_store->gtFlags & flagsToPropagate;
+            if (m_store->AsBlk()->GetLayout()->IsStackOnly(m_compiler))
+            {
+                indirFlags |= GTF_IND_TGT_NOT_HEAP;
+            }
         }
         else if (m_src->OperIs(GT_BLK))
         {
             addr       = m_src->AsIndir()->Addr();
-            indirFlags = m_src->gtFlags & GTF_IND_COPYABLE_FLAGS;
+            indirFlags = m_src->gtFlags & flagsToPropagate;
         }
 
         int numAddrUses = 0;