Skip to content

Commit

Permalink
[release/6.0-vs4mac] Fix the MacOS remote unwinder for VS4Mac (#63758)
Browse files Browse the repository at this point in the history
* 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: #63309

* Fix native frame unwind in syscall on arm64 for VS4Mac crash report

From PR in main: #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.

Co-authored-by: Mike McLaughlin <[email protected]>
  • Loading branch information
github-actions[bot] and mikem8361 authored Jan 13, 2022
1 parent 337202c commit 91f3110
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 41 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/debug/createdump/crashinfomac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
{
Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/debug/createdump/stackframe.h
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
10 changes: 10 additions & 0 deletions src/coreclr/debug/createdump/threadinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/debug/dbgutil/machoreader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
129 changes: 95 additions & 34 deletions src/coreclr/pal/src/exception/remote-unwind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
#include <mach-o/nlist.h>
#include <mach-o/dyld_images.h>
#include "compact_unwind_encoding.h"
#define MACOS_ARM64_POINTER_AUTH_MASK 0x7fffffffffffull
#endif

// Sub-headers included from the libunwind.h contain an empty struct
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 91f3110

Please sign in to comment.