diff --git a/src/coreclr/gcinfo/gcinfoencoder.cpp b/src/coreclr/gcinfo/gcinfoencoder.cpp index c116d6d0dbc7b8..0abf65047b8d9a 100644 --- a/src/coreclr/gcinfo/gcinfoencoder.cpp +++ b/src/coreclr/gcinfo/gcinfoencoder.cpp @@ -1216,19 +1216,42 @@ void GcInfoEncoder::Build() /////////////////////////////////////////////////////////////////////// // Normalize call sites + // Eliminate call sites that fall inside interruptible ranges /////////////////////////////////////////////////////////////////////// - _ASSERTE(m_NumCallSites == 0 || numInterruptibleRanges == 0); - UINT32 numCallSites = 0; for(UINT32 callSiteIndex = 0; callSiteIndex < m_NumCallSites; callSiteIndex++) { UINT32 callSite = m_pCallSites[callSiteIndex]; - callSite += m_pCallSiteSizes[callSiteIndex]; + // There's a contract with the EE that says for non-leaf stack frames, where the + // method is stopped at a call site, the EE will not query with the return PC, but + // rather the return PC *minus 1*. + // The reason is that variable/register liveness may change at the instruction immediately after the + // call, so we want such frames to appear as if they are "within" the call. + // Since we use "callSite" as the "key" when we search for the matching descriptor, also subtract 1 here + // (after, of course, adding the size of the call instruction to get the return PC). + callSite += m_pCallSiteSizes[callSiteIndex] - 1; _ASSERTE(DENORMALIZE_CODE_OFFSET(NORMALIZE_CODE_OFFSET(callSite)) == callSite); UINT32 normOffset = NORMALIZE_CODE_OFFSET(callSite); - m_pCallSites[numCallSites++] = normOffset; + + BOOL keepIt = TRUE; + + for(UINT32 intRangeIndex = 0; intRangeIndex < numInterruptibleRanges; intRangeIndex++) + { + InterruptibleRange *pRange = &pRanges[intRangeIndex]; + if(pRange->NormStopOffset > normOffset) + { + if(pRange->NormStartOffset <= normOffset) + { + keepIt = FALSE; + } + break; + } + } + + if(keepIt) + m_pCallSites[numCallSites++] = normOffset; } GCINFO_WRITE_VARL_U(m_Info1, NORMALIZE_NUM_SAFE_POINTS(numCallSites), NUM_SAFE_POINTS_ENCBASE, NumCallSitesSize); @@ -1395,7 +1418,7 @@ void GcInfoEncoder::Build() for(pCurrent = pTransitions; pCurrent < pEndTransitions; ) { - if(pCurrent->CodeOffset >= callSite) + if(pCurrent->CodeOffset > callSite) { couldBeLive |= liveState; @@ -1750,7 +1773,7 @@ void GcInfoEncoder::Build() { for(pCurrent = pTransitions; pCurrent < pEndTransitions; ) { - if(pCurrent->CodeOffset >= callSite) + if(pCurrent->CodeOffset > callSite) { // Time to record the call site @@ -1849,7 +1872,7 @@ void GcInfoEncoder::Build() for(pCurrent = pTransitions; pCurrent < pEndTransitions; ) { - if(pCurrent->CodeOffset >= callSite) + if(pCurrent->CodeOffset > callSite) { // Time to encode the call site @@ -1896,7 +1919,7 @@ void GcInfoEncoder::Build() for(pCurrent = pTransitions; pCurrent < pEndTransitions; ) { - if(pCurrent->CodeOffset >= callSite) + if(pCurrent->CodeOffset > callSite) { // Time to encode the call site GCINFO_WRITE_VECTOR(m_Info1, liveState, CallSiteStateSize); diff --git a/src/coreclr/inc/gcinfotypes.h b/src/coreclr/inc/gcinfotypes.h index 0727e5e1a7b3e4..7457063d47eb3f 100644 --- a/src/coreclr/inc/gcinfotypes.h +++ b/src/coreclr/inc/gcinfotypes.h @@ -678,8 +678,8 @@ void FASTCALL decodeCallPattern(int pattern, #define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>2) #define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<2) #define CODE_OFFSETS_NEED_NORMALIZATION 1 -#define NORMALIZE_CODE_OFFSET(x) ((x)>>1) // Instructions are 2/4 bytes long in Thumb/ARM states, -#define DENORMALIZE_CODE_OFFSET(x) ((x)<<1) +#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 2/4 bytes long in Thumb/ARM states, +#define DENORMALIZE_CODE_OFFSET(x) (x) // but the safe-point offsets are encoded with a -1 adjustment. #define NORMALIZE_REGISTER(x) (x) #define DENORMALIZE_REGISTER(x) (x) #define NORMALIZE_NUM_SAFE_POINTS(x) (x) @@ -734,9 +734,9 @@ void FASTCALL decodeCallPattern(int pattern, #define DENORMALIZE_STACK_BASE_REGISTER(x) ((x)^29) #define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3) #define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3) -#define CODE_OFFSETS_NEED_NORMALIZATION 1 -#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long -#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2) +#define CODE_OFFSETS_NEED_NORMALIZATION 0 +#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point +#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment. #define NORMALIZE_REGISTER(x) (x) #define DENORMALIZE_REGISTER(x) (x) #define NORMALIZE_NUM_SAFE_POINTS(x) (x) @@ -789,9 +789,9 @@ void FASTCALL decodeCallPattern(int pattern, #define DENORMALIZE_STACK_BASE_REGISTER(x) ((x) == 0 ? 22 : 3) #define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3) #define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3) -#define CODE_OFFSETS_NEED_NORMALIZATION 1 -#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long -#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2) +#define CODE_OFFSETS_NEED_NORMALIZATION 0 +#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point +#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment. #define NORMALIZE_REGISTER(x) (x) #define DENORMALIZE_REGISTER(x) (x) #define NORMALIZE_NUM_SAFE_POINTS(x) (x) @@ -844,9 +844,9 @@ void FASTCALL decodeCallPattern(int pattern, #define DENORMALIZE_STACK_BASE_REGISTER(x) ((x) == 0 ? 8 : 2) #define NORMALIZE_SIZE_OF_STACK_AREA(x) ((x)>>3) #define DENORMALIZE_SIZE_OF_STACK_AREA(x) ((x)<<3) -#define CODE_OFFSETS_NEED_NORMALIZATION 1 -#define NORMALIZE_CODE_OFFSET(x) ((x)>>2) // Instructions are 4 bytes long -#define DENORMALIZE_CODE_OFFSET(x) ((x)<<2) +#define CODE_OFFSETS_NEED_NORMALIZATION 0 +#define NORMALIZE_CODE_OFFSET(x) (x) // Instructions are 4 bytes long, but the safe-point +#define DENORMALIZE_CODE_OFFSET(x) (x) // offsets are encoded with a -1 adjustment. #define NORMALIZE_REGISTER(x) (x) #define DENORMALIZE_REGISTER(x) (x) #define NORMALIZE_NUM_SAFE_POINTS(x) (x) diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index b2bdd295adf54a..dad1ba0b929b89 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1517,6 +1517,29 @@ void CodeGen::genExitCode(BasicBlock* block) if (compiler->getNeedsGSSecurityCookie()) { genEmitGSCookieCheck(jmpEpilog); + + if (jmpEpilog) + { + // Dev10 642944 - + // The GS cookie check created a temp label that has no live + // incoming GC registers, we need to fix that + + unsigned varNum; + LclVarDsc* varDsc; + + /* Figure out which register parameters hold pointers */ + + for (varNum = 0, varDsc = compiler->lvaTable; varNum < compiler->lvaCount && varDsc->lvIsRegArg; + varNum++, varDsc++) + { + noway_assert(varDsc->lvIsParam); + + gcInfo.gcMarkRegPtrVal(varDsc->GetArgReg(), varDsc->TypeGet()); + } + + GetEmitter()->emitThisGCrefRegs = GetEmitter()->emitInitGCrefRegs = gcInfo.gcRegGCrefSetCur; + GetEmitter()->emitThisByrefRegs = GetEmitter()->emitInitByrefRegs = gcInfo.gcRegByrefSetCur; + } } genReserveEpilog(block); diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index c82d2df22b9e12..d91b822afa11a9 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -10427,9 +10427,9 @@ regMaskTP emitter::emitGetGCRegsKilledByNoGCCall(CorInfoHelpFunc helper) // of the last instruction in the region makes GC safe again. // In particular - once the IP is on the first instruction, but not executed it yet, // it is still safe to do GC. -// The only special case is when NoGC region is used for prologs. -// In such case the GC info could be incorrect until the prolog completes, so the first -// instruction cannot have GC. +// The only special case is when NoGC region is used for prologs/epilogs. +// In such case the GC info could be incorrect until the prolog completes and epilogs +// may have unwindability restrictions, so the first instruction cannot have GC. void emitter::emitDisableGC() { diff --git a/src/coreclr/jit/emitinl.h b/src/coreclr/jit/emitinl.h index a586193dd5b714..022064073d908d 100644 --- a/src/coreclr/jit/emitinl.h +++ b/src/coreclr/jit/emitinl.h @@ -594,7 +594,8 @@ bool emitter::emitGenNoGCLst(Callback& cb) emitter::instrDesc* id = emitFirstInstrDesc(ig->igData); assert(id != nullptr); assert(id->idCodeSize() > 0); - if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize, id->idCodeSize(), ig->igFlags & (IGF_FUNCLET_PROLOG))) + if (!cb(ig->igFuncIdx, ig->igOffs, ig->igSize, id->idCodeSize(), + ig->igFlags & (IGF_FUNCLET_PROLOG | IGF_FUNCLET_EPILOG | IGF_EPILOG))) { return false; } diff --git a/src/coreclr/jit/gcencode.cpp b/src/coreclr/jit/gcencode.cpp index f99407e90b4e28..4863c95a7f59dd 100644 --- a/src/coreclr/jit/gcencode.cpp +++ b/src/coreclr/jit/gcencode.cpp @@ -4027,7 +4027,8 @@ class InterruptibleRangeReporter // Report everything between the previous region and the current // region as interruptible. - bool operator()(unsigned igFuncIdx, unsigned igOffs, unsigned igSize, unsigned firstInstrSize, bool isInProlog) + bool operator()( + unsigned igFuncIdx, unsigned igOffs, unsigned igSize, unsigned firstInstrSize, bool isInPrologOrEpilog) { if (igOffs < m_uninterruptibleEnd) { @@ -4043,7 +4044,7 @@ class InterruptibleRangeReporter // Once the first instruction in IG executes, we cannot have GC. // But it is ok to have GC while the IP is on the first instruction, unless we are in prolog/epilog. unsigned interruptibleEnd = igOffs; - if (!isInProlog) + if (!isInPrologOrEpilog) { interruptibleEnd += firstInstrSize; } diff --git a/src/coreclr/nativeaot/Runtime/thread.cpp b/src/coreclr/nativeaot/Runtime/thread.cpp index c2e94a1dc8f383..b796b052182260 100644 --- a/src/coreclr/nativeaot/Runtime/thread.cpp +++ b/src/coreclr/nativeaot/Runtime/thread.cpp @@ -675,16 +675,10 @@ void Thread::HijackCallback(NATIVE_CONTEXT* pThreadContext, void* pThreadToHijac if (runtime->IsConservativeStackReportingEnabled() || codeManager->IsSafePoint(pvAddress)) { - // IsUnwindable is precise on arm64, but can give false negatives on other architectures. - // (when IP is on the first instruction of an epilog, we still can unwind, - // but we can tell if the instruction is the first only if we can navigate instructions backwards and check) - // The preciseness of IsUnwindable is tracked in https://github.com/dotnet/runtime/issues/101932 -#if defined(TARGET_ARM64) // we may not be able to unwind in some locations, such as epilogs. // such locations should not contain safe points. // when scanning conservatively we do not need to unwind ASSERT(codeManager->IsUnwindable(pvAddress) || runtime->IsConservativeStackReportingEnabled()); -#endif // if we are not given a thread to hijack // perform in-line wait on the current thread diff --git a/src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp b/src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp index 7cf5bfc792edb7..b12d63bf726129 100644 --- a/src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp +++ b/src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp @@ -213,18 +213,44 @@ void UnixNativeCodeManager::EnumGcRefs(MethodInfo * pMethodInfo, #ifdef TARGET_ARM // Ensure that code offset doesn't have the Thumb bit set. We need - // it to be aligned to instruction start + // it to be aligned to instruction start to make the !isActiveStackFrame + // branch below work. ASSERT(((uintptr_t)codeOffset & 1) == 0); #endif + bool executionAborted = ((UnixNativeMethodInfo*)pMethodInfo)->executionAborted; + + if (!isActiveStackFrame && !executionAborted) + { + // the reasons for this adjustment are explained in EECodeManager::EnumGcRefs + codeOffset--; + } + GcInfoDecoder decoder( GCInfoToken(gcInfo), GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG), codeOffset ); + if (isActiveStackFrame) + { + // CONSIDER: We can optimize this by remembering the need to adjust in IsSafePoint and propagating into here. + // Or, better yet, maybe we should change the decoder to not require this adjustment. + // The scenario that adjustment tries to handle (fallthrough into BB with random liveness) + // does not seem possible. + if (!decoder.HasInterruptibleRanges()) + { + decoder = GcInfoDecoder( + GCInfoToken(gcInfo), + GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG), + codeOffset - 1 + ); + + assert(decoder.IsInterruptibleSafePoint()); + } + } + ICodeManagerFlags flags = (ICodeManagerFlags)0; - bool executionAborted = ((UnixNativeMethodInfo*)pMethodInfo)->executionAborted; if (executionAborted) flags = ICodeManagerFlags::ExecutionAborted; diff --git a/src/coreclr/nativeaot/Runtime/windows/CoffNativeCodeManager.cpp b/src/coreclr/nativeaot/Runtime/windows/CoffNativeCodeManager.cpp index 051db8ddd00b77..b8c2310d644004 100644 --- a/src/coreclr/nativeaot/Runtime/windows/CoffNativeCodeManager.cpp +++ b/src/coreclr/nativeaot/Runtime/windows/CoffNativeCodeManager.cpp @@ -440,8 +440,9 @@ void CoffNativeCodeManager::EnumGcRefs(MethodInfo * pMethodInfo, PTR_uint8_t gcInfo; uint32_t codeOffset = GetCodeOffset(pMethodInfo, safePointAddress, &gcInfo); - ICodeManagerFlags flags = (ICodeManagerFlags)0; bool executionAborted = ((CoffNativeMethodInfo *)pMethodInfo)->executionAborted; + + ICodeManagerFlags flags = (ICodeManagerFlags)0; if (executionAborted) flags = ICodeManagerFlags::ExecutionAborted; @@ -452,6 +453,11 @@ void CoffNativeCodeManager::EnumGcRefs(MethodInfo * pMethodInfo, flags = (ICodeManagerFlags)(flags | ICodeManagerFlags::ActiveStackFrame); #ifdef USE_GC_INFO_DECODER + if (!isActiveStackFrame && !executionAborted) + { + // the reasons for this adjustment are explained in EECodeManager::EnumGcRefs + codeOffset--; + } GcInfoDecoder decoder( GCInfoToken(gcInfo), @@ -459,6 +465,24 @@ void CoffNativeCodeManager::EnumGcRefs(MethodInfo * pMethodInfo, codeOffset ); + if (isActiveStackFrame) + { + // CONSIDER: We can optimize this by remembering the need to adjust in IsSafePoint and propagating into here. + // Or, better yet, maybe we should change the decoder to not require this adjustment. + // The scenario that adjustment tries to handle (fallthrough into BB with random liveness) + // does not seem possible. + if (!decoder.HasInterruptibleRanges()) + { + decoder = GcInfoDecoder( + GCInfoToken(gcInfo), + GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG), + codeOffset - 1 + ); + + assert(decoder.IsInterruptibleSafePoint()); + } + } + if (!decoder.EnumerateLiveSlots( pRegisterSet, isActiveStackFrame /* reportScratchSlots */, diff --git a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/GcInfo.cs b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/GcInfo.cs index aa635e1b06158f..b934e36719e8a2 100644 --- a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/GcInfo.cs +++ b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/GcInfo.cs @@ -48,7 +48,6 @@ public SafePointOffset(int index, uint value) private const int MIN_GCINFO_VERSION_WITH_RETURN_KIND = 2; private const int MIN_GCINFO_VERSION_WITH_REV_PINVOKE_FRAME = 2; - private const int MIN_GCINFO_VERSION_WITH_NORMALIZED_CODE_OFFSETS = 3; private bool _slimHeader; private bool _hasSecurityObject; @@ -89,9 +88,7 @@ public GcInfo() { } public GcInfo(byte[] image, int offset, Machine machine, ushort majorVersion, ushort minorVersion) { Offset = offset; - Version = ReadyToRunVersionToGcInfoVersion(majorVersion, minorVersion); - bool denormalizeCodeOffsets = Version > MIN_GCINFO_VERSION_WITH_NORMALIZED_CODE_OFFSETS; - _gcInfoTypes = new GcInfoTypes(machine, denormalizeCodeOffsets); + _gcInfoTypes = new GcInfoTypes(machine); _machine = machine; SecurityObjectStackSlot = -1; @@ -103,6 +100,7 @@ public GcInfo(byte[] image, int offset, Machine machine, ushort majorVersion, us SizeOfEditAndContinuePreservedArea = 0xffffffff; ReversePInvokeFrameStackSlot = -1; + Version = ReadyToRunVersionToGcInfoVersion(majorVersion, minorVersion); int bitOffset = offset * 8; ParseHeaderFlags(image, ref bitOffset); @@ -120,13 +118,12 @@ public GcInfo(byte[] image, int offset, Machine machine, ushort majorVersion, us uint normPrologSize = NativeReader.DecodeVarLengthUnsigned(image, _gcInfoTypes.NORM_PROLOG_SIZE_ENCBASE, ref bitOffset) + 1; uint normEpilogSize = NativeReader.DecodeVarLengthUnsigned(image, _gcInfoTypes.NORM_EPILOG_SIZE_ENCBASE, ref bitOffset); - ValidRangeStart = _gcInfoTypes.DenormalizeCodeOffset(normPrologSize); - ValidRangeEnd = (uint)CodeLength - _gcInfoTypes.DenormalizeCodeOffset(normEpilogSize); + ValidRangeStart = normPrologSize; + ValidRangeEnd = (uint)CodeLength - normEpilogSize; } else if (_hasSecurityObject || _hasGenericsInstContext) { - uint normValidRangeStart = NativeReader.DecodeVarLengthUnsigned(image, _gcInfoTypes.NORM_PROLOG_SIZE_ENCBASE, ref bitOffset) + 1; - ValidRangeStart = _gcInfoTypes.DenormalizeCodeOffset(normValidRangeStart); + ValidRangeStart = NativeReader.DecodeVarLengthUnsigned(image, _gcInfoTypes.NORM_PROLOG_SIZE_ENCBASE, ref bitOffset) + 1; ValidRangeEnd = ValidRangeStart + 1; } @@ -355,11 +352,11 @@ private void ParseHeaderFlags(byte[] image, ref int bitOffset) private List EnumerateSafePoints(byte[] image, ref int bitOffset) { List safePoints = new List(); - uint numBitsPerOffset = GcInfoTypes.CeilOfLog2((int)_gcInfoTypes.NormalizeCodeOffset((uint)CodeLength)); + uint numBitsPerOffset = GcInfoTypes.CeilOfLog2(CodeLength); for (int i = 0; i < NumSafePoints; i++) { uint normOffset = (uint)NativeReader.ReadBits(image, (int)numBitsPerOffset, ref bitOffset); - safePoints.Add(new SafePointOffset(i, _gcInfoTypes.DenormalizeCodeOffset(normOffset))); + safePoints.Add(new SafePointOffset(i, normOffset)); } return safePoints; } @@ -370,21 +367,18 @@ private List EnumerateSafePoints(byte[] image, ref int bitOffse private List EnumerateInterruptibleRanges(byte[] image, int interruptibleRangeDelta1EncBase, int interruptibleRangeDelta2EncBase, ref int bitOffset) { List ranges = new List(); - uint normLastinterruptibleRangeStopOffset = 0; + uint lastinterruptibleRangeStopOffset = 0; for (uint i = 0; i < NumInterruptibleRanges; i++) { uint normStartDelta = NativeReader.DecodeVarLengthUnsigned(image, interruptibleRangeDelta1EncBase, ref bitOffset); uint normStopDelta = NativeReader.DecodeVarLengthUnsigned(image, interruptibleRangeDelta2EncBase, ref bitOffset) + 1; - uint normRangeStartOffset = normLastinterruptibleRangeStopOffset + normStartDelta; - uint normRangeStopOffset = normRangeStartOffset + normStopDelta; - - uint rangeStartOffset = _gcInfoTypes.DenormalizeCodeOffset(normRangeStopOffset); - uint rangeStopOffset = _gcInfoTypes.DenormalizeCodeOffset(normRangeStartOffset); + uint rangeStartOffset = lastinterruptibleRangeStopOffset + normStartDelta; + uint rangeStopOffset = rangeStartOffset + normStopDelta; ranges.Add(new InterruptibleRange(i, rangeStartOffset, rangeStopOffset)); - normLastinterruptibleRangeStopOffset = normRangeStopOffset; + lastinterruptibleRangeStopOffset = rangeStopOffset; } return ranges; } diff --git a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/GCInfoTypes.cs b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/GCInfoTypes.cs index 653425781a7cf6..d3adc462409589 100644 --- a/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/GCInfoTypes.cs +++ b/src/coreclr/tools/aot/ILCompiler.Reflection.ReadyToRun/GCInfoTypes.cs @@ -73,7 +73,6 @@ enum InfoHdrAdjust public class GcInfoTypes { private Machine _target; - private bool _denormalizeCodeOffsets; internal int SIZE_OF_RETURN_KIND_SLIM { get; } = 2; internal int SIZE_OF_RETURN_KIND_FAT { get; } = 2; @@ -106,10 +105,9 @@ public class GcInfoTypes internal int LIVESTATE_RLE_SKIP_ENCBASE { get; } = 4; internal int NUM_NORM_CODE_OFFSETS_PER_CHUNK_LOG2 { get; } = 6; - internal GcInfoTypes(Machine machine, bool denormalizeCodeOffsets) + internal GcInfoTypes(Machine machine) { _target = machine; - _denormalizeCodeOffsets = denormalizeCodeOffsets; switch (machine) { @@ -178,34 +176,6 @@ internal int DenormalizeCodeLength(int x) return x; } - internal int NormalizeCodeLength(int x) - { - switch (_target) - { - case Machine.ArmThumb2: - return (x >> 1); - case Machine.Arm64: - case Machine.LoongArch64: - case Machine.RiscV64: - return (x >> 2); - } - return x; - } - - internal uint DenormalizeCodeOffset(uint x) - { - return _denormalizeCodeOffsets ? - (uint)DenormalizeCodeLength((int)x) : - x; - } - - internal uint NormalizeCodeOffset(uint x) - { - return _denormalizeCodeOffsets ? - (uint)NormalizeCodeLength((int)x) : - x; - } - internal int DenormalizeStackSlot(int x) { switch (_target) diff --git a/src/coreclr/vm/eetwain.cpp b/src/coreclr/vm/eetwain.cpp index cf2867f38fe5a8..5746c44de4a770 100644 --- a/src/coreclr/vm/eetwain.cpp +++ b/src/coreclr/vm/eetwain.cpp @@ -1436,6 +1436,37 @@ bool EECodeManager::EnumGcRefs( PREGDISPLAY pRD, } #endif + /* If we are not in the active method, we are currently pointing + * to the return address; at the return address stack variables + * can become dead if the call is the last instruction of a try block + * and the return address is the jump around the catch block. Therefore + * we simply assume an offset inside of call instruction. + * NOTE: The GcInfoDecoder depends on this; if you change it, you must + * revisit the GcInfoEncoder/Decoder + */ + + if (!(flags & ExecutionAborted)) + { + if (!(flags & ActiveStackFrame)) + { + curOffs--; + LOG((LF_GCINFO, LL_INFO1000, "Adjusted GC reporting offset due to flags !ExecutionAborted && !ActiveStackFrame. Now reporting GC refs for %s at offset %04x.\n", + methodName, curOffs)); + } + } + else + { + // Since we are aborting execution, we are either in a frame that actually faulted or in a throwing call. + // * We do not need to adjust in a leaf + // * A throwing call will have unreachable after it, thus GC info is the same as before the call. + // + // Either way we do not need to adjust. + + // NOTE: only fully interruptible methods may need to report anything here as without + // exception handling all current local variables are already unreachable. + // EnumerateLiveSlots will shortcircuit the partially interruptible case just a bit later. + } + // Check if we have been given an override value for relOffset if (relOffsetOverride != NO_OVERRIDE_OFFSET) { @@ -1479,6 +1510,7 @@ bool EECodeManager::EnumGcRefs( PREGDISPLAY pRD, // A frame is non-leaf if we are executing a call, or a fault occurred in the function. // The only case in which we need to report scratch slots for a non-leaf frame // is when execution has to be resumed at the point of interruption (via ResumableFrame) + //Implement ResumableFrame _ASSERTE( sizeof( BOOL ) >= sizeof( ActiveStackFrame ) ); reportScratchSlots = (flags & ActiveStackFrame) != 0; @@ -1489,6 +1521,24 @@ bool EECodeManager::EnumGcRefs( PREGDISPLAY pRD, curOffs ); + if ((flags & ActiveStackFrame) != 0) + { + // CONSIDER: We can optimize this by remembering the need to adjust in IsSafePoint and propagating into here. + // Or, better yet, maybe we should change the decoder to not require this adjustment. + // The scenario that adjustment tries to handle (fallthrough into BB with random liveness) + // does not seem possible. + if (!gcInfoDecoder.HasInterruptibleRanges()) + { + gcInfoDecoder = GcInfoDecoder( + gcInfoToken, + GcInfoDecoderFlags(DECODE_GC_LIFETIMES | DECODE_SECURITY_OBJECT | DECODE_VARARG), + curOffs - 1 + ); + + _ASSERTE((InterruptibleSafePointsEnabled() && gcInfoDecoder.CouldBeInterruptibleSafePoint())); + } + } + if (!gcInfoDecoder.EnumerateLiveSlots( pRD, reportScratchSlots, diff --git a/src/coreclr/vm/gcinfodecoder.cpp b/src/coreclr/vm/gcinfodecoder.cpp index 97588ca0c93138..32c74bab8b4092 100644 --- a/src/coreclr/vm/gcinfodecoder.cpp +++ b/src/coreclr/vm/gcinfodecoder.cpp @@ -367,7 +367,11 @@ GcInfoDecoder::GcInfoDecoder( { if(m_NumSafePoints) { - m_SafePointIndex = FindSafePoint(m_InstructionOffset); + // Safepoints are encoded with a -1 adjustment + // DECODE_GC_LIFETIMES adjusts the offset accordingly, but DECODE_INTERRUPTIBILITY does not + // adjust here + UINT32 offset = flags & DECODE_INTERRUPTIBILITY ? m_InstructionOffset - 1 : m_InstructionOffset; + m_SafePointIndex = FindSafePoint(offset); } } else if(flags & DECODE_FOR_RANGES_CALLBACK) @@ -453,6 +457,10 @@ bool GcInfoDecoder::IsSafePoint(UINT32 codeOffset) if(m_NumSafePoints == 0) return false; +#if defined(TARGET_AMD64) || defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) + // Safepoints are encoded with a -1 adjustment + codeOffset--; +#endif size_t savedPos = m_Reader.GetCurrentPos(); UINT32 safePointIndex = FindSafePoint(codeOffset); m_Reader.SetCurrentPos(savedPos); @@ -499,26 +507,32 @@ UINT32 GcInfoDecoder::FindSafePoint(UINT32 breakOffset) const size_t savedPos = m_Reader.GetCurrentPos(); const UINT32 numBitsPerOffset = CeilOfLog2(NORMALIZE_CODE_OFFSET(m_CodeLength)); - const UINT32 normBreakOffset = NORMALIZE_CODE_OFFSET(breakOffset); - UINT32 linearSearchStart = 0; - UINT32 linearSearchEnd = m_NumSafePoints; - if (linearSearchEnd - linearSearchStart > MAX_LINEAR_SEARCH) - { - linearSearchStart = NarrowSafePointSearch(savedPos, normBreakOffset, &linearSearchEnd); - } - - for (UINT32 i = linearSearchStart; i < linearSearchEnd; i++) +#if defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) + // Safepoints are encoded with a -1 adjustment + if ((breakOffset & 1) != 0) +#endif { - UINT32 spOffset = (UINT32)m_Reader.Read(numBitsPerOffset); - if (spOffset == normBreakOffset) + const UINT32 normBreakOffset = NORMALIZE_CODE_OFFSET(breakOffset); + UINT32 linearSearchStart = 0; + UINT32 linearSearchEnd = m_NumSafePoints; + if (linearSearchEnd - linearSearchStart > MAX_LINEAR_SEARCH) { - result = i; - break; + linearSearchStart = NarrowSafePointSearch(savedPos, normBreakOffset, &linearSearchEnd); } - if (spOffset > normBreakOffset) + for (UINT32 i = linearSearchStart; i < linearSearchEnd; i++) { - break; + UINT32 spOffset = (UINT32)m_Reader.Read(numBitsPerOffset); + if (spOffset == normBreakOffset) + { + result = i; + break; + } + + if (spOffset > normBreakOffset) + { + break; + } } } @@ -539,7 +553,13 @@ void GcInfoDecoder::EnumerateSafePoints(EnumerateSafePointsCallback *pCallback, for(UINT32 i = 0; i < m_NumSafePoints; i++) { UINT32 normOffset = (UINT32)m_Reader.Read(numBitsPerOffset); - UINT32 offset = DENORMALIZE_CODE_OFFSET(normOffset); + UINT32 offset = DENORMALIZE_CODE_OFFSET(normOffset) + 2; + +#if defined(TARGET_AMD64) || defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64) || defined(TARGET_RISCV64) + // Safepoints are encoded with a -1 adjustment + offset--; +#endif + pCallback(this, offset, hCallback); } } @@ -709,6 +729,15 @@ bool GcInfoDecoder::EnumerateLiveSlots( return true; } + // + // If this is a non-leaf frame and we are executing a call, the unwinder has given us the PC + // of the call instruction. We should adjust it to the PC of the instruction after the call in order to + // obtain transition information for scratch slots. However, we always assume scratch slots to be + // dead for non-leaf frames (except for ResumableFrames), so we don't need to adjust the PC. + // If this is a non-leaf frame and we are not executing a call (i.e.: a fault occurred in the function), + // then it would be incorrect to adjust the PC + // + _ASSERTE(GC_SLOT_INTERIOR == GC_CALL_INTERIOR); _ASSERTE(GC_SLOT_PINNED == GC_CALL_PINNED);