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.

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

Use alternatives to redirecting threads via IP changes in the context, for a few cases on Windows #55649

Merged
merged 6 commits into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
17 changes: 16 additions & 1 deletion eng/native/configurecompiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ add_compile_definitions("$<$<CONFIG:CHECKED>:DEBUG;_DEBUG;_DBG;URTBLDENV_FRIENDL
add_compile_definitions("$<$<OR:$<CONFIG:RELEASE>,$<CONFIG:RELWITHDEBINFO>>:NDEBUG;URTBLDENV_FRIENDLY=Retail>")

if (MSVC)
add_linker_flag(/GUARD:CF)
add_linker_flag(/guard:cf)
kouvel marked this conversation as resolved.
Show resolved Hide resolved
#if (CLR_CMAKE_HOST_ARCH_AMD64)
# add_linker_flag(/guard:ehcont)
#endif (CLR_CMAKE_HOST_ARCH_AMD64)

# Linker flags
#
Expand All @@ -71,6 +74,10 @@ if (MSVC)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /IGNORE:4197,4013,4254,4070,4221")
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /SUBSYSTEM:WINDOWS,${WINDOWS_SUBSYSTEM_VERSION}")

#if (CLR_CMAKE_HOST_ARCH_AMD64)
# set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /CETCOMPAT")
#endif (CLR_CMAKE_HOST_ARCH_AMD64)

set(CMAKE_STATIC_LINKER_FLAGS "${CMAKE_STATIC_LINKER_FLAGS} /IGNORE:4221")

set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /DEBUG")
Expand Down Expand Up @@ -565,6 +572,14 @@ if (MSVC)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /guard:cf")
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /guard:cf")

# Enable EH-continuation table for native components for amd64 builds
# Added using variables instead of add_compile_options to let individual projects override it
#if (CLR_CMAKE_HOST_ARCH_AMD64)
# set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /guard:ehcont")
# set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /guard:ehcont")
# set(CMAKE_ASM_MASM_FLAGS "${CMAKE_C_FLAGS} /guard:ehcont")
#endif (CLR_CMAKE_HOST_ARCH_AMD64)

# Statically linked CRT (libcmt[d].lib, libvcruntime[d].lib and libucrt[d].lib) by default. This is done to avoid
# linking in VCRUNTIME140.DLL for a simplified xcopy experience by reducing the dependency on VC REDIST.
#
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/clrdefinitions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,10 @@ if (NOT CLR_CMAKE_TARGET_ARCH_I386 OR NOT CLR_CMAKE_TARGET_WIN32)
add_compile_definitions($<$<NOT:$<BOOL:$<TARGET_PROPERTY:IGNORE_DEFAULT_TARGET_ARCH>>>:FEATURE_EH_FUNCLETS>)
endif (NOT CLR_CMAKE_TARGET_ARCH_I386 OR NOT CLR_CMAKE_TARGET_WIN32)

if (CLR_CMAKE_TARGET_WIN32 AND CLR_CMAKE_TARGET_ARCH_AMD64)
add_definitions(-DFEATURE_SPECIAL_USER_MODE_APC)
endif()


# Use this function to enable building with a specific target OS and architecture set of defines
# This is known to work for the set of defines used by the JIT and gcinfo, it is not likely correct for
Expand Down
28 changes: 20 additions & 8 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -992,10 +992,9 @@ Debugger::~Debugger()
}

#if defined(FEATURE_HIJACK) && !defined(TARGET_UNIX)
typedef void (*PFN_HIJACK_FUNCTION) (void);

// Given the start address and the end address of a function, return a MemoryRange for the function.
inline MemoryRange GetMemoryRangeForFunction(PFN_HIJACK_FUNCTION pfnStart, PFN_HIJACK_FUNCTION pfnEnd)
inline MemoryRange GetMemoryRangeForFunction(void *pfnStart, void *pfnEnd)
{
PCODE pfnStartAddress = (PCODE)GetEEFuncEntryPoint(pfnStart);
PCODE pfnEndAddress = (PCODE)GetEEFuncEntryPoint(pfnEnd);
Expand All @@ -1016,6 +1015,11 @@ MemoryRange Debugger::s_hijackFunction[kMaxHijackFunctions] =
GetMemoryRangeForFunction(RedirectedHandledJITCaseForGCStress_Stub,
RedirectedHandledJITCaseForGCStress_StubEnd)
#endif // HAVE_GCCOVER && TARGET_AMD64
#ifdef FEATURE_SPECIAL_USER_MODE_APC
,
GetMemoryRangeForFunction(ApcActivationCallbackStub,
ApcActivationCallbackStubEnd)
#endif // FEATURE_SPECIAL_USER_MODE_APC
};
#endif // FEATURE_HIJACK && !TARGET_UNIX

