Debugger fixes for the Windows X86 EH Funclets model

src/coreclr/debug/ee/debugger.cpp

@@ -7573,7 +7573,8 @@ HRESULT Debugger::SendException(Thread *pThread,
     }
     CONTRACTL_END;
 
-    LOG((LF_CORDB, LL_INFO10000, "D::SendException\n"));
+    LOG((LF_CORDB, LL_INFO10000, "D::SendException pThread=0x%p fFirstChance=%s currentIP=0x%p currentSP= 0x%p fContinuable=%s fAttaching=%s fForceNonInterceptable=%s\n",
+        pThread, fFirstChance ? "true" : "false", (void*)currentIP, (void*)currentSP, fContinuable ? "true" : "false", fAttaching ? "true" : "false", fForceNonInterceptable ? "true" : "false"));
 
     if (CORDBUnrecoverableError(this))
     {

src/coreclr/debug/ee/frameinfo.cpp

@@ -1265,7 +1265,7 @@ FramePointer GetFramePointerForDebugger(DebuggerFrameData* pData, CrawlFrame* pC
 
     FramePointer fpResult;
 
-#if defined(FEATURE_EH_FUNCLETS)
+#if !defined(TARGET_X86)
     if (pData->info.frame == NULL)
     {
         // This is a managed method frame.
@@ -1277,7 +1277,7 @@ FramePointer GetFramePointerForDebugger(DebuggerFrameData* pData, CrawlFrame* pC
         fpResult = FramePointer::MakeFramePointer((LPVOID)(pData->info.frame));
     }
 
-#else  // !FEATURE_EH_FUNCLETS
+#else  // !TARGET_X86
     if ((pCF == NULL || !pCF->IsFrameless()) && pData->info.frame != NULL)
     {
         //
@@ -1299,7 +1299,7 @@ FramePointer GetFramePointerForDebugger(DebuggerFrameData* pData, CrawlFrame* pC
         fpResult = FramePointer::MakeFramePointer((LPVOID)GetRegdisplayStackMark(&(pData->regDisplay)));
     }
 
-#endif // !FEATURE_EH_FUNCLETS
+#endif // !TARGET_X86
 
     LOG((LF_CORDB, LL_INFO100000, "GFPFD: Frame pointer is 0x%p\n", fpResult.GetSPValue()));
 

src/coreclr/inc/clrnt.h

@@ -211,7 +211,7 @@ RtlpGetFunctionEndAddress (
     _In_ TADDR ImageBase
     )
 {
-    PUNWIND_INFO pUnwindInfo = (PUNWIND_INFO)(ImageBase + FunctionEntry->UnwindData);
+    DPTR(UNWIND_INFO) pUnwindInfo = dac_cast<DPTR(UNWIND_INFO)>(ImageBase + FunctionEntry->UnwindData);
 
     return FunctionEntry->BeginAddress + pUnwindInfo->FunctionLength;
 }

src/coreclr/inc/eetwain.h

@@ -259,14 +259,15 @@ virtual void * GetGSCookieAddr(PREGDISPLAY     pContext,
 virtual bool IsInPrologOrEpilog(DWORD  relPCOffset,
                                 GCInfoToken gcInfoToken,
                                 size_t* prologSize) = 0;
-
+#ifndef FEATURE_EH_FUNCLETS
 /*
   Returns true if the given IP is in the synchronized region of the method (valid for synchronized methods only)
 */
 virtual bool IsInSynchronizedRegion(
                 DWORD       relOffset,
                 GCInfoToken gcInfoToken,
                 unsigned    flags) = 0;
+#endif // FEATURE_EH_FUNCLETS
 #endif // !USE_GC_INFO_DECODER
 
 /*
@@ -513,6 +514,7 @@ bool IsInPrologOrEpilog(
                 GCInfoToken gcInfoToken,
                 size_t*     prologSize);
 
+#ifndef FEATURE_EH_FUNCLETS
 /*
   Returns true if the given IP is in the synchronized region of the method (valid for synchronized functions only)
 */
@@ -521,6 +523,7 @@ bool IsInSynchronizedRegion(
                 DWORD       relOffset,
                 GCInfoToken gcInfoToken,
                 unsigned    flags);
+#endif // FEATURE_EH_FUNCLETS
 #endif // !USE_GC_INFO_DECODER
 
 /*
@@ -710,6 +713,7 @@ bool IsInPrologOrEpilog(
     return false;
 }
 
+#ifndef FEATURE_EH_FUNCLETS
 virtual
 bool IsInSynchronizedRegion(
                 DWORD       relOffset,
@@ -720,6 +724,7 @@ bool IsInSynchronizedRegion(
     _ASSERTE(FALSE);
     return false;
 }
+#endif // FEATURE_EH_FUNCLETS
 #endif // !USE_GC_INFO_DECODER
 
 virtual

src/coreclr/inc/regdisp.h

@@ -132,7 +132,11 @@ inline TADDR GetRegdisplayFP(REGDISPLAY *display) {
 inline LPVOID GetRegdisplayFPAddress(REGDISPLAY *display) {
     LIMITED_METHOD_CONTRACT;
 
+#ifdef FEATURE_EH_FUNCLETS
+    return &display->pCurrentContext->Ebp;
+#else
     return (LPVOID)display->GetEbpLocation();
+#endif
 }
 
 inline TADDR GetRegdisplayPCTAddr(REGDISPLAY *display)

src/coreclr/jit/codegenxarch.cpp

@@ -11101,6 +11101,13 @@ void CodeGen::genFuncletProlog(BasicBlock* block)
 #ifdef UNIX_X86_ABI
     // Add a padding for 16-byte alignment
     inst_RV_IV(INS_sub, REG_SPBASE, 12, EA_PTRSIZE);
+#else
+    if (!compiler->IsTargetAbi(CORINFO_NATIVEAOT_ABI))
+    {
+        // Funclet prologs need to have at least 1 byte or the IL->Native mapping data will not
+        // include the first IL instruction in the funclet.
+        instGen(INS_nop);
+    }
 #endif
 }
 

src/coreclr/jit/gcencode.cpp

@@ -1578,6 +1578,22 @@ size_t GCInfo::gcInfoBlockHdrSave(
         assert(header->epilogCount <= 1);
     }
 #endif
+    if (compiler->UsesFunclets() && compiler->info.compFlags & CORINFO_FLG_SYNCH)
+    {
+        // While the sync start offset and end offset are not used by the stackwalker/EH system
+        // in funclets mode, we do need to know if the code is synchronized if we are generating
+        // an edit and continue method, so that we can properly manage the stack during a Remap
+        // operation, for determining the ParamTypeArg for collectible generics purposes, and
+        // for determining the offset of the localloc variable in the stack frame.
+        // Instead of inventing a new encoding, just encode some non-0 offsets into these fields.
+        // to indicate that the method is synchronized.
+        //
+        // Use 1 for both offsets, since that doesn't actually make sense and implies that the
+        // sync region is 0 bytes long. The JIT will never emit a sync region of 0 bytes in non-
+        // funclet mode.
+        header->syncStartOffset = 1;
+        header->syncEndOffset   = 1;
+    }
 
     header->revPInvokeOffset = INVALID_REV_PINVOKE_OFFSET;
     if (compiler->opts.IsReversePInvoke())

src/coreclr/jit/lclvars.cpp

@@ -3673,6 +3673,9 @@ PhaseStatus Compiler::lvaMarkLocalVars()
     //      saved EBP                       <-- EBP points here
     //      other callee-saved registers    // InfoHdrSmall.savedRegsCountExclFP specifies this size
     //      optional GS cookie              // InfoHdrSmall.security is 1 if this exists
+    // if FEATURE_EH_FUNCLETS
+    //      issynchronized bool if it is a synchronized method
+    // endif // FEATURE_EH_FUNCLETS
     //      LocAllocSP slot
     //      -- lower addresses --
     //
@@ -4085,13 +4088,15 @@ unsigned Compiler::lvaGetMaxSpillTempSize()
  *      |   security object     |
  *      |-----------------------|
  *      |     ParamTypeArg      |
+// If funclet support is disabled
  *      |-----------------------|
  *      |  Last-executed-filter |
  *      |-----------------------|
  *      |                       |
  *      ~      Shadow SPs       ~
  *      |                       |
  *      |-----------------------|
+// Endif funclet support is disabled
  *      |                       |
  *      ~      Variables        ~
  *      |                       |

src/coreclr/nativeaot/Runtime.Base/src/System/Runtime/StackFrameIterator.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 #if !NATIVEAOT
 using System.Runtime.ExceptionServices;
@@ -60,6 +61,7 @@ internal unsafe struct StackFrameIterator
 #pragma warning restore CA1822
 #endif // NATIVEAOT
 
+        [StackTraceHidden]
         internal bool Init(EH.PAL_LIMITED_CONTEXT* pStackwalkCtx, bool instructionFault = false, bool* fIsExceptionIntercepted = null)
         {
             return InternalCalls.RhpSfiInit(ref this, pStackwalkCtx, instructionFault, fIsExceptionIntercepted);

src/coreclr/vm/eetwain.cpp

@@ -161,7 +161,7 @@ bool        VarIsInReg(ICorDebugInfo::VarLoc varLoc)
  *  could easily duplicate what they do, which is why we're calling into them.
  */
 
-HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
+ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
                                         EECodeInfo *     pOldCodeInfo,
                    const ICorDebugInfo::NativeVarInfo *  oldMethodVars,
                                         SIZE_T           oldMethodVarsCount,
@@ -206,6 +206,11 @@ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
         return E_FAIL; // stack should be empty - <TODO> @TODO : Barring localloc</TODO>
     }
 
+#ifdef FEATURE_EH_FUNCLETS
+    // EnC remap inside handlers is not supported
+    if (pOldCodeInfo->IsFunclet() || pNewCodeInfo->IsFunclet())
+        return CORDBG_E_ENC_IN_FUNCLET;
+#else
     if (oldInfo->handlers)
     {
         bool      hasInnerFilter;
@@ -229,13 +234,16 @@ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
             }
         }
     }
+#endif // FEATURE_EH_FUNCLETS
 
     /* @TODO: Check if we have grown out of space for locals, in the face of localloc */
     _ASSERTE(!oldInfo->localloc && !newInfo->localloc);
 
+#ifndef FEATURE_EH_FUNCLETS
     // @TODO: If nesting level grows above the MAX_EnC_HANDLER_NESTING_LEVEL,
     // we should return EnC_NESTED_HANLDERS
     _ASSERTE(oldInfo->handlers && newInfo->handlers);
+#endif
 
     LOG((LF_ENC, LL_INFO100, "EECM::FixContextForEnC: Checks out\n"));
 
@@ -1542,7 +1550,7 @@ OBJECTREF EECodeManager::GetInstance( PREGDISPLAY    pContext,
 #endif
 
 #else // FEATURE_EH_FUNCLETS
-    if (pCodeInfo->GetMethodDesc()->AcquiresInstMethodTableFromThis()) // Generic Context is "this"
+    if (pCodeInfo->GetMethodDesc()->AcquiresInstMethodTableFromThis() && (hdrInfoBody->genericsContext)) // Generic Context is "this"
     {
         // Untracked table must have at least one entry - this pointer
         _ASSERTE(hdrInfoBody->untrackedCnt > 0);
@@ -1797,6 +1805,7 @@ bool EECodeManager::IsInPrologOrEpilog(DWORD       relPCoffset,
             (info.epilogOffs != hdrInfo::NOT_IN_EPILOG));
 }
 
+#ifndef FEATURE_EH_FUNCLETS
 /*****************************************************************************
  *
  *  Returns true if the given IP is in the synchronized region of the method (valid for synchronized functions only)
@@ -1827,6 +1836,7 @@ bool  EECodeManager::IsInSynchronizedRegion(DWORD       relOffset,
         // Everything after the epilog is also in synchronized region.
         (info.epilogCnt != 0 && info.syncEpilogStart + info.epilogSize <= relOffset);
 }
+#endif // FEATURE_EH_FUNCLETS
 #endif // !USE_GC_INFO_DECODER
 
 /*****************************************************************************

src/coreclr/vm/excep.cpp

@@ -6999,7 +6999,7 @@ LONG WINAPI CLRVectoredExceptionHandlerShim(PEXCEPTION_POINTERS pExceptionInfo)
         return EXCEPTION_CONTINUE_SEARCH;
     }
 
-#if defined(TARGET_X86)
+#if defined(TARGET_X86) && !defined(FEATURE_EH_FUNCLETS)
     if (dwCode == EXCEPTION_BREAKPOINT || dwCode == EXCEPTION_SINGLE_STEP)
     {
         // For interop debugging, debugger bashes our managed exception handler.
@@ -11324,7 +11324,13 @@ void SoftwareExceptionFrame::UpdateRegDisplay_Impl(const PREGDISPLAY pRD, bool u
     ENUM_CALLEE_SAVED_REGISTERS();
 #undef CALLEE_SAVED_REGISTER
 
+#if defined(DACCESS_COMPILE) && defined(TARGET_X86)
+// X86 unwinding always works in terms of context pointers, so they need to be in the correct address space when debugging
+// This may work for other architectures as well, but that isn't tested.
+#define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = &pRD->pCurrentContext->regname;
+#else
 #define CALLEE_SAVED_REGISTER(regname) pRD->pCurrentContextPointers->regname = m_ContextPointers.regname;
+#endif
     ENUM_CALLEE_SAVED_REGISTERS();
 #undef CALLEE_SAVED_REGISTER
 

src/coreclr/vm/exceptionhandling.cpp

@@ -2956,7 +2956,11 @@ void MarkInlinedCallFrameAsEHHelperCall(Frame* pFrame)
 static TADDR GetSpForDiagnosticReporting(REGDISPLAY *pRD)
 {
 #ifdef ESTABLISHER_FRAME_ADDRESS_IS_CALLER_SP
-    return CallerStackFrame::FromRegDisplay(pRD).SP;
+    TADDR sp = CallerStackFrame::FromRegDisplay(pRD).SP;
+#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_X86)
+    sp -= sizeof(TADDR); // For X86 with funclets we want the address 1 pointer into the callee.
+#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_X86)
+    return sp;
 #else
     return GetSP(pRD->pCurrentContext);
 #endif

src/coreclr/vm/gc_unwind_x86.inl

@@ -189,7 +189,12 @@ size_t DecodeGCHdrInfo(GCInfoToken gcInfoToken,
         header.syncEndOffset = decodeUnsigned(table);
 
         _ASSERTE(header.syncStartOffset != INVALID_SYNC_OFFSET && header.syncEndOffset != INVALID_SYNC_OFFSET);
+#ifdef FEATURE_EH_FUNCLETS
+        _ASSERTE(header.syncStartOffset == 1);
+        _ASSERTE(header.syncEndOffset == 1);
+#else
         _ASSERTE(header.syncStartOffset < header.syncEndOffset);
+#endif
     }
 
     if (header.revPInvokeOffset == HAS_REV_PINVOKE_FRAME_OFFSET)
@@ -366,6 +371,9 @@ size_t GetLocallocSPOffset(hdrInfo * info)
     _ASSERTE(info->localloc && info->ebpFrame);
 
     unsigned position = info->savedRegsCountExclFP +
+#ifdef FEATURE_EH_FUNCLETS
+                        ((info->syncStartOffset != INVALID_SYNC_OFFSET) ? 1 : 0) + // Is this method synchronized
+#endif
                         1;
     return position * sizeof(TADDR);
 }
@@ -377,12 +385,20 @@ size_t GetParamTypeArgOffset(hdrInfo * info)
 
     _ASSERTE((info->genericsContext || info->handlers) && info->ebpFrame);
 
+#ifdef FEATURE_EH_FUNCLETS
     unsigned position = info->savedRegsCountExclFP +
+                        ((info->syncStartOffset != INVALID_SYNC_OFFSET) ? 1 : 0) + // Is this method synchronized
                         info->localloc +
                         1;  // For CORINFO_GENERICS_CTXT_FROM_PARAMTYPEARG
+#else
+    unsigned position = info->savedRegsCountExclFP +
+                        info->localloc +
+                        1;  // For CORINFO_GENERICS_CTXT_FROM_PARAMTYPEARG
+#endif
     return position * sizeof(TADDR);
 }
 
+#ifndef FEATURE_EH_FUNCLETS
 inline size_t GetStartShadowSPSlotsOffset(hdrInfo * info)
 {
     LIMITED_METHOD_DAC_CONTRACT;
@@ -425,6 +441,7 @@ inline size_t GetEndShadowSPSlotsOffset(hdrInfo * info, unsigned maxHandlerNesti
     return GetStartShadowSPSlotsOffset(info) +
            (numberOfShadowSPSlots * sizeof(TADDR));
 }
+#endif // FEATURE_EH_FUNCLETS
 
 /*****************************************************************************
  *    returns the base frame pointer corresponding to the target nesting level.
@@ -524,7 +541,6 @@ FrameType   GetHandlerFrameInfo(hdrInfo   * info,
             _ASSERTE(condition);                        \
         }
 
-    PTR_TADDR pFirstBaseSPslot = GetFirstBaseSPslotPtr(frameEBP, info);
     TADDR  baseSP            = GetOutermostBaseFP(frameEBP, info);
     bool    nonLocalHandlers = false; // Are the funclets invoked by EE (instead of managed code itself)
     bool    hasInnerFilter   = false;
@@ -539,6 +555,7 @@ FrameType   GetHandlerFrameInfo(hdrInfo   * info,
     // expected to be in decreasing order.
     size_t lvl = 0;
 #ifndef FEATURE_EH_FUNCLETS
+    PTR_TADDR pFirstBaseSPslot = GetFirstBaseSPslotPtr(frameEBP, info);
     PTR_TADDR pSlot;
     for(lvl = 0, pSlot = pFirstBaseSPslot;
         *pSlot && lvl < unwindLevel;
@@ -653,6 +670,15 @@ inline size_t GetSizeOfFrameHeaderForEnC(hdrInfo * info)
 {
     WRAPPER_NO_CONTRACT;
 
+#ifdef FEATURE_EH_FUNCLETS
+    _ASSERTE(info->ebpFrame);
+    unsigned position = info->savedRegsCountExclFP +
+                        info->localloc +
+                        info->genericsContext + // For CORINFO_GENERICS_CTXT_FROM_PARAMTYPEARG
+                        ((info->syncStartOffset != INVALID_SYNC_OFFSET) ? 1 : 0) // Is this method synchronized
+                        + 1; // for ebpFrame
+    return position * sizeof(TADDR);
+#else
     // See comment above Compiler::lvaAssignFrameOffsets() in src\jit\il\lclVars.cpp
     // for frame layout
 
@@ -664,6 +690,7 @@ inline size_t GetSizeOfFrameHeaderForEnC(hdrInfo * info)
     // to get the total size of the header.
     return sizeof(TADDR) +
             GetEndShadowSPSlotsOffset(info, MAX_EnC_HANDLER_NESTING_LEVEL);
+#endif // FEATURE_EH_FUNCLETS
 }
 #endif // FEATURE_NATIVEAOT