Delete security object from GCInfo encoder/decoder (#76487)
authorJan Kotas <jkotas@microsoft.com>
Sun, 2 Oct 2022 23:57:04 +0000 (16:57 -0700)
committerGitHub <noreply@github.com>
Sun, 2 Oct 2022 23:57:04 +0000 (16:57 -0700)
Fixes #76482

src/coreclr/gcdump/gcdumpnonx86.cpp
src/coreclr/gcinfo/gcinfoencoder.cpp
src/coreclr/inc/eetwain.h
src/coreclr/inc/gcinfodecoder.h
src/coreclr/inc/gcinfoencoder.h
src/coreclr/inc/gcinfotypes.h
src/coreclr/jit/gcencode.cpp
src/coreclr/vm/eetwain.cpp
src/coreclr/vm/gcinfodecoder.cpp
src/coreclr/vm/stackwalktypes.h

index d9790a1..f93e9bd 100644 (file)
@@ -293,8 +293,7 @@ size_t      GCDump::DumpGCTable(PTR_CBYTE      gcInfoBlock,
                                                  ),
                              0);
 
-    if (NO_SECURITY_OBJECT != hdrdecoder.GetSecurityObjectStackSlot() ||
-        NO_GENERICS_INST_CONTEXT != hdrdecoder.GetGenericsInstContextStackSlot() ||
+    if (NO_GENERICS_INST_CONTEXT != hdrdecoder.GetGenericsInstContextStackSlot() ||
         NO_GS_COOKIE == hdrdecoder.GetGSCookieStackSlot())
     {
         gcPrintf("Prolog size: ");
@@ -302,25 +301,6 @@ size_t      GCDump::DumpGCTable(PTR_CBYTE      gcInfoBlock,
         gcPrintf("%d\n", prologSize);
     }
 
-    gcPrintf("Security object: ");
-    if (NO_SECURITY_OBJECT == hdrdecoder.GetSecurityObjectStackSlot())
-    {
-        gcPrintf("<none>\n");
-    }
-    else
-    {
-        INT32 ofs = hdrdecoder.GetSecurityObjectStackSlot();
-        char sign = '+';
-
-        if (ofs < 0)
-        {
-            sign = '-';
-            ofs = -ofs;
-        }
-
-        gcPrintf("caller.sp%c%x\n", sign, ofs);
-    }
-
     gcPrintf("GS cookie: ");
     if (NO_GS_COOKIE == hdrdecoder.GetGSCookieStackSlot())
     {
index 8d01159..d459633 100644 (file)
@@ -473,7 +473,6 @@ GcInfoEncoder::GcInfoEncoder(
     m_NumCallSites = 0;
 #endif
 
-    m_SecurityObjectStackSlot = NO_SECURITY_OBJECT;
     m_GSCookieStackSlot = NO_GS_COOKIE;
     m_GSCookieValidRangeStart = 0;
     _ASSERTE(sizeof(m_GSCookieValidRangeEnd) == sizeof(UINT32));
@@ -694,18 +693,6 @@ void GcInfoEncoder::SetCodeLength( UINT32 length )
     m_CodeLength = length;
 }
 
-
-void GcInfoEncoder::SetSecurityObjectStackSlot( INT32 spOffset )
-{
-    _ASSERTE( spOffset != NO_SECURITY_OBJECT );
-#if defined(TARGET_AMD64)
-    _ASSERTE( spOffset < 0x10 && "The security object cannot reside in an input variable!" );
-#endif
-    _ASSERTE( m_SecurityObjectStackSlot == NO_SECURITY_OBJECT || m_SecurityObjectStackSlot == spOffset );
-
-    m_SecurityObjectStackSlot  = spOffset;
-}
-
 void GcInfoEncoder::SetPrologSize( UINT32 prologSize )
 {
     _ASSERTE(prologSize != 0);
@@ -1019,12 +1006,11 @@ void GcInfoEncoder::Build()
     ///////////////////////////////////////////////////////////////////////
 
 
-    UINT32 hasSecurityObject = (m_SecurityObjectStackSlot != NO_SECURITY_OBJECT);
     UINT32 hasGSCookie = (m_GSCookieStackSlot != NO_GS_COOKIE);
     UINT32 hasContextParamType = (m_GenericsInstContextStackSlot != NO_GENERICS_INST_CONTEXT);
     UINT32 hasReversePInvokeFrame = (m_ReversePInvokeFrameSlot != NO_REVERSE_PINVOKE_FRAME);
 
-    BOOL slimHeader = (!m_IsVarArg && !hasSecurityObject && !hasGSCookie && (m_PSPSymStackSlot == NO_PSP_SYM) &&
+    BOOL slimHeader = (!m_IsVarArg && !hasGSCookie && (m_PSPSymStackSlot == NO_PSP_SYM) &&
         !hasContextParamType && (m_InterruptibleRanges.Count() == 0) && !hasReversePInvokeFrame &&
         ((m_StackBaseRegister == NO_STACK_BASE_REGISTER) || (NORMALIZE_STACK_BASE_REGISTER(m_StackBaseRegister) == 0))) &&
         (m_SizeOfEditAndContinuePreservedArea == NO_SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA) &&
@@ -1052,7 +1038,7 @@ void GcInfoEncoder::Build()
     {
         GCINFO_WRITE(m_Info1, 1, 1, FlagsSize); // Fat encoding
         GCINFO_WRITE(m_Info1, (m_IsVarArg ? 1 : 0), 1, FlagsSize);
-        GCINFO_WRITE(m_Info1, (hasSecurityObject ? 1 : 0), 1, FlagsSize);
+        GCINFO_WRITE(m_Info1, 0 /* unused - was hasSecurityObject */, 1, FlagsSize);
         GCINFO_WRITE(m_Info1, (hasGSCookie ? 1 : 0), 1, FlagsSize);
         GCINFO_WRITE(m_Info1, ((m_PSPSymStackSlot != NO_PSP_SYM) ? 1 : 0), 1, FlagsSize);
         GCINFO_WRITE(m_Info1, m_contextParamType, 2, FlagsSize);
@@ -1095,7 +1081,7 @@ void GcInfoEncoder::Build()
         GCINFO_WRITE_VARL_U(m_Info1, normPrologSize-1, NORM_PROLOG_SIZE_ENCBASE, ProEpilogSize);
         GCINFO_WRITE_VARL_U(m_Info1, normEpilogSize, NORM_EPILOG_SIZE_ENCBASE, ProEpilogSize);
     }
-    else if (hasSecurityObject || hasContextParamType)
+    else if (hasContextParamType)
     {
         _ASSERTE(!slimHeader);
         // Save the prolog size, to be used for determining when it is not safe
@@ -1107,19 +1093,6 @@ void GcInfoEncoder::Build()
         GCINFO_WRITE_VARL_U(m_Info1, normPrologSize-1, NORM_PROLOG_SIZE_ENCBASE, ProEpilogSize);
     }
 
-    // Encode the offset to the security object.
-    if(hasSecurityObject)
-    {
-        _ASSERTE(!slimHeader);
-#ifdef _DEBUG
-        LOG((LF_GCINFO, LL_INFO1000, "Security object at " FMT_STK "\n",
-             DBG_STK(m_SecurityObjectStackSlot)
-             ));
-#endif
-
-        GCINFO_WRITE_VARL_S(m_Info1, NORMALIZE_STACK_SLOT(m_SecurityObjectStackSlot), SECURITY_OBJECT_STACK_SLOT_ENCBASE, SecObjSize);
-    }
-
     // Encode the offset to the GS cookie.
     if(hasGSCookie)
     {
index aba7a14..9d75c2f 100644 (file)
@@ -701,7 +701,6 @@ struct hdrInfo
     bool                doubleAlign;    // is the stack double-aligned? locals addressed relative to ESP, and arguments relative to EBP
     bool                interruptible;  // intr. at all times (excluding prolog/epilog), not just call sites
 
-    bool                securityCheck;  // has a slot for security object
     bool                handlers;       // has callable handlers
     bool                localloc;       // uses localloc
     bool                editNcontinue;  // has been compiled in EnC mode
index 3a7d310..93bdd33 100644 (file)
@@ -224,7 +224,7 @@ enum GcInfoDecoderFlags
 enum GcInfoHeaderFlags
 {
     GC_INFO_IS_VARARG                   = 0x1,
-    GC_INFO_HAS_SECURITY_OBJECT         = 0x2,
+    // unused                           = 0x2, // was GC_INFO_HAS_SECURITY_OBJECT
     GC_INFO_HAS_GS_COOKIE               = 0x4,
     GC_INFO_HAS_PSP_SYM                 = 0x8,
     GC_INFO_HAS_GENERICS_INST_CONTEXT_MASK   = 0x30,
@@ -528,7 +528,6 @@ public:
     // Miscellaneous method information
     //------------------------------------------------------------------------
 
-    INT32   GetSecurityObjectStackSlot();
     INT32   GetGSCookieStackSlot();
     UINT32  GetGSCookieValidRangeStart();
     UINT32  GetGSCookieValidRangeEnd();
@@ -571,7 +570,6 @@ private:
 #elif defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64)
     bool    m_HasTailCalls;
 #endif // TARGET_AMD64
-    INT32   m_SecurityObjectStackSlot;
     INT32   m_GSCookieStackSlot;
     INT32   m_ReversePInvokeFrameStackSlot;
     UINT32  m_ValidRangeStart;
index dd438a7..33c1054 100644 (file)
@@ -21,7 +21,7 @@
  Fat Header for other cases:
     - EncodingType[Fat]
     - Flag:     isVarArg,
-                hasSecurityObject,
+                unused (was hasSecurityObject),
                 hasGSCookie,
                 hasPSPSymStackSlot,
                 hasGenericsInstContextStackSlot,
@@ -32,7 +32,7 @@
                 hasReversePInvokeFrame,
     - ReturnKind (Fat: 4 bits)
     - CodeLength
-    - Prolog (if hasSecurityObject || hasGenericsInstContextStackSlot || hasGSCookie)
+    - Prolog (if hasGenericsInstContextStackSlot || hasGSCookie)
     - Epilog (if hasGSCookie)
     - SecurityObjectStackSlot (if any)
     - GSCookieStackSlot (if any)
@@ -420,7 +420,6 @@ public:
     // Miscellaneous method information
     //------------------------------------------------------------------------
 
-    void SetSecurityObjectStackSlot( INT32 spOffset );
     void SetPrologSize( UINT32 prologSize );
     void SetGSCookieStackSlot( INT32 spOffsetGSCookie, UINT32 validRangeStart, UINT32 validRangeEnd );
     void SetPSPSymStackSlot( INT32 spOffsetPSPSym );
@@ -502,7 +501,6 @@ private:
 #elif defined(TARGET_ARM) || defined(TARGET_ARM64) || defined(TARGET_LOONGARCH64)
     bool   m_HasTailCalls;
 #endif // TARGET_AMD64
-    INT32  m_SecurityObjectStackSlot;
     INT32  m_GSCookieStackSlot;
     UINT32 m_GSCookieValidRangeStart;
     UINT32 m_GSCookieValidRangeEnd;
index 454babf..57cae4d 100644 (file)
@@ -596,7 +596,6 @@ void FASTCALL decodeCallPattern(int         pattern,
 
 // Stack offsets must be 8-byte aligned, so we use this unaligned
 //  offset to represent that the method doesn't have a security object
-#define NO_SECURITY_OBJECT        (-1)
 #define NO_GS_COOKIE              (-1)
 #define NO_STACK_BASE_REGISTER    (0xffffffff)
 #define NO_SIZE_OF_EDIT_AND_CONTINUE_PRESERVED_AREA (0xffffffff)
index e85066b..99ddf4f 100644 (file)
@@ -3793,15 +3793,6 @@ public:
         }
     }
 
-    void SetSecurityObjectStackSlot(INT32 spOffset)
-    {
-        m_gcInfoEncoder->SetSecurityObjectStackSlot(spOffset);
-        if (m_doLogging)
-        {
-            printf("Set security object stack slot to %d.\n", spOffset);
-        }
-    }
-
     void SetIsVarArg()
     {
         m_gcInfoEncoder->SetIsVarArg();
index a45336c..a6eb212 100644 (file)
@@ -252,7 +252,6 @@ size_t DecodeGCHdrInfo(GCInfoToken gcInfoToken,
     infoPtr->revPInvokeOffset = header.revPInvokeOffset;
 
     infoPtr->doubleAlign     = header.doubleAlign;
-    infoPtr->securityCheck   = header.security;
     infoPtr->handlers        = header.handlers;
     infoPtr->localloc        = header.localloc;
     infoPtr->editNcontinue   = header.editNcontinue;
@@ -378,19 +377,6 @@ size_t DecodeGCHdrInfo(GCInfoToken gcInfoToken,
 // We do a "pop eax; jmp eax" to return from a fault or finally handler
 const size_t END_FIN_POP_STACK = sizeof(TADDR);
 
-
-// The offset (in bytes) from EBP for the secutiy object on the stack
-inline size_t GetSecurityObjectOffset(hdrInfo * info)
-{
-    LIMITED_METHOD_DAC_CONTRACT;
-
-    _ASSERTE(info->securityCheck && info->ebpFrame);
-
-    unsigned position = info->savedRegsCountExclFP +
-                        1;
-    return position * sizeof(TADDR);
-}
-
 inline
 size_t GetLocallocSPOffset(hdrInfo * info)
 {
@@ -399,7 +385,6 @@ size_t GetLocallocSPOffset(hdrInfo * info)
     _ASSERTE(info->localloc && info->ebpFrame);
 
     unsigned position = info->savedRegsCountExclFP +
-                        info->securityCheck +
                         1;
     return position * sizeof(TADDR);
 }
@@ -412,7 +397,6 @@ size_t GetParamTypeArgOffset(hdrInfo * info)
     _ASSERTE((info->genericsContext || info->handlers) && info->ebpFrame);
 
     unsigned position = info->savedRegsCountExclFP +
-                        info->securityCheck +
                         info->localloc +
                         1;  // For CORINFO_GENERICS_CTXT_FROM_PARAMTYPEARG
     return position * sizeof(TADDR);
@@ -906,9 +890,6 @@ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
     /* @TODO: Check if we have grown out of space for locals, in the face of localloc */
     _ASSERTE(!oldInfo.localloc && !newInfo.localloc);
 
-    // Always reserve space for the securityCheck slot
-    _ASSERTE(oldInfo.securityCheck && newInfo.securityCheck);
-
     // @TODO: If nesting level grows above the MAX_EnC_HANDLER_NESTING_LEVEL,
     // we should return EnC_NESTED_HANLDERS
     _ASSERTE(oldInfo.handlers && newInfo.handlers);
@@ -1046,14 +1027,6 @@ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
 
     TADDR callerSP = oldStackBase + oldFixedStackSize;
 
-    // If the old code saved a security object, store the object's reference now.
-    OBJECTREF securityObject = NULL;
-    INT32 nOldSecurityObjectStackSlot = oldGcDecoder.GetSecurityObjectStackSlot();
-    if (nOldSecurityObjectStackSlot != NO_SECURITY_OBJECT)
-    {
-        securityObject = ObjectToOBJECTREF(*PTR_PTR_Object(callerSP + nOldSecurityObjectStackSlot));
-    }
-
 #ifdef _DEBUG
     // If the old method has a PSPSym, then its value should == FP for x64 and callerSP for arm64
     INT32 nOldPspSymStackSlot = oldGcDecoder.GetPSPSymStackSlot();
@@ -1434,19 +1407,6 @@ HRESULT EECodeManager::FixContextForEnC(PCONTEXT         pCtx,
 #elif defined(TARGET_AMD64) || defined(TARGET_ARM64)
         memset((void*)newStackBase, 0, newFixedStackSize - frameHeaderSize);
 
-        // On AMD64/ARM64, after zeroing out the stack, restore the security object and PSPSym...
-
-        // There is no relationship we can guarantee between the old code having a security
-        // object and the new code having a security object.  If the new code does have a
-        // security object, then we copy over the old security object's reference if there
-        // was one (else we copy over NULL, which is fine).  If the new code doesn't have a
-        // security object, we do nothing.
-        INT32 nNewSecurityObjectStackSlot = newGcDecoder.GetSecurityObjectStackSlot();
-        if (nNewSecurityObjectStackSlot != NO_SECURITY_OBJECT)
-        {
-            *PTR_PTR_Object(callerSP + nNewSecurityObjectStackSlot) = OBJECTREFToObject(securityObject);
-        }
-
         // Restore PSPSym for the new function. Its value should be set to our new FP. But
         // first, we gotta find PSPSym's location on the stack
         INT32 nNewPspSymStackSlot = newGcDecoder.GetPSPSymStackSlot();
@@ -4263,15 +4223,6 @@ bool UnwindStackFrame(PREGDISPLAY     pContext,
 
     if (pUnwindInfo != NULL)
     {
-        pUnwindInfo->securityObjectOffset = 0;
-        if (info->securityCheck)
-        {
-            _ASSERTE(info->ebpFrame);
-            SIZE_T securityObjectOffset = (GetSecurityObjectOffset(info) / sizeof(void*));
-            _ASSERTE(securityObjectOffset != 0);
-            pUnwindInfo->securityObjectOffset = DWORD(securityObjectOffset);
-        }
-
         pUnwindInfo->fUseEbpAsFrameReg = info->ebpFrame;
         pUnwindInfo->fUseEbp = ((info->savedRegMask & RM_EBP) != 0);
     }
index f69513e..67fbb3b 100644 (file)
@@ -125,7 +125,6 @@ GcInfoDecoder::GcInfoDecoder(
     }
 
     m_IsVarArg                 = headerFlags & GC_INFO_IS_VARARG;
-    int hasSecurityObject      = headerFlags & GC_INFO_HAS_SECURITY_OBJECT;
     int hasGSCookie            = headerFlags & GC_INFO_HAS_GS_COOKIE;
     int hasPSPSym              = headerFlags & GC_INFO_HAS_PSP_SYM;
     int hasGenericsInstContext = (headerFlags & GC_INFO_HAS_GENERICS_INST_CONTEXT_MASK) != GC_INFO_HAS_GENERICS_INST_CONTEXT_NONE;
@@ -177,7 +176,7 @@ GcInfoDecoder::GcInfoDecoder(
         m_ValidRangeEnd = (UINT32) DENORMALIZE_CODE_OFFSET(normCodeLength - normEpilogSize);
         _ASSERTE(m_ValidRangeStart < m_ValidRangeEnd);
     }
-    else if (hasSecurityObject || hasGenericsInstContext)
+    else if (hasGenericsInstContext)
     {
         // Decode prolog information
         UINT32 normPrologSize = (UINT32) m_Reader.DecodeVarLengthUnsigned(NORM_PROLOG_SIZE_ENCBASE) + 1;
@@ -197,23 +196,6 @@ GcInfoDecoder::GcInfoDecoder(
         return;
     }
 
-    // Decode the offset to the security object.
-    if(hasSecurityObject)
-    {
-        m_SecurityObjectStackSlot = (INT32) DENORMALIZE_STACK_SLOT(m_Reader.DecodeVarLengthSigned(SECURITY_OBJECT_STACK_SLOT_ENCBASE));
-    }
-    else
-    {
-        m_SecurityObjectStackSlot = NO_SECURITY_OBJECT;
-    }
-
-    remainingFlags &= ~DECODE_SECURITY_OBJECT;
-    if (remainingFlags == 0)
-    {
-        // Bail, if we've decoded enough,
-        return;
-    }
-
     // Decode the offset to the GS cookie.
     if(hasGSCookie)
     {
@@ -506,12 +488,6 @@ void GcInfoDecoder::EnumerateInterruptibleRanges (
     }
 }
 
-INT32 GcInfoDecoder::GetSecurityObjectStackSlot()
-{
-    _ASSERTE( m_Flags & DECODE_SECURITY_OBJECT );
-    return m_SecurityObjectStackSlot;
-}
-
 INT32 GcInfoDecoder::GetGSCookieStackSlot()
 {
     _ASSERTE( m_Flags & DECODE_GS_COOKIE );
index 640d5f3..601e001 100644 (file)
@@ -86,7 +86,6 @@ struct StackwalkCacheUnwindInfo
     ULONG RBPOffset;
     ULONG RSPOffsetFromUnwindInfo;
 #else  // !TARGET_AMD64
-    size_t securityObjectOffset;    // offset of SecurityObject. 0 if there is no security object
     BOOL fUseEbp;                   // Is EBP modified by the method - either for a frame-pointer or for a scratch-register?
     BOOL fUseEbpAsFrameReg;         // use EBP as the frame pointer?
 #endif // !TARGET_AMD64
@@ -114,10 +113,9 @@ StackwalkCacheEntry
     UINT_PTR IP;
 #if !defined(TARGET_AMD64)
     WORD ESPOffset:15;          // stack offset (frame size + pending arguments + etc)
-    WORD securityObjectOffset:3;// offset of SecurityObject. 0 if there is no security object
     WORD fUseEbp:1;             // For ESP methods, is EBP touched at all?
     WORD fUseEbpAsFrameReg:1;   // use EBP as the frame register?
-    WORD argSize:11;            // size of args pushed on stack
+    WORD argSize:15;            // size of args pushed on stack
 #else  // TARGET_AMD64
     DWORD RSPOffset;
     DWORD RBPOffset;
@@ -136,9 +134,6 @@ StackwalkCacheEntry
         this->ESPOffset         = SPOffset;
         this->argSize           = argSize;
 
-        this->securityObjectOffset = (WORD)pUnwindInfo->securityObjectOffset;
-        _ASSERTE(this->securityObjectOffset == pUnwindInfo->securityObjectOffset);
-
         this->fUseEbp           = pUnwindInfo->fUseEbp;
         this->fUseEbpAsFrameReg = pUnwindInfo->fUseEbpAsFrameReg;
         _ASSERTE(!fUseEbpAsFrameReg || fUseEbp);
@@ -162,20 +157,6 @@ StackwalkCacheEntry
 #endif // !TARGET_X86 && !TARGET_AMD64
     }
 
-    inline BOOL HasSecurityObject()
-    {
-        LIMITED_METHOD_CONTRACT;
-
-#if defined(TARGET_X86)
-        return securityObjectOffset != 0;
-#else  // !TARGET_X86
-        // On AMD64 we don't save anything by grabbing the security object before it is needed.  This is because
-        // we need to crack the GC info in order to find the security object, and to unwind we only need to
-        // crack the unwind info.
-        return FALSE;
-#endif // !TARGET_X86
-    }
-
     inline BOOL IsSafeToUseCache()
     {
         LIMITED_METHOD_CONTRACT;
@@ -236,7 +217,6 @@ inline StackwalkCacheUnwindInfo::StackwalkCacheUnwindInfo(StackwalkCacheEntry *
 #if defined(TARGET_AMD64)
     RBPOffset = pCacheEntry->RBPOffset;
 #else  // !TARGET_AMD64
-    securityObjectOffset = pCacheEntry->securityObjectOffset;
     fUseEbp = pCacheEntry->fUseEbp;
     fUseEbpAsFrameReg = pCacheEntry->fUseEbpAsFrameReg;
 #endif // !TARGET_AMD64