diff --git a/src/coreclr/debug/daccess/stdafx.h b/src/coreclr/debug/daccess/stdafx.h index bb7b7b2365de59..bff6a4f660379b 100644 --- a/src/coreclr/debug/daccess/stdafx.h +++ b/src/coreclr/debug/daccess/stdafx.h @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include diff --git a/src/coreclr/debug/ee/controller.cpp b/src/coreclr/debug/ee/controller.cpp index a7bed1a1001bfd..f7dfde2ae67f83 100644 --- a/src/coreclr/debug/ee/controller.cpp +++ b/src/coreclr/debug/ee/controller.cpp @@ -67,9 +67,79 @@ bool DebuggerControllerPatch::IsSafeForStackTrace() } +#ifndef DACCESS_COMPILE #ifndef FEATURE_EMULATE_SINGLESTEP -// returns a pointer to the shared buffer. each call will AddRef() the object -// before returning it so callers only need to Release() when they're finished with it. + +// +// We have to have a whole separate function for this because you +// can't use __try in a function that requires object unwinding... +// + +LONG FilterAccessViolation2(LPEXCEPTION_POINTERS ep, PVOID pv) +{ + LIMITED_METHOD_CONTRACT; + + return (ep->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) + ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH; +} + +// This helper is required because the AVInRuntimeImplOkayHolder can not +// be directly placed inside the scope of a PAL_TRY +void _CopyInstructionBlockHelper(BYTE* to, const BYTE* from) +{ + AVInRuntimeImplOkayHolder AVOkay; + + // This function only copies the portion of the instruction that follows the + // breakpoint opcode, not the breakpoint itself + to += CORDbg_BREAK_INSTRUCTION_SIZE; + from += CORDbg_BREAK_INSTRUCTION_SIZE; + + // If an AV occurs because we walked off a valid page then we need + // to be certain that all bytes on the previous page were copied. + // We are certain that we copied enough bytes to contain the instruction + // because it must have fit within the valid page. + for (int i = 0; i < MAX_INSTRUCTION_LENGTH - CORDbg_BREAK_INSTRUCTION_SIZE; i++) + { + *to++ = *from++; + } + +} + +// WARNING: this function skips copying the first CORDbg_BREAK_INSTRUCTION_SIZE bytes by design +// See the comment at the callsite in DebuggerPatchSkip::DebuggerPatchSkip for more details on +// this +void DebuggerControllerPatch::CopyInstructionBlock(BYTE *to, const BYTE* from) +{ + // We wrap the memcpy in an exception handler to handle the + // extremely rare case where we're copying an instruction off the + // end of a method that is also at the end of a page, and the next + // page is unmapped. + struct Param + { + BYTE *to; + const BYTE* from; + } param; + param.to = to; + param.from = from; + PAL_TRY(Param *, pParam, ¶m) + { + _CopyInstructionBlockHelper(pParam->to, pParam->from); + } + PAL_EXCEPT_FILTER(FilterAccessViolation2) + { + // The whole point is that if we copy up the AV, then + // that's enough to execute, otherwise we would not have been + // able to execute the code anyway. So we just ignore the + // exception. + LOG((LF_CORDB, LL_INFO10000, + "DCP::CIP: AV copying instruction block ignored.\n")); + } + PAL_ENDTRY +} + + +// Creates a new shared patch bypass buffer +// AddRef() before returning it so callers need to Release() when they're finished with it. SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBuffer() { CONTRACTL @@ -79,27 +149,110 @@ SharedPatchBypassBuffer* DebuggerControllerPatch::GetOrCreateSharedPatchBypassBu } CONTRACTL_END; - if (m_pSharedPatchBypassBuffer == NULL) + if (m_pSharedPatchBypassBuffer != NULL) { - void *pSharedPatchBypassBufferRX = g_pDebugger->GetInteropSafeExecutableHeap()->Alloc(sizeof(SharedPatchBypassBuffer)); + m_pSharedPatchBypassBuffer->AddRef(); + return m_pSharedPatchBypassBuffer; + } + + // NOTE: in order to correctly single-step RIP-relative writes on multiple threads we need to set up + // a shared buffer with the instruction and a buffer for the RIP-relative value so that all threads + // are working on the same copy. as the single-steps complete the modified data in the buffer is + // copied back to the real address to ensure proper execution of the program. + + SharedPatchBypassBuffer *pSharedPatchBypassBufferRX = (SharedPatchBypassBuffer*)g_pDebugger->GetInteropSafeExecutableHeap()->Alloc(sizeof(SharedPatchBypassBuffer)); #if defined(HOST_OSX) && defined(HOST_ARM64) - ExecutableWriterHolder sharedPatchBypassBufferWriterHolder((SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX, sizeof(SharedPatchBypassBuffer)); - void *pSharedPatchBypassBufferRW = sharedPatchBypassBufferWriterHolder.GetRW(); + ExecutableWriterHolder sharedPatchBypassBufferWriterHolder((SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX, sizeof(SharedPatchBypassBuffer)); + SharedPatchBypassBuffer *pSharedPatchBypassBufferRW = sharedPatchBypassBufferWriterHolder.GetRW(); #else // HOST_OSX && HOST_ARM64 - void *pSharedPatchBypassBufferRW = pSharedPatchBypassBufferRX; + SharedPatchBypassBuffer *pSharedPatchBypassBufferRW = pSharedPatchBypassBufferRX; #endif // HOST_OSX && HOST_ARM64 - new (pSharedPatchBypassBufferRW) SharedPatchBypassBuffer(); - m_pSharedPatchBypassBuffer = (SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX; - - _ASSERTE(m_pSharedPatchBypassBuffer); - TRACE_ALLOC(m_pSharedPatchBypassBuffer); - } + new (pSharedPatchBypassBufferRW) SharedPatchBypassBuffer(); + m_pSharedPatchBypassBuffer = (SharedPatchBypassBuffer*)pSharedPatchBypassBufferRX; + _ASSERTE(m_pSharedPatchBypassBuffer); + TRACE_ALLOC(m_pSharedPatchBypassBuffer); m_pSharedPatchBypassBuffer->AddRef(); + BYTE* patchBypassRW = pSharedPatchBypassBufferRW->PatchBypass; + BYTE* patchBypassRX = m_pSharedPatchBypassBuffer->PatchBypass; + + LOG((LF_CORDB, LL_INFO10000, "DCP::CSPBB: Patch skip for opcode 0x%.4x at address %p buffer allocated at 0x%.8x\n", this->opcode, this->address, m_pSharedPatchBypassBuffer)); + + // CopyInstructionBlock copies all the code bytes except the breakpoint byte(s). + _ASSERTE( this->IsBound() ); + CopyInstructionBlock(patchBypassRW, (const BYTE *)this->address); + + // Technically, we could create a patch skipper for an inactive patch, but we rely on the opcode being + // set here. + _ASSERTE( this->IsActivated() ); + CORDbgSetInstruction((CORDB_ADDRESS_TYPE *)patchBypassRW, this->opcode); + + LOG((LF_CORDB, LL_EVERYTHING, "DCP::CSPBB: SetInstruction was called\n")); + + // + // Look at instruction to get some attributes + // + InstructionAttribute instrAttrib = { 0 }; + NativeWalker::DecodeInstructionForPatchSkip(patchBypassRX, &instrAttrib); + +#if defined(TARGET_AMD64) + // The code below handles RIP-relative addressing on AMD64. The original implementation made the assumption that + // we are only using RIP-relative addressing to access read-only data (see VSW 246145 for more information). This + // has since been expanded to handle RIP-relative writes as well. + if (instrAttrib.m_dwOffsetToDisp != 0) + { + _ASSERTE(pSharedPatchBypassBufferRW != NULL); + _ASSERTE(instrAttrib.m_cbInstr != 0); + + // + // Populate the RIP-relative buffer with the current value if needed + // + + BYTE* bufferBypassRW = pSharedPatchBypassBufferRW->BypassBuffer; + + // Overwrite the *signed* displacement. + int dwOldDisp = *(int*)(&patchBypassRX[instrAttrib.m_dwOffsetToDisp]); + int dwNewDisp = offsetof(SharedPatchBypassBuffer, BypassBuffer) - + (offsetof(SharedPatchBypassBuffer, PatchBypass) + instrAttrib.m_cbInstr); + *(int*)(&patchBypassRW[instrAttrib.m_dwOffsetToDisp]) = dwNewDisp; + + // This could be an LEA, which we'll just have to change into a MOV and copy the original address. + if (((patchBypassRX[0] == 0x4C) || (patchBypassRX[0] == 0x48)) && (patchBypassRX[1] == 0x8d)) + { + patchBypassRW[1] = 0x8b; // MOV reg, mem + _ASSERTE((int)sizeof(void*) <= SharedPatchBypassBuffer::cbBufferBypass); + *(void**)bufferBypassRW = (void*)(this->address + instrAttrib.m_cbInstr + dwOldDisp); + } + else + { + _ASSERTE(instrAttrib.m_cOperandSize <= SharedPatchBypassBuffer::cbBufferBypass); + // Copy the data into our buffer. + memcpy(bufferBypassRW, this->address + instrAttrib.m_cbInstr + dwOldDisp, instrAttrib.m_cOperandSize); + + if (instrAttrib.m_fIsWrite) + { + // save the actual destination address and size so when we TriggerSingleStep() we can update the value + pSharedPatchBypassBufferRW->RipTargetFixup = (UINT_PTR)(this->address + instrAttrib.m_cbInstr + dwOldDisp); + pSharedPatchBypassBufferRW->RipTargetFixupSize = instrAttrib.m_cOperandSize; + } + } + } + + #endif // TARGET_AMD64 + + m_pSharedPatchBypassBuffer->SetInstructionAttrib(instrAttrib); + + // Since we just created a new buffer of code, but the CPU caches code and may + // not be aware of our changes. This should force the CPU to dump any cached + // instructions it has in this region and load the new ones from memory + FlushInstructionCache(GetCurrentProcess(), patchBypassRW + CORDbg_BREAK_INSTRUCTION_SIZE, + MAX_INSTRUCTION_LENGTH - CORDbg_BREAK_INSTRUCTION_SIZE); + return m_pSharedPatchBypassBuffer; } #endif // !FEATURE_EMULATE_SINGLESTEP +#endif // !DACCESS_COMPILE // @todo - remove all this splicing trash // This Sort/Splice stuff just reorders the patches within a particular chain such @@ -4509,53 +4662,9 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, // the single-step emulation itself. #ifndef FEATURE_EMULATE_SINGLESTEP - // NOTE: in order to correctly single-step RIP-relative writes on multiple threads we need to set up - // a shared buffer with the instruction and a buffer for the RIP-relative value so that all threads - // are working on the same copy. as the single-steps complete the modified data in the buffer is - // copied back to the real address to ensure proper execution of the program. - - // - // Create the shared instruction block. this will also create the shared RIP-relative buffer - // - + _ASSERTE(DebuggerController::HasLock()); m_pSharedPatchBypassBuffer = patch->GetOrCreateSharedPatchBypassBuffer(); -#if defined(HOST_OSX) && defined(HOST_ARM64) - ExecutableWriterHolder sharedPatchBypassBufferWriterHolder((SharedPatchBypassBuffer*)m_pSharedPatchBypassBuffer, sizeof(SharedPatchBypassBuffer)); - SharedPatchBypassBuffer *pSharedPatchBypassBufferRW = sharedPatchBypassBufferWriterHolder.GetRW(); -#else // HOST_OSX && HOST_ARM64 - SharedPatchBypassBuffer *pSharedPatchBypassBufferRW = m_pSharedPatchBypassBuffer; -#endif // HOST_OSX && HOST_ARM64 - - BYTE* patchBypassRX = m_pSharedPatchBypassBuffer->PatchBypass; - BYTE* patchBypassRW = pSharedPatchBypassBufferRW->PatchBypass; - LOG((LF_CORDB, LL_INFO10000, "DPS::DPS: Patch skip for opcode 0x%.4x at address %p buffer allocated at 0x%.8x\n", patch->opcode, patch->address, m_pSharedPatchBypassBuffer)); - - // Copy the instruction block over to the patch skip - // WARNING: there used to be an issue here because CopyInstructionBlock copied the breakpoint from the - // jitted code stream into the patch buffer. Further below CORDbgSetInstruction would correct the - // first instruction. This buffer is shared by all threads so if another thread executed the buffer - // between this thread's execution of CopyInstructionBlock and CORDbgSetInstruction the wrong - // code would be executed. The bug has been fixed by changing CopyInstructionBlock to only copy - // the code bytes after the breakpoint. - // You might be tempted to stop copying the code at all, however that wouldn't work well with rejit. - // If we skip a breakpoint that is sitting at the beginning of a method, then the profiler rejits that - // method causing a jump-stamp to be placed, then we skip the breakpoint again, we need to make sure - // the 2nd skip executes the new jump-stamp code and not the original method prologue code. Copying - // the code every time ensures that we have the most up-to-date version of the code in the buffer. - _ASSERTE( patch->IsBound() ); - CopyInstructionBlock(patchBypassRW, (const BYTE *)patch->address); - - // Technically, we could create a patch skipper for an inactive patch, but we rely on the opcode being - // set here. - _ASSERTE( patch->IsActivated() ); - CORDbgSetInstruction((CORDB_ADDRESS_TYPE *)patchBypassRW, patch->opcode); - - LOG((LF_CORDB, LL_EVERYTHING, "SetInstruction was called\n")); - // - // Look at instruction to get some attributes - // - - NativeWalker::DecodeInstructionForPatchSkip(patchBypassRX, &(m_instrAttrib)); + m_instrAttrib = m_pSharedPatchBypassBuffer->GetInstructionAttrib(); #ifdef OUT_OF_PROCESS_SETTHREADCONTEXT if (g_pDebugInterface->IsOutOfProcessSetContextEnabled() && m_instrAttrib.m_fIsCall) @@ -4564,51 +4673,6 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, } #endif -#if defined(TARGET_AMD64) - - // The code below handles RIP-relative addressing on AMD64. The original implementation made the assumption that - // we are only using RIP-relative addressing to access read-only data (see VSW 246145 for more information). This - // has since been expanded to handle RIP-relative writes as well. - if (m_instrAttrib.m_dwOffsetToDisp != 0 && !IsInPlaceSingleStep()) - { - _ASSERTE(m_instrAttrib.m_cbInstr != 0); - - // - // Populate the RIP-relative buffer with the current value if needed - // - - BYTE* bufferBypassRW = pSharedPatchBypassBufferRW->BypassBuffer; - - // Overwrite the *signed* displacement. - int dwOldDisp = *(int*)(&patchBypassRX[m_instrAttrib.m_dwOffsetToDisp]); - int dwNewDisp = offsetof(SharedPatchBypassBuffer, BypassBuffer) - - (offsetof(SharedPatchBypassBuffer, PatchBypass) + m_instrAttrib.m_cbInstr); - *(int*)(&patchBypassRW[m_instrAttrib.m_dwOffsetToDisp]) = dwNewDisp; - - // This could be an LEA, which we'll just have to change into a MOV and copy the original address. - if (((patchBypassRX[0] == 0x4C) || (patchBypassRX[0] == 0x48)) && (patchBypassRX[1] == 0x8d)) - { - patchBypassRW[1] = 0x8b; // MOV reg, mem - _ASSERTE((int)sizeof(void*) <= SharedPatchBypassBuffer::cbBufferBypass); - *(void**)bufferBypassRW = (void*)(patch->address + m_instrAttrib.m_cbInstr + dwOldDisp); - } - else - { - _ASSERTE(m_instrAttrib.m_cOperandSize <= SharedPatchBypassBuffer::cbBufferBypass); - // Copy the data into our buffer. - memcpy(bufferBypassRW, patch->address + m_instrAttrib.m_cbInstr + dwOldDisp, m_instrAttrib.m_cOperandSize); - - if (m_instrAttrib.m_fIsWrite) - { - // save the actual destination address and size so when we TriggerSingleStep() we can update the value - pSharedPatchBypassBufferRW->RipTargetFixup = (UINT_PTR)(patch->address + m_instrAttrib.m_cbInstr + dwOldDisp); - pSharedPatchBypassBufferRW->RipTargetFixupSize = m_instrAttrib.m_cOperandSize; - } - } - } - -#endif // TARGET_AMD64 - #endif // !FEATURE_EMULATE_SINGLESTEP // Signals our thread that the debugger will be manipulating the context @@ -4672,7 +4736,9 @@ DebuggerPatchSkip::DebuggerPatchSkip(Thread *thread, #else // FEATURE_EMULATE_SINGLESTEP #ifdef TARGET_ARM64 - patchBypassRX = NativeWalker::SetupOrSimulateInstructionForPatchSkip(context, m_pSharedPatchBypassBuffer, (const BYTE *)patch->address, patch->opcode); + BYTE* patchBypassRX = NativeWalker::SetupOrSimulateInstructionForPatchSkip(context, m_pSharedPatchBypassBuffer, (const BYTE *)patch->address, patch->opcode); +#else + BYTE* patchBypassRX = m_pSharedPatchBypassBuffer->PatchBypass; #endif //TARGET_ARM64 if (!IsInPlaceSingleStep()) @@ -4742,80 +4808,6 @@ void DebuggerPatchSkip::DebuggerDetachClean() #endif // !FEATURE_EMULATE_SINGLESTEP } - -// -// We have to have a whole separate function for this because you -// can't use __try in a function that requires object unwinding... -// - -LONG FilterAccessViolation2(LPEXCEPTION_POINTERS ep, PVOID pv) -{ - LIMITED_METHOD_CONTRACT; - - return (ep->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) - ? EXCEPTION_EXECUTE_HANDLER : EXCEPTION_CONTINUE_SEARCH; -} - -// This helper is required because the AVInRuntimeImplOkayHolder can not -// be directly placed inside the scope of a PAL_TRY -void _CopyInstructionBlockHelper(BYTE* to, const BYTE* from) -{ - AVInRuntimeImplOkayHolder AVOkay; - - // This function only copies the portion of the instruction that follows the - // breakpoint opcode, not the breakpoint itself - to += CORDbg_BREAK_INSTRUCTION_SIZE; - from += CORDbg_BREAK_INSTRUCTION_SIZE; - - // If an AV occurs because we walked off a valid page then we need - // to be certain that all bytes on the previous page were copied. - // We are certain that we copied enough bytes to contain the instruction - // because it must have fit within the valid page. - for (int i = 0; i < MAX_INSTRUCTION_LENGTH - CORDbg_BREAK_INSTRUCTION_SIZE; i++) - { - *to++ = *from++; - } - -} - -// WARNING: this function skips copying the first CORDbg_BREAK_INSTRUCTION_SIZE bytes by design -// See the comment at the callsite in DebuggerPatchSkip::DebuggerPatchSkip for more details on -// this -void DebuggerPatchSkip::CopyInstructionBlock(BYTE *to, const BYTE* from) -{ - // We wrap the memcpy in an exception handler to handle the - // extremely rare case where we're copying an instruction off the - // end of a method that is also at the end of a page, and the next - // page is unmapped. - struct Param - { - BYTE *to; - const BYTE* from; - } param; - param.to = to; - param.from = from; - PAL_TRY(Param *, pParam, ¶m) - { - _CopyInstructionBlockHelper(pParam->to, pParam->from); - } - PAL_EXCEPT_FILTER(FilterAccessViolation2) - { - // The whole point is that if we copy up the AV, then - // that's enough to execute, otherwise we would not have been - // able to execute the code anyway. So we just ignore the - // exception. - LOG((LF_CORDB, LL_INFO10000, - "DPS::DPS: AV copying instruction block ignored.\n")); - } - PAL_ENDTRY - - // We just created a new buffer of code, but the CPU caches code and may - // not be aware of our changes. This should force the CPU to dump any cached - // instructions it has in this region and load the new ones from memory - FlushInstructionCache(GetCurrentProcess(), to + CORDbg_BREAK_INSTRUCTION_SIZE, - MAX_INSTRUCTION_LENGTH - CORDbg_BREAK_INSTRUCTION_SIZE); -} - TP_RESULT DebuggerPatchSkip::TriggerPatch(DebuggerControllerPatch *patch, Thread *thread, TRIGGER_WHY tyWhy) diff --git a/src/coreclr/debug/ee/controller.h b/src/coreclr/debug/ee/controller.h index b02cd2802d91ad..32138edc36aba7 100644 --- a/src/coreclr/debug/ee/controller.h +++ b/src/coreclr/debug/ee/controller.h @@ -312,7 +312,11 @@ class SharedPatchBypassBuffer UINT_PTR RipTargetFixup; #endif + const InstructionAttribute& GetInstructionAttrib() { return m_instrAttrib; } + void SetInstructionAttrib(const InstructionAttribute& instrAttrib) { m_instrAttrib = instrAttrib; } + private: + InstructionAttribute m_instrAttrib; // info about the instruction being skipped over const static DWORD SentinelValue = 0xffffffff; LONG m_refCount; }; @@ -550,10 +554,13 @@ struct DebuggerControllerPatch // Is this patch at a position at which it's safe to take a stack? bool IsSafeForStackTrace(); +#ifndef DACCESS_COMPILE #ifndef FEATURE_EMULATE_SINGLESTEP // gets a pointer to the shared buffer SharedPatchBypassBuffer* GetOrCreateSharedPatchBypassBuffer(); + void CopyInstructionBlock(BYTE *to, const BYTE* from); + // entry point for general initialization when the controller is being created void Initialize() { @@ -567,6 +574,7 @@ struct DebuggerControllerPatch m_pSharedPatchBypassBuffer->Release(); } #endif // !FEATURE_EMULATE_SINGLESTEP +#endif // !DACCESS_COMPILE void LogInstance() { @@ -1515,8 +1523,6 @@ class DebuggerPatchSkip : public DebuggerController virtual DEBUGGER_CONTROLLER_TYPE GetDCType(void) { return DEBUGGER_CONTROLLER_PATCH_SKIP; } - void CopyInstructionBlock(BYTE *to, const BYTE* from); - void DecodeInstruction(CORDB_ADDRESS_TYPE *code); void DebuggerDetachClean(); diff --git a/src/coreclr/debug/ee/walker.h b/src/coreclr/debug/ee/walker.h index 63bd4cfa2fd273..4dcae6d75ed273 100644 --- a/src/coreclr/debug/ee/walker.h +++ b/src/coreclr/debug/ee/walker.h @@ -62,6 +62,8 @@ struct InstructionAttribute } }; +#ifndef DACCESS_COMPILE + /* ------------------------------------------------------------------------- * * Classes * ------------------------------------------------------------------------- */ @@ -280,4 +282,6 @@ class NativeWalker : public Walker }; #endif +#endif // DACCESS_COMPILE + #endif // WALKER_H_