Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove ThreadNative_GetProcessDefaultStackSize #102003

Merged
merged 2 commits into from
May 8, 2024
Merged

Remove ThreadNative_GetProcessDefaultStackSize #102003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ef89a1
Remove ThreadNative_GetProcessDefaultStackSize
elinor-fung May 7, 2024
21fa147
Remove PEDecoder::CheckWillCreateGuardPage
elinor-fung May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/coreclr/inc/pedecoder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,6 @@ class PEDecoder
BOOL HasReadyToRunHeader() const;
READYTORUN_HEADER *GetReadyToRunHeader() const;

void GetEXEStackSizes(SIZE_T *PE_SizeOfStackReserve, SIZE_T *PE_SizeOfStackCommit) const;

CHECK CheckWillCreateGuardPage() const;

// Native DLLMain Entrypoint
BOOL HasNativeEntryPoint() const;
void *GetNativeEntryPoint() const;
Expand Down
36 changes: 0 additions & 36 deletions src/coreclr/inc/pedecoder.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,42 +411,6 @@ inline WORD PEDecoder::GetCharacteristics() const
return VAL16(FindNTHeaders()->FileHeader.Characteristics);
}

inline SIZE_T PEDecoder::GetSizeOfStackReserve() const
{
CONTRACTL
{
INSTANCE_CHECK;
PRECONDITION(CheckNTHeaders());
NOTHROW;
GC_NOTRIGGER;
}
CONTRACTL_END;

if (Has32BitNTHeaders())
return (SIZE_T) VAL32(GetNTHeaders32()->OptionalHeader.SizeOfStackReserve);
else
return (SIZE_T) VAL64(GetNTHeaders64()->OptionalHeader.SizeOfStackReserve);
}


inline SIZE_T PEDecoder::GetSizeOfStackCommit() const
{
CONTRACTL
{
INSTANCE_CHECK;
PRECONDITION(CheckNTHeaders());
NOTHROW;
GC_NOTRIGGER;
}
CONTRACTL_END;

if (Has32BitNTHeaders())
return (SIZE_T) VAL32(GetNTHeaders32()->OptionalHeader.SizeOfStackCommit);
else
return (SIZE_T) VAL64(GetNTHeaders64()->OptionalHeader.SizeOfStackCommit);
}


inline SIZE_T PEDecoder::GetSizeOfHeapReserve() const
{
CONTRACTL
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/inc/utilcode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3718,8 +3718,6 @@ namespace util

INDEBUG(BOOL DbgIsExecutable(LPVOID lpMem, SIZE_T length);)

BOOL ThreadWillCreateGuardPage(SIZE_T sizeReservedStack, SIZE_T sizeCommittedStack);

#ifdef FEATURE_COMINTEROP
FORCEINLINE void HolderSysFreeString(BSTR str) { CONTRACT_VIOLATION(ThrowsViolation); SysFreeString(str); }

Expand Down
40 changes: 0 additions & 40 deletions src/coreclr/utilcode/pedecoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ CHECK PEDecoder::CheckFormat() const

if (IsILOnly())
CHECK(CheckILOnly());

CHECK(CheckWillCreateGuardPage());
}
}

Expand Down Expand Up @@ -2435,44 +2433,6 @@ PTR_CVOID PEDecoder::GetNativeManifestMetadata(COUNT_T *pSize) const
RETURN dac_cast<PTR_VOID>(GetDirectoryData(pDir));
}

// Get the SizeOfStackReserve and SizeOfStackCommit from the PE file that was used to create
// the calling process (.exe file).
void PEDecoder::GetEXEStackSizes(SIZE_T *PE_SizeOfStackReserve, SIZE_T *PE_SizeOfStackCommit) const
{
CONTRACTL {
PRECONDITION(!IsDll()); // This routine should only be called for EXE files.
NOTHROW;
GC_NOTRIGGER;
} CONTRACTL_END;

* PE_SizeOfStackReserve = GetSizeOfStackReserve();
* PE_SizeOfStackCommit = GetSizeOfStackCommit();
}

