Skip to content

Commit

Permalink
Revert "Fix native frame unwind in syscall on arm64 for VS4Mac crash …
Browse files Browse the repository at this point in the history 
…report. (#63598)"

This reverts commit dc67c70.
  • Loading branch information
@agocke
agocke committed Jan 13, 2022
1 parent 93c6134 commit fc94ec4
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 116 deletions.
6 changes: 3 additions & 3 deletions src/coreclr/debug/createdump/crashinfomac.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,6 @@ 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 @@ -300,6 +297,9 @@ 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: 1 addition & 8 deletions src/coreclr/debug/createdump/stackframe.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,9 @@ 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 & STACK_POINTER_MASK; }
inline uint64_t StackPointer() const { return m_stackPointer; }
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: 0 additions & 10 deletions src/coreclr/debug/createdump/threadinfo.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,6 @@ 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
129 changes: 34 additions & 95 deletions src/coreclr/pal/src/exception/remote-unwind.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ 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 @@ -1423,56 +1422,25 @@ StepWithCompactNoEncoding(const libunwindInfo* info)

#if defined(TARGET_ARM64)

#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
inline static bool
ReadCompactEncodingRegister(const libunwindInfo* info, unw_word_t* addr, DWORD64* reg)
{
uint64_t value;
if (!info->ReadMemory((PVOID)*addr, &value, sizeof(value))) {
*addr -= sizeof(uint64_t);
if (!ReadValue64(info, addr, (uint64_t*)reg)) {
return false;
}
*reg = VAL64(value);
*addr -= sizeof(uint64_t);
return true;
}

static bool
ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWORD64* first, DWORD64* second)
inline static bool
ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWORD64*second, DWORD64* first)
{
// Registers are effectively pushed in pairs
//
// *first = **addr
// *addr -= 8
// *second= **addr
// **addr = *first
// *addr -= 8
// **addr = *second
if (!ReadCompactEncodingRegister(info, addr, first)) {
return false;
}
Expand All @@ -1482,8 +1450,8 @@ ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, DWO
return true;
}

static bool
ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, NEON128* first, NEON128* second)
inline static bool
ReadCompactEncodingRegisterPair(const libunwindInfo* info, unw_word_t* addr, NEON128*second, NEON128* first)
{
if (!ReadCompactEncodingRegisterPair(info, addr, &first->Low, &second->Low)) {
return false;
Expand Down Expand Up @@ -1516,28 +1484,30 @@ static bool
StepWithCompactEncodingArm64(const libunwindInfo* info, compact_unwind_encoding_t compactEncoding, bool hasFrame)
{
CONTEXT* context = info->Context;
unw_word_t addr;

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
{
unw_word_t callerSp;

if (hasFrame) {
// caller Sp is callee Fp plus saved FP and LR
callerSp = context->Fp + 2 * sizeof(uint64_t);
} 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;

addr = context->Sp + stackSize;
callerSp = context->Sp + stackSize;
}

// Unwound return address is stored in Lr
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
context->Pc = context->Lr;

if (compactEncoding & UNWIND_ARM64_FRAME_X19_X20_PAIR &&
Expand Down Expand Up @@ -1576,10 +1546,7 @@ 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 @@ -1590,11 +1557,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 @@ -1608,6 +1575,11 @@ 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 @@ -1745,12 +1717,6 @@ 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 @@ -2160,33 +2126,6 @@ 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 fc94ec4

Please sign in to comment.