Expand Down Expand Up @@ -15890,7 +15894,7 @@ HRESULT Debugger::UpdateSpecialThreadList(DWORD cThreadArrayLength,
//
// 3) If the IP is in the prolog or epilog of a managed function.
//
BOOL Debugger::IsThreadContextInvalid(Thread *pThread)
BOOL Debugger::IsThreadContextInvalid(Thread *pThread, CONTEXT *pCtx)
{
CONTRACTL
{
Expand All @@ -15902,14 +15906,22 @@ BOOL Debugger::IsThreadContextInvalid(Thread *pThread)
BOOL invalid = FALSE;

// Get the thread context.
BOOL success = pCtx != NULL;
CONTEXT ctx;
ctx.ContextFlags = CONTEXT_CONTROL;
BOOL success = pThread->GetThreadContext(&ctx);
if (!success)
{
ctx.ContextFlags = CONTEXT_CONTROL;
BOOL success = pThread->GetThreadContext(&ctx);
if (success)
{
pCtx = &ctx;
}
}

if (success)
{
// Check single-step flag
if (IsSSFlagEnabled(reinterpret_cast<DT_CONTEXT *>(&ctx) ARM_ARG(pThread) ARM64_ARG(pThread)))
if (IsSSFlagEnabled(reinterpret_cast<DT_CONTEXT *>(pCtx) ARM_ARG(pThread) ARM64_ARG(pThread)))
{
// Can't hijack a thread whose SS-flag is set. This could lead to races
// with the thread taking the SS-exception.
Expand All @@ -15923,7 +15935,7 @@ BOOL Debugger::IsThreadContextInvalid(Thread *pThread)
{
#ifdef TARGET_X86
// Grab Eip - 1
LPVOID address = (((BYTE*)GetIP(&ctx)) - 1);
LPVOID address = (((BYTE*)GetIP(pCtx)) - 1);

EX_TRY
{
Expand All @@ -15934,7 +15946,7 @@ BOOL Debugger::IsThreadContextInvalid(Thread *pThread)
if (AddressIsBreakpoint((CORDB_ADDRESS_TYPE*)address))
{
size_t prologSize; // Unused...
if (g_pEEInterface->IsInPrologOrEpilog((BYTE*)GetIP(&ctx), &prologSize))
if (g_pEEInterface->IsInPrologOrEpilog((BYTE*)GetIP(pCtx), &prologSize))
{
LOG((LF_CORDB, LL_INFO1000, "D::ITCI: thread is after a BP and in prolog or epilog.\n"));
invalid = TRUE;
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -2505,7 +2505,7 @@ class Debugger : public DebugInterface
SIZE_T *rgVal2,
BYTE **rgpVCs);

BOOL IsThreadContextInvalid(Thread *pThread);
BOOL IsThreadContextInvalid(Thread *pThread, T_CONTEXT *pCtx);

// notification for SQL fiber debugging support
void CreateConnection(CONNID dwConnectionId, __in_z WCHAR *wzName);
Expand Down Expand Up @@ -2875,6 +2875,9 @@ class Debugger : public DebugInterface
#if defined(HAVE_GCCOVER) && defined(TARGET_AMD64)
kRedirectedForGCStress,
#endif // HAVE_GCCOVER && TARGET_AMD64
#ifdef FEATURE_SPECIAL_USER_MODE_APC
kRedirectedForApcActivation,
#endif // FEATURE_SPECIAL_USER_MODE_APC
kMaxHijackFunctions,
};

Expand Down Expand Up @@ -2976,6 +2979,11 @@ void RedirectedHandledJITCaseForUserSuspend_StubEnd();
void RedirectedHandledJITCaseForGCStress_Stub();
void RedirectedHandledJITCaseForGCStress_StubEnd();
#endif // HAVE_GCCOVER && TARGET_AMD64

#ifdef FEATURE_SPECIAL_USER_MODE_APC
void NTAPI ApcActivationCallbackStub(ULONG_PTR Parameter);
void ApcActivationCallbackStubEnd();
#endif // FEATURE_SPECIAL_USER_MODE_APC
};


Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/dlls/clretwrc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,12 @@ if(CLR_CMAKE_HOST_WIN32)
string(REPLACE "/LTCG" "" CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO ${CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO})
string(REPLACE "/LTCG" "" CMAKE_STATIC_LINKER_FLAGS_RELWITHDEBINFO ${CMAKE_STATIC_LINKER_FLAGS_RELWITHDEBINFO})

# remove /guard:cf from resource-only libraries
# remove /guard:cf, /guard:ehcont, and /CETCOMPAT from resource-only libraries
string(REPLACE "/guard:cf" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
if (CLR_CMAKE_HOST_ARCH_AMD64)
string(REPLACE "/guard:ehcont" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
string(REPLACE "/CETCOMPAT" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
endif (CLR_CMAKE_HOST_ARCH_AMD64)

set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /NOENTRY")
endif(CLR_CMAKE_HOST_WIN32)
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/dlls/mscorrc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ if(CLR_CMAKE_HOST_WIN32)
string(REPLACE "/LTCG" "" CMAKE_SHARED_LINKER_FLAGS_RELEASE ${CMAKE_SHARED_LINKER_FLAGS_RELEASE})
string(REPLACE "/LTCG" "" CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO ${CMAKE_SHARED_LINKER_FLAGS_RELWITHDEBINFO})

# remove /guard:cf from resource-only libraries
# remove /guard:cf, /guard:ehcont, and /CETCOMPAT from resource-only libraries
string(REPLACE "/guard:cf" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
if (CLR_CMAKE_HOST_ARCH_AMD64)
string(REPLACE "/guard:ehcont" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
string(REPLACE "/CETCOMPAT" "" CMAKE_SHARED_LINKER_FLAGS ${CMAKE_SHARED_LINKER_FLAGS})
endif (CLR_CMAKE_HOST_ARCH_AMD64)

add_library_clr(mscorrc SHARED
include.rc
Expand Down
33 changes: 32 additions & 1 deletion src/coreclr/vm/amd64/RedirectedHandledJITCase.asm
Original file line number Diff line number Diff line change
Expand Up @@ -232,5 +232,36 @@ NESTED_END NakedThrowHelper2, _TEXT
GenerateRedirectedStubWithFrame NakedThrowHelper, FixContextHandler, NakedThrowHelper2


end
ifdef FEATURE_SPECIAL_USER_MODE_APC

extern ?ApcActivationCallback@Thread@@CAX_K@Z:proc

; extern "C" void NTAPI ApcActivationCallbackStub(ULONG_PTR Parameter);
NESTED_ENTRY ApcActivationCallbackStub, _TEXT, FixRedirectContextHandler

push_nonvol_reg rbp
alloc_stack 30h ; padding for alignment, CONTEXT *, callee scratch area
set_frame rbp, 0
.errnz REDIRECTSTUB_ESTABLISHER_OFFSET_RBP, REDIRECTSTUB_ESTABLISHER_OFFSET_RBP has changed - update asm stubs
END_PROLOGUE

; Save the pointer to the interrupted context on the stack for the stack walker
mov rax, [rcx + OFFSETOF__APC_CALLBACK_DATA__ContextRecord]
mov [rbp + 20h], rax
.errnz REDIRECTSTUB_RBP_OFFSET_CONTEXT - 20h, REDIRECTSTUB_RBP_OFFSET_CONTEXT has changed - update asm stubs
kouvel marked this conversation as resolved.
Show resolved Hide resolved

call ?ApcActivationCallback@Thread@@CAX_K@Z

add rsp, 30h
pop rbp
ret

; Put a label here to tell the debugger where the end of this function is.
PATCH_LABEL ApcActivationCallbackStubEnd

NESTED_END ApcActivationCallbackStub, _TEXT

endif ; FEATURE_SPECIAL_USER_MODE_APC


end
4 changes: 4 additions & 0 deletions src/coreclr/vm/amd64/asmconstants.h
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,10 @@ ASMCONSTANTS_C_ASSERT(OFFSET__TEB__ThreadLocalStoragePointer == offsetof(TEB, Th

#define THROWSTUB_ESTABLISHER_OFFSET_FaultingExceptionFrame 0x30

#ifdef FEATURE_SPECIAL_USER_MODE_APC
#define OFFSETOF__APC_CALLBACK_DATA__ContextRecord 0x8
#endif

#define Thread__ObjectRefFlush ?ObjectRefFlush@Thread@@SAXPEAV1@@Z


Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/ceemain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -677,6 +677,7 @@ void EEStartupHelper()

IfFailGo(ExecutableAllocator::StaticInitialize(FatalErrorHandler));

Thread::StaticInitialize();
ThreadpoolMgr::StaticInitialize();

MethodDescBackpatchInfoTracker::StaticInitialize();
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/dbginterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ class DebugInterface
SIZE_T *rgVal2,
BYTE **rgpVCs) = 0;

virtual BOOL IsThreadContextInvalid(Thread *pThread) = 0;
virtual BOOL IsThreadContextInvalid(Thread *pThread, CONTEXT *pCtx) = 0;

// For Just-My-Code (aka Just-User-Code).
// The jit inserts probes that look like.
Expand Down
28 changes: 23 additions & 5 deletions src/coreclr/vm/excep.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6971,13 +6971,31 @@ void HandleManagedFault(EXCEPTION_RECORD* pExceptionRecord,
WRAPPER_NO_CONTRACT;

// Ok. Now we have a brand new fault in jitted code.
g_SavedExceptionInfo.Enter();
g_SavedExceptionInfo.SaveExceptionRecord(pExceptionRecord);
g_SavedExceptionInfo.SaveContext(pContext);
if (!Thread::UseContextBasedThreadRedirection())
{
// Once this code path gets enough bake time, perhaps this path could always be used instead of the alternative path to
// redirect the thread
FrameWithCookie<FaultingExceptionFrame> frameWithCookie;
FaultingExceptionFrame *frame = &frameWithCookie;
#if defined(FEATURE_EH_FUNCLETS)
*frame->GetGSCookiePtr() = GetProcessGSCookie();
#endif // FEATURE_EH_FUNCLETS
frame->InitAndLink(pContext);

SEHException exception(pExceptionRecord);
OBJECTREF managedException = CLRException::GetThrowableFromException(&exception);
RaiseTheExceptionInternalOnly(managedException, FALSE);
}
else
{
g_SavedExceptionInfo.Enter();
g_SavedExceptionInfo.SaveExceptionRecord(pExceptionRecord);
g_SavedExceptionInfo.SaveContext(pContext);

SetNakedThrowHelperArgRegistersInContext(pContext);
SetNakedThrowHelperArgRegistersInContext(pContext);

SetIP(pContext, GetEEFuncEntryPoint(NakedThrowHelper));
SetIP(pContext, GetEEFuncEntryPoint(NakedThrowHelper));
}
}

#else // USE_FEF && !TARGET_UNIX
Expand Down
55 changes: 55 additions & 0 deletions src/coreclr/vm/threads.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
#include "roapi.h"
#endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT

#ifdef FEATURE_SPECIAL_USER_MODE_APC
#include "asmconstants.h"
#endif

static const PortableTailCallFrame g_sentinelTailCallFrame = { NULL, NULL };

TailCallTls::TailCallTls()
Expand Down Expand Up @@ -1641,6 +1645,7 @@ Thread::Thread()

m_currentPrepareCodeConfig = nullptr;
m_isInForbidSuspendForDebuggerRegion = false;
m_hasPendingActivation = false;

#ifdef _DEBUG
memset(dangerousObjRefs, 0, sizeof(dangerousObjRefs));
Expand Down Expand Up @@ -8304,7 +8309,57 @@ BOOL dbgOnly_IsSpecialEEThread()

#endif // _DEBUG

void Thread::StaticInitialize()
{
WRAPPER_NO_CONTRACT;

#ifdef FEATURE_SPECIAL_USER_MODE_APC
InitializeSpecialUserModeApc();

// When CET shadow stacks are enabled, support for special user-mode APCs with the necessary functionality is required
_ASSERTE_ALL_BUILDS(__FILE__, !AreCetShadowStacksEnabled() || UseSpecialUserModeApc());
#endif
}

#ifdef FEATURE_SPECIAL_USER_MODE_APC

QueueUserAPC2Proc Thread::s_pfnQueueUserAPC2Proc;

static void NTAPI EmptyApcCallback(ULONG_PTR Parameter)
{
LIMITED_METHOD_CONTRACT;
}

void Thread::InitializeSpecialUserModeApc()
{
WRAPPER_NO_CONTRACT;
static_assert_no_msg(OFFSETOF__APC_CALLBACK_DATA__ContextRecord == offsetof(CLONE_APC_CALLBACK_DATA, ContextRecord));

HMODULE hKernel32 = WszLoadLibraryEx(WINDOWS_KERNEL32_DLLNAME_W, NULL, LOAD_LIBRARY_SEARCH_SYSTEM32);

// See if QueueUserAPC2 exists
QueueUserAPC2Proc pfnQueueUserAPC2Proc = (QueueUserAPC2Proc)GetProcAddress(hKernel32, "QueueUserAPC2");
if (pfnQueueUserAPC2Proc == nullptr)
{
return;
}

// See if QueueUserAPC2 supports the special user-mode APC with a callback that includes the interrupted CONTEXT. A special
// user-mode APC can interrupt a thread that is in user mode and not in a non-alertable wait.
if (!(*pfnQueueUserAPC2Proc)(EmptyApcCallback, GetCurrentThread(), 0, SpecialUserModeApcWithContextFlags))
{
return;
}

// In the future, once code paths using the special user-mode APC get some bake time, it should be used regardless of
// whether CET shadow stacks are enabled
if (AreCetShadowStacksEnabled())
{
s_pfnQueueUserAPC2Proc = pfnQueueUserAPC2Proc;
}
}

#endif // FEATURE_SPECIAL_USER_MODE_APC
#endif // #ifndef DACCESS_COMPILE

#ifdef DACCESS_COMPILE
Expand Down
Loading