CHECK PEDecoder::CheckWillCreateGuardPage() const
{
CONTRACT_CHECK
{
PRECONDITION(CheckNTHeaders());
NOTHROW;
GC_NOTRIGGER;
}
CONTRACT_CHECK_END;

if (!IsDll())
{
SIZE_T sizeReservedStack = 0;
SIZE_T sizeCommittedStack = 0;

GetEXEStackSizes(&sizeReservedStack, &sizeCommittedStack);

CHECK(ThreadWillCreateGuardPage(sizeReservedStack, sizeCommittedStack));

}

CHECK_OK;
}

BOOL PEDecoder::HasNativeEntryPoint() const
{
CONTRACTL {
Expand Down
42 changes: 0 additions & 42 deletions src/coreclr/utilcode/util_nodependencies.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,45 +730,3 @@ void OutputDebugStringUtf8(LPCUTF8 utf8DebugMsg)
OutputDebugStringW(wideDebugMsg);
#endif // !TARGET_UNIX
}

BOOL ThreadWillCreateGuardPage(SIZE_T sizeReservedStack, SIZE_T sizeCommittedStack)
{
// We need to make sure there will be a reserved but never committed page at the end
// of the stack. We do here the check NT does when it creates the user stack to decide
// if there is going to be a guard page. However, that is not enough, as if we only
// have a guard page, we have nothing to protect us from going pass it. Well, in
// fact, there is something that we will protect you, there are certain places
// (RTLUnwind) in NT that will check that the current frame is within stack limits.
// If we are not it will bomb out. We will also bomb out if we touch the hard guard
// page.
//
// For situation B, teb->StackLimit is at the beginning of the user stack (ie
// before updating StackLimit it checks if it was able to create a new guard page,
// in this case, it can't), which makes the check fail in RtlUnwind.
//
// Situation A [ Hard guard page | Guard page | user stack]
//
// Situation B [ Guard page | User stack ]
//
// Situation C [ User stack ( no room for guard page) ]
//
// Situation D (W9x) : Guard page or not, w9x has a 64k reserved region below
// the stack, we don't need any checks at all
//
// We really want to be in situation A all the time, so we add one more page
// to our requirements (we require guard page + hard guard)

SYSTEM_INFO sysInfo;
::GetSystemInfo(&sysInfo);

// OS rounds up sizes the following way to decide if it marks a guard page
sizeReservedStack = ALIGN(sizeReservedStack, ((size_t)sysInfo.dwAllocationGranularity)); // Allocation granularity
sizeCommittedStack = ALIGN(sizeCommittedStack, ((size_t)sysInfo.dwPageSize)); // Page Size

// OS wont create guard page, we can't execute managed code safely.
// We also have to make sure we have a 'hard' guard, thus we add another
// page to the memory we would need comitted.
// That is, the following code will check if sizeReservedStack is at least 2 pages
// more than sizeCommittedStack.
return (sizeReservedStack > sizeCommittedStack + ((size_t)sysInfo.dwPageSize));
} // ThreadWillCreateGuardPage
19 changes: 0 additions & 19 deletions src/coreclr/vm/comsynchronizable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -913,25 +913,6 @@ void ThreadNative::InformThreadNameChange(Thread* pThread, LPCWSTR name, INT32 l
#endif // DEBUGGING_SUPPORTED
}

extern "C" UINT64 QCALLTYPE ThreadNative_GetProcessDefaultStackSize()
{
QCALL_CONTRACT;

SIZE_T reserve = 0;
SIZE_T commit = 0;

BEGIN_QCALL;

if (!Thread::GetProcessDefaultStackSize(&reserve, &commit))
reserve = 1024 * 1024;

END_QCALL;

return (UINT64)reserve;
}



FCIMPL1(FC_BOOL_RET, ThreadNative::IsThreadpoolThread, ThreadBaseObject* thread)
{
FCALL_CONTRACT;
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/comsynchronizable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ friend class ThreadBaseObject;
extern "C" void QCALLTYPE ThreadNative_Start(QCall::ThreadHandle thread, int threadStackSize, int priority, PCWSTR pThreadName);
extern "C" void QCALLTYPE ThreadNative_SetIsBackground(QCall::ThreadHandle thread, BOOL value);
extern "C" void QCALLTYPE ThreadNative_InformThreadNameChange(QCall::ThreadHandle thread, LPCWSTR name, INT32 len);
extern "C" UINT64 QCALLTYPE ThreadNative_GetProcessDefaultStackSize();
extern "C" BOOL QCALLTYPE ThreadNative_YieldThread();
extern "C" UINT64 QCALLTYPE ThreadNative_GetCurrentOSThreadId();
extern "C" void QCALLTYPE ThreadNative_Abort(QCall::ThreadHandle thread);
Expand Down
60 changes: 0 additions & 60 deletions src/coreclr/vm/threads.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2179,66 +2179,6 @@ SIZE_T GetDefaultStackSizeSetting()
return (SIZE_T) value;
}

BOOL Thread::GetProcessDefaultStackSize(SIZE_T* reserveSize, SIZE_T* commitSize)
{
CONTRACTL
{
NOTHROW;
GC_NOTRIGGER;
}
CONTRACTL_END;

//
// Let's get the stack sizes from the PE file that started process.
//
static SIZE_T ExeSizeOfStackReserve = 0;
static SIZE_T ExeSizeOfStackCommit = 0;

static BOOL fSizesGot = FALSE;

if (!fSizesGot)
{
SIZE_T defaultStackSizeSetting = GetDefaultStackSizeSetting();

if (defaultStackSizeSetting != 0)
{
ExeSizeOfStackReserve = defaultStackSizeSetting;
ExeSizeOfStackCommit = defaultStackSizeSetting;
fSizesGot = TRUE;
}
}

#ifndef TARGET_UNIX
if (!fSizesGot)
{
HINSTANCE hInst = WszGetModuleHandle(NULL);
_ASSERTE(hInst); // WszGetModuleHandle should never fail on the module that started the process.
EX_TRY
{
PEDecoder pe(hInst);
pe.GetEXEStackSizes(&ExeSizeOfStackReserve, &ExeSizeOfStackCommit);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can delete PEDecoder::GetEXEStackSize and PEDecoder::CheckWillCreateGuardPage. CheckWillCreateGuardPage made sense in .NET Framework when the .exe was used to launch the process, it does not make sense in .NET Core.

fSizesGot = TRUE;
}
EX_CATCH
{
fSizesGot = FALSE;
}
EX_END_CATCH(SwallowAllExceptions);
}
#endif // !TARGET_UNIX

if (!fSizesGot) {
//return some somewhat-reasonable numbers
if (NULL != reserveSize) *reserveSize = 256*1024;
if (NULL != commitSize) *commitSize = 256*1024;
return FALSE;
}

if (NULL != reserveSize) *reserveSize = ExeSizeOfStackReserve;
if (NULL != commitSize) *commitSize = ExeSizeOfStackCommit;
return TRUE;
}

BOOL Thread::CreateNewOSThread(SIZE_T sizeToCommitOrReserve, LPTHREAD_START_ROUTINE start, void *args)
{
CONTRACTL {
Expand Down
11 changes: 1 addition & 10 deletions src/coreclr/vm/threads.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2867,7 +2867,6 @@ class Thread

DWORD m_Priority; // initialized to INVALID_THREAD_PRIORITY, set to actual priority when a
// thread does a busy wait for GC, reset to INVALID_THREAD_PRIORITY after wait is over
friend class NDirect; // Quick access to thread stub creation

#ifdef HAVE_GCCOVER
friend void DoGcStress (PT_CONTEXT regs, NativeCodeVersion nativeCodeVersion); // Needs to call UnhijackThread
Expand Down Expand Up @@ -3663,14 +3662,6 @@ class Thread
#endif // defined(GCCOVER_TOLERATE_SPURIOUS_AV)
#endif // HAVE_GCCOVER

public:
static BOOL CheckThreadStackSize(SIZE_T *SizeToCommitOrReserve,
BOOL isSizeToReserve // When TRUE, the previous argument is the stack size to reserve.
// Otherwise, it is the size to commit.
);

static BOOL GetProcessDefaultStackSize(SIZE_T* reserveSize, SIZE_T* commitSize);

private:

// Although this is a pointer, it is used as a flag to indicate the current context is unsafe
Expand Down Expand Up @@ -4067,7 +4058,7 @@ struct cdac_offsets<Thread>
static constexpr size_t ExposedObject = offsetof(Thread, m_ExposedObject);
static constexpr size_t Link = offsetof(Thread, m_Link);
};

// End of class Thread

typedef Thread::ForbidSuspendThreadHolder ForbidSuspendThreadHolder;
Expand Down
Loading