From 3c2d2a14d466c91304ec5f616e29d0236ddb06dd Mon Sep 17 00:00:00 2001 From: Mike McLaughlin Date: Thu, 13 Jan 2022 10:20:13 -0800 Subject: [PATCH] Fix the MacOS remote unwinder for VS4Mac (#63405) * Fix the MacOS remote unwinder for VS4Mac The wrong module was being passed to the remote unwinder because the load bias for shared modules was being calculated incorrectly. Issue: https://github.com/dotnet/runtime/issues/63309 * Fix native frame unwind in syscall on arm64 for VS4Mac crash report From PR in main: https://github.com/dotnet/runtime/pull/63598 Add arm64 version of StepWithCompactNoEncoding for syscall leaf node wrappers that have compact encoding of 0. Fix ReadCompactEncodingRegister so it actually decrements the addr. Change StepWithCompactEncodingArm64 to match what MacOS libunwind does for framed and frameless stepping. arm64 can have frames with the same SP (but different IPs). Increment SP for this condition so createdump's unwind loop doesn't break out on the "SP not increasing" check and the frames are added to the thread frame list in the correct order. Add getting the unwind info for tail called functions like this: __ZL14PROCEndProcessPvji: 36630: f6 57 bd a9 stp x22, x21, [sp, #-48]! 36634: f4 4f 01 a9 stp x20, x19, [sp, #16] 36638: fd 7b 02 a9 stp x29, x30, [sp, #32] 3663c: fd 83 00 91 add x29, sp, #32 ... 367ac: e9 01 80 52 mov w9, #15 367b0: 7f 3e 02 71 cmp w19, #143 367b4: 20 01 88 1a csel w0, w9, w8, eq 367b8: 2e 00 00 94 bl _PROCAbort _TerminateProcess: -> 367bc: 22 00 80 52 mov w2, #1 367c0: 9c ff ff 17 b __ZL14PROCEndProcessPvji The IP (367bc) returns the (incorrect) frameless encoding with nothing on the stack (uses an incorrect LR to unwind). To fix this get the unwind info for PC -1 which points to PROCEndProcess with the correct unwind info. This matches how lldb unwinds this frame. Always address module segment to IP lookup list instead of checking the module regions. Strip pointer authentication bits on PC/LR. --- src/coreclr/debug/createdump/crashinfomac.cpp | 6 +- src/coreclr/debug/createdump/stackframe.h | 9 +- src/coreclr/debug/createdump/threadinfo.cpp | 10 ++ src/coreclr/debug/dbgutil/machoreader.cpp | 5 +- .../pal/src/exception/remote-unwind.cpp | 129 +++++++++++++----- 5 files changed, 118 insertions(+), 41 deletions(-) diff --git a/src/coreclr/debug/createdump/crashinfomac.cpp b/src/coreclr/debug/createdump/crashinfomac.cpp index ad9c247e37dfd..3ada3bd767c96 100644 --- a/src/coreclr/debug/createdump/crashinfomac.cpp +++ b/src/coreclr/debug/createdump/crashinfomac.cpp @@ -277,6 +277,9 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm uint64_t start = segment.vmaddr + module.LoadBias(); uint64_t end = start + segment.vmsize; + // Add this module segment to the set used by the thread unwinding to lookup the module base address for an ip. + AddModuleAddressRange(start, end, module.BaseAddress()); + // Round to page boundary start = start & PAGE_MASK; _ASSERTE(start > 0); @@ -297,9 +300,6 @@ void CrashInfo::VisitSegment(MachOModule& module, const segment_command_64& segm } // Add this module segment to the module mappings list m_moduleMappings.insert(moduleRegion); - - // Add this module segment to the set used by the thread unwinding to lookup the module base address for an ip. - AddModuleAddressRange(start, end, module.BaseAddress()); } else { diff --git a/src/coreclr/debug/createdump/stackframe.h b/src/coreclr/debug/createdump/stackframe.h index 75e20d93120c0..00c3a1cfb7fd8 100644 --- a/src/coreclr/debug/createdump/stackframe.h +++ b/src/coreclr/debug/createdump/stackframe.h @@ -66,9 +66,16 @@ struct StackFrame } } +// See comment in threadinfo.cpp UnwindNativeFrames function +#if defined(__aarch64__) + #define STACK_POINTER_MASK ~0x7 +#else + #define STACK_POINTER_MASK ~0x0 +#endif + inline uint64_t ModuleAddress() const { return m_moduleAddress; } inline uint64_t InstructionPointer() const { return m_instructionPointer; } - inline uint64_t StackPointer() const { return m_stackPointer; } + inline uint64_t StackPointer() const { return m_stackPointer & STACK_POINTER_MASK; } inline uint32_t NativeOffset() const { return m_nativeOffset; } inline uint32_t Token() const { return m_token; } inline uint32_t ILOffset() const { return m_ilOffset; } diff --git a/src/coreclr/debug/createdump/threadinfo.cpp b/src/coreclr/debug/createdump/threadinfo.cpp index 82509f5275065..e64eafc8d29a9 100644 --- a/src/coreclr/debug/createdump/threadinfo.cpp +++ b/src/coreclr/debug/createdump/threadinfo.cpp @@ -53,6 +53,16 @@ ThreadInfo::UnwindNativeFrames(CONTEXT* pContext) uint64_t ip = 0, sp = 0; GetFrameLocation(pContext, &ip, &sp); +#if defined(__aarch64__) + // ARM64 can have frames with the same SP but different IPs. Increment sp so it gets added to the stack + // frames in the correct order and to prevent the below loop termination on non-increasing sp. Since stack + // pointers are always 8 byte align, this increase is masked off in StackFrame::StackPointer() to get the + // original stack pointer. + if (sp == previousSp && ip != previousIp) + { + sp++; + } +#endif if (ip == 0 || sp <= previousSp) { TRACE_VERBOSE("Unwind: sp not increasing or ip == 0 sp %p ip %p\n", (void*)sp, (void*)ip); break; diff --git a/src/coreclr/debug/dbgutil/machoreader.cpp b/src/coreclr/debug/dbgutil/machoreader.cpp index d2801547cb9ba..7fef34e1afb16 100644 --- a/src/coreclr/debug/dbgutil/machoreader.cpp +++ b/src/coreclr/debug/dbgutil/machoreader.cpp @@ -229,9 +229,8 @@ MachOModule::ReadLoadCommands() m_segments.push_back(segment); // Calculate the load bias for the module. This is the value to add to the vmaddr of a - // segment to get the actual address. For shared modules, this is 0 since those segments - // are absolute address. - if (segment->fileoff == 0 && segment->filesize > 0) + // segment to get the actual address. + if (strcmp(segment->segname, SEG_TEXT) == 0) { m_loadBias = m_baseAddress - segment->vmaddr; } diff --git a/src/coreclr/pal/src/exception/remote-unwind.cpp b/src/coreclr/pal/src/exception/remote-unwind.cpp index af0293ba5bc60..b27fc9680bbed 100644 --- a/src/coreclr/pal/src/exception/remote-unwind.cpp +++ b/src/coreclr/pal/src/exception/remote-unwind.cpp @@ -57,6 +57,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #include #include #include "compact_unwind_encoding.h" +#define MACOS_ARM64_POINTER_AUTH_MASK 0x7fffffffffffull #endif // Sub-headers included from the libunwind.h contain an empty struct @@ -1422,25 +1423,56 @@ StepWithCompactNoEncoding(const libunwindInfo* info) #if defined(TARGET_ARM64) -inline static bool +#define ARM64_SYSCALL_OPCODE 0xD4001001 +#define ARM64_BL_OPCODE_MASK 0xFC000000 +#define ARM64_BL_OPCODE 0x94000000 +#define ARM64_BLR_OPCODE_MASK 0xFFFFFC00 +#define ARM64_BLR_OPCODE 0xD63F0000 +#define ARM64_BLRA_OPCODE_MASK 0xFEFFF800 +#define ARM64_BLRA_OPCODE 0xD63F0800 + +static bool +StepWithCompactNoEncoding(const libunwindInfo* info) +{ + // Check that the function is a syscall "wrapper" and assume there is no frame and pop the return address. + uint32_t opcode; + unw_word_t addr = info->Context->Pc - sizeof(opcode); + if (!ReadValue32(info, &addr, &opcode)) { + ERROR("StepWithCompactNoEncoding: can read opcode %p\n", (void*)addr); + return false; + } + // Is the IP pointing just after a "syscall" opcode? + if (opcode != ARM64_SYSCALL_OPCODE) { + ERROR("StepWithCompactNoEncoding: not in syscall wrapper function\n"); + return false; + } + // Pop the return address from the stack + info->Context->Pc = info->Context->Lr; + TRACE("StepWithCompactNoEncoding: SUCCESS new pc %p sp %p\n", (void*)info->Context->Pc, (void*)info->Context->Sp); + return true; +} + +static bool ReadCompactEncodingRegister(const libunwindInfo* info, unw_word_t* addr, DWORD64* reg) { - *addr -= sizeof(uint64_t); - if (!ReadValue64(info, addr, (uint64_t*)reg)) { + uint64_t value; + if (!info->ReadMemory((PVOID)*addr, &value, sizeof(value))) { return false; } + *reg = VAL64(value); + *addr -= sizeof(uint64_t); return true; } -inline static bool -ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWORD64*second, DWORD64* first) +static bool +ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWORD64* first, DWORD64* second) { // Registers are effectively pushed in pairs // + // *first = **addr // *addr -= 8 - // **addr = *first + // *second= **addr // *addr -= 8 - // **addr = *second if (!ReadCompactEncodingRegister(info, addr, first)) { return false; } @@ -1450,8 +1482,8 @@ ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWO return true; } -inline static bool -ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, NEON128*second, NEON128* first) +static bool +ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, NEON128* first, NEON128* second) { if (!ReadCompactEncodingRegisterPair(info, addr, &first->Low, &second->Low)) { return false; @@ -1484,30 +1516,28 @@ static bool StepWithCompactEncodingArm64(const libunwindInfo* info, compact_unwind_encoding_t compactEncoding, bool hasFrame) { CONTEXT* context = info->Context; + unw_word_t addr; - unw_word_t callerSp; - - if (hasFrame) { - // caller Sp is callee Fp plus saved FP and LR - callerSp = context->Fp + 2 * sizeof(uint64_t); - } else { + if (hasFrame) + { + context->Sp = context->Fp + 16; + addr = context->Fp + 8; + if (!ReadCompactEncodingRegisterPair(info, &addr, &context->Lr, &context->Fp)) { + return false; + } + // Strip pointer authentication bits + context->Lr &= MACOS_ARM64_POINTER_AUTH_MASK; + } + else + { // Get the leat significant bit in UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK uint64_t stackSizeScale = UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK & ~(UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK - 1); - uint64_t stackSize = (compactEncoding & UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK) / stackSizeScale * 16; + uint64_t stackSize = ((compactEncoding & UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK) / stackSizeScale) * 16; - callerSp = context->Sp + stackSize; + addr = context->Sp + stackSize; } - context->Sp = callerSp; - - unw_word_t addr = callerSp; - - if (hasFrame && - !ReadCompactEncodingRegisterPair(info, &addr, &context->Lr, &context->Fp)) { - return false; - } - - // unwound return address is stored in Lr + // Unwound return address is stored in Lr context->Pc = context->Lr; if (compactEncoding & UNWIND_ARM64_FRAME_X19_X20_PAIR && @@ -1546,7 +1576,10 @@ StepWithCompactEncodingArm64(const libunwindInfo* info, compact_unwind_encoding_ !ReadCompactEncodingRegisterPair(info, &addr, &context->V[14], &context->V[15])) { return false; } - + if (!hasFrame) + { + context->Sp = addr; + } TRACE("SUCCESS: compact step encoding %08x pc %p sp %p fp %p lr %p\n", compactEncoding, (void*)context->Pc, (void*)context->Sp, (void*)context->Fp, (void*)context->Lr); return true; @@ -1557,11 +1590,11 @@ StepWithCompactEncodingArm64(const libunwindInfo* info, compact_unwind_encoding_ static bool StepWithCompactEncoding(const libunwindInfo* info, compact_unwind_encoding_t compactEncoding, unw_word_t functionStart) { -#if defined(TARGET_AMD64) if (compactEncoding == 0) { return StepWithCompactNoEncoding(info); } +#if defined(TARGET_AMD64) switch (compactEncoding & UNWIND_X86_64_MODE_MASK) { case UNWIND_X86_64_MODE_RBP_FRAME: @@ -1575,11 +1608,6 @@ StepWithCompactEncoding(const libunwindInfo* info, compact_unwind_encoding_t com return false; } #elif defined(TARGET_ARM64) - if (compactEncoding == 0) - { - TRACE("Compact unwind missing for %p\n", (void*)info->Context->Pc); - return false; - } switch (compactEncoding & UNWIND_ARM64_MODE_MASK) { case UNWIND_ARM64_MODE_FRAME: @@ -1717,6 +1745,12 @@ static void UnwindContextToContext(unw_cursor_t *cursor, CONTEXT *winContext) unw_get_reg(cursor, UNW_AARCH64_X28, (unw_word_t *) &winContext->X28); unw_get_reg(cursor, UNW_AARCH64_X29, (unw_word_t *) &winContext->Fp); unw_get_reg(cursor, UNW_AARCH64_X30, (unw_word_t *) &winContext->Lr); +#ifdef __APPLE__ + // Strip pointer authentication bits which seem to be leaking out of libunwind + // Seems like ptrauth_strip() / __builtin_ptrauth_strip() should work, but currently + // errors with "this target does not support pointer authentication" + winContext->Pc = winContext->Pc & MACOS_ARM64_POINTER_AUTH_MASK; +#endif // __APPLE__ TRACE("sp %p pc %p lr %p fp %p\n", winContext->Sp, winContext->Pc, winContext->Lr, winContext->Fp); #elif defined(TARGET_S390X) unw_get_reg(cursor, UNW_REG_IP, (unw_word_t *) &winContext->PSWAddr); @@ -2126,6 +2160,33 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont #elif defined(TARGET_ARM64) TRACE("Unwind: pc %p sp %p fp %p\n", (void*)context->Pc, (void*)context->Sp, (void*)context->Fp); result = GetProcInfo(context->Pc, &procInfo, &info, &step, false); + if (result && step) + { + // If the PC is at the start of the function, the previous instruction is BL and the unwind encoding is frameless + // with nothing on stack (0x02000000), back up PC by 1 to the previous function and get the unwind info for that + // function. + if ((context->Pc == procInfo.start_ip) && + (procInfo.format & (UNWIND_ARM64_MODE_MASK | UNWIND_ARM64_FRAMELESS_STACK_SIZE_MASK)) == UNWIND_ARM64_MODE_FRAMELESS) + { + uint32_t opcode; + unw_word_t addr = context->Pc - sizeof(opcode); + if (ReadValue32(&info, &addr, &opcode)) + { + // Is the previous instruction a BL opcode? + if ((opcode & ARM64_BL_OPCODE_MASK) == ARM64_BL_OPCODE || + (opcode & ARM64_BLR_OPCODE_MASK) == ARM64_BLR_OPCODE || + (opcode & ARM64_BLRA_OPCODE_MASK) == ARM64_BLRA_OPCODE) + { + TRACE("Unwind: getting unwind info for PC - 1 opcode %08x\n", opcode); + result = GetProcInfo(context->Pc - 1, &procInfo, &info, &step, false); + } + else + { + TRACE("Unwind: not BL* opcode %08x\n", opcode); + } + } + } + } #else #error Unexpected architecture #endif