diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 39bd8898548e5..61d2da86b7cc2 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -9965,6 +9965,17 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) // static bool GetStaticFieldSeqAndAddress(ValueNumStore* vnStore, GenTree* tree, ssize_t* byteOffset, FieldSeq** pFseq) { + VNFuncApp funcApp; + if (vnStore->GetVNFunc(tree->gtVNPair.GetLiberal(), &funcApp) && (funcApp.m_func == VNF_PtrToStatic)) + { + FieldSeq* fseq = vnStore->FieldSeqVNToFieldSeq(funcApp.m_args[1]); + if (fseq->GetKind() == FieldSeq::FieldKind::SimpleStatic) + { + *byteOffset = vnStore->ConstantValue(funcApp.m_args[2]); + *pFseq = fseq; + return true; + } + } ssize_t val = 0; // Special case for NativeAOT: ADD(ICON_STATIC, CNS_INT) where CNS_INT has field sequence corresponding to field's @@ -10048,7 +10059,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) // ssize_t byteOffset = 0; FieldSeq* fieldSeq = nullptr; - if ((varTypeIsSIMD(tree) || varTypeIsIntegral(tree) || varTypeIsFloating(tree)) && + if ((varTypeIsSIMD(tree) || varTypeIsIntegral(tree) || varTypeIsFloating(tree) || tree->TypeIs(TYP_REF)) && GetStaticFieldSeqAndAddress(vnStore, tree->gtGetOp1(), &byteOffset, &fieldSeq)) { CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); @@ -10127,6 +10138,20 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) tree->gtVNPair.SetBoth(vnStore->VNForDoubleCon(val)); return true; } + case TYP_REF: + { + READ_VALUE(ssize_t); + if (val == 0) + { + tree->gtVNPair.SetBoth(vnStore->VNForNull()); + } + else + { + tree->gtVNPair.SetBoth(vnStore->VNForHandle(val, GTF_ICON_OBJ_HDL)); + setMethodHasFrozenObjects(); + } + return true; + } #if defined(FEATURE_SIMD) case TYP_SIMD8: { @@ -10514,6 +10539,10 @@ void Compiler::fgValueNumberTree(GenTree* tree) tree->gtVNPair = vnStore->VNPairForLoad(lclVNPair, lclSize, tree->TypeGet(), offset, loadSize); } + else if (tree->OperIs(GT_IND, GT_BLK, GT_OBJ) && fgValueNumberConstLoad(tree->AsIndir())) + { + // VN is assigned inside fgValueNumberConstLoad + } else if (vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp) && (funcApp.m_func == VNF_PtrToStatic)) { fldSeq = vnStore->FieldSeqVNToFieldSeq(funcApp.m_args[1]); @@ -10522,10 +10551,6 @@ void Compiler::fgValueNumberTree(GenTree* tree) // Note VNF_PtrToStatic statics are currently always "simple". fgValueNumberFieldLoad(tree, /* baseAddr */ nullptr, fldSeq, offset); } - else if (tree->OperIs(GT_IND, GT_BLK, GT_OBJ) && fgValueNumberConstLoad(tree->AsIndir())) - { - // VN is assigned inside fgValueNumberConstLoad - } else if (vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp) && (funcApp.m_func == VNF_PtrToArrElem)) { fgValueNumberArrayElemLoad(tree, &funcApp); diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 8e888542fecfd..38939b28ee5d0 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11688,8 +11688,59 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t if (size >= (UINT)bufferSize && valueOffset >= 0 && (UINT)valueOffset <= size - (UINT)bufferSize) { - memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); - result = true; + // For structs containing GC pointers we want to make sure those GC pointers belong to FOH + // so we expect valueOffset to be a real field offset (same for bufferSize) + if (!field->IsRVA() && field->GetFieldType() == ELEMENT_TYPE_VALUETYPE) + { + PTR_MethodTable structType = field->GetFieldTypeHandleThrowing().AsMethodTable(); + if (structType->ContainsPointers() && (UINT)bufferSize == sizeof(CORINFO_OBJECT_HANDLE)) + { + ApproxFieldDescIterator fieldIterator(structType, ApproxFieldDescIterator::INSTANCE_FIELDS); + for (FieldDesc* subField = fieldIterator.Next(); subField != NULL; subField = fieldIterator.Next()) + { + // TODO: If subField is also a struct we might want to inspect its fields too + if (subField->GetOffset() == (DWORD)valueOffset && subField->IsObjRef()) + { + GCX_COOP(); + + // Read field's value + Object* subFieldValue = nullptr; + memcpy(&subFieldValue, (uint8_t*)baseAddr + valueOffset, bufferSize); + + if (subFieldValue == nullptr) + { + // Report null + memset(buffer, 0, bufferSize); + result = true; + } + else if (GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(subFieldValue)) + { + CORINFO_OBJECT_HANDLE handle = getJitHandleForObject( + ObjectToOBJECTREF(subFieldValue), /*knownFrozen*/ true); + + // GC handle is either from FOH or null + memcpy(buffer, &handle, bufferSize); + result = true; + } + + // We're done with this struct + break; + } + } + } + else if (!structType->ContainsPointers()) + { + // No gc pointers in the struct + memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); + result = true; + } + } + else + { + // Primitive or RVA + memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); + result = true; + } } } } diff --git a/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.cs b/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.cs new file mode 100644 index 0000000000000..b8227ece73c1f --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.cs @@ -0,0 +1,71 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.CompilerServices; +using System.Threading; + +class StaticReadonlyStructWithGC +{ + static int Main() + { + // Pre-initialize host type + RuntimeHelpers.RunClassConstructor(typeof(StaticReadonlyStructWithGC).TypeHandle); + + if (!Test1()) throw new Exception("Test1 failed"); + if (!Test2()) throw new Exception("Test2 failed"); + if (!Test3()) throw new Exception("Test3 failed"); + if (!Test4()) throw new Exception("Test4 failed"); + if (!Test5()) throw new Exception("Test5 failed"); + if (!Test6()) throw new Exception("Test6 failed"); + if (!Test7()) throw new Exception("Test7 failed"); + if (!Test8()) throw new Exception("Test8 failed"); + if (!Test9()) throw new Exception("Test9 failed"); + return 100; + } + + static readonly MyStruct MyStructFld = new() + { + A = "A", + B = 111111.ToString(), // non-literal + C = new MyStruct2 { A = "AA" }, + D = typeof(int), + E = () => 42, + F = new MyStruct3 { A = typeof(double), B = typeof(string) }, + G = new int[0], + H = null + }; + + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test1() => MyStructFld.A == "A"; + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test2() => MyStructFld.B == "111111"; + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test3() => MyStructFld.C.A == "AA"; + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test4() => MyStructFld.D == typeof(int); + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test5() => MyStructFld.E() == 42; + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test6() => MyStructFld.F.A == typeof(double); + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test7() => MyStructFld.F.B == typeof(string); + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test8() => MyStructFld.G.Length == 0; + [MethodImpl(MethodImplOptions.NoInlining)] static bool Test9() => MyStructFld.H == null; + + struct MyStruct + { + public string A; + public string B; + public MyStruct2 C; + public Type D; + public Func E; + public MyStruct3 F; + public int[] G; + public object H; + } + + struct MyStruct2 + { + public string A; + } + + struct MyStruct3 + { + public Type A; + public Type B; + } +} diff --git a/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.csproj b/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.csproj new file mode 100644 index 0000000000000..6946bed81bfd5 --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/StaticReadonlyStructWithGC.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + +