Skip to content

Commit

Permalink
Delete security object from GCInfo encoder/decoder (#76487)
Browse files Browse the repository at this point in the history
Fixes #76482
  • Loading branch information
jkotas authored Oct 2, 2022
1 parent 144a33a commit ae4dc9c
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 164 deletions.
22 changes: 1 addition & 21 deletions src/coreclr/gcdump/gcdumpnonx86.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,34 +293,14 @@ 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: ");
UINT32 prologSize = hdrdecoder.GetPrologSize();
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())
{
Expand Down
33 changes: 3 additions & 30 deletions src/coreclr/gcinfo/gcinfoencoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) &&
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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)
{
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/inc/eetwain.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/inc/gcinfodecoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -528,7 +528,6 @@ class GcInfoDecoder
// Miscellaneous method information
//------------------------------------------------------------------------

INT32 GetSecurityObjectStackSlot();
INT32 GetGSCookieStackSlot();
UINT32 GetGSCookieValidRangeStart();
UINT32 GetGSCookieValidRangeEnd();
Expand Down Expand Up @@ -571,7 +570,6 @@ class GcInfoDecoder
#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;
Expand Down
6 changes: 2 additions & 4 deletions src/coreclr/inc/gcinfoencoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
Fat Header for other cases:
- EncodingType[Fat]
- Flag: isVarArg,
hasSecurityObject,
unused (was hasSecurityObject),
hasGSCookie,
hasPSPSymStackSlot,
hasGenericsInstContextStackSlot,
Expand All @@ -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)
Expand Down Expand Up @@ -420,7 +420,6 @@ class GcInfoEncoder
// Miscellaneous method information
//------------------------------------------------------------------------

void SetSecurityObjectStackSlot( INT32 spOffset );
void SetPrologSize( UINT32 prologSize );
void SetGSCookieStackSlot( INT32 spOffsetGSCookie, UINT32 validRangeStart, UINT32 validRangeEnd );
void SetPSPSymStackSlot( INT32 spOffsetPSPSym );
Expand Down Expand Up @@ -502,7 +501,6 @@ class GcInfoEncoder
#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;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/inc/gcinfotypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
9 changes: 0 additions & 9 deletions src/coreclr/jit/gcencode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3793,15 +3793,6 @@ class GcInfoEncoderWithLogging
}
}

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();
Expand Down
49 changes: 0 additions & 49 deletions src/coreclr/vm/eetwain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down
26 changes: 1 addition & 25 deletions src/coreclr/vm/gcinfodecoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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)
{
Expand Down Expand Up @@ -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 );
Expand Down
Loading

0 comments on commit ae4dc9c

Please sign in to comment.