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.

Sign up for GitHub

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

Remove usage of HP libunwind on Apple platforms #110023

Closed
wants to merge 8 commits into from
Closed

Remove usage of HP libunwind on Apple platforms #110023

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5f7063e
WIP: Test sys/ucontext.h
filipnavara Nov 20, 2024
1e24599
Drop HP libunwind fallback on Apple platforms
filipnavara Nov 22, 2024
659046d
Revert accidental changes
filipnavara Nov 22, 2024
5af4acf
Revert one more unnecessary change
filipnavara Nov 22, 2024
378fcbc
:(
filipnavara Nov 22, 2024
6a26378
Fix osx-x64 build
filipnavara Nov 22, 2024
97fb693
Revert sys/ucontext.h change
filipnavara Nov 30, 2024
5a81459
Revert one more change
filipnavara Nov 30, 2024
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
22 changes: 2 additions & 20 deletions src/coreclr/pal/src/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,29 +222,11 @@ add_library(coreclrpal
)

# Build separate pal library for DAC (addition to regular pal library)
if(CLR_CMAKE_TARGET_OSX)
set(LIBUNWIND_DAC_OBJECTS $<TARGET_OBJECTS:libunwind_dac>)

if(NOT FEATURE_CROSSBITNESS)
add_library(coreclrpal_dac STATIC
exception/remote-unwind.cpp
${LIBUNWIND_DAC_OBJECTS}
)

target_include_directories(coreclrpal_dac PUBLIC
${CLR_SRC_NATIVE_DIR}/external/libunwind/include
${CLR_SRC_NATIVE_DIR}/external/libunwind/include/tdep
${CLR_ARTIFACTS_OBJ_DIR}/external/libunwind/include
${CLR_ARTIFACTS_OBJ_DIR}/external/libunwind/include/tdep
)

target_compile_definitions(coreclrpal_dac PUBLIC -DUNW_REMOTE_ONLY)
else()
if(NOT FEATURE_CROSSBITNESS)
add_library(coreclrpal_dac STATIC
exception/remote-unwind.cpp
)
endif(NOT FEATURE_CROSSBITNESS)
endif(CLR_CMAKE_TARGET_OSX)
endif(NOT FEATURE_CROSSBITNESS)

# There is only one function exported in 'tracepointprovider.cpp' namely 'PAL_InitializeTracing',
# which is guarded with '#if defined(__linux__)'. On macOS, Xcode issues the following warning:
Expand Down
71 changes: 17 additions & 54 deletions src/coreclr/pal/src/exception/remote-unwind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -887,8 +887,7 @@ static bool
ExtractProcInfoFromFde(
const libunwindInfo* info,
unw_word_t* addrp,
unw_proc_info_t *pip,
int need_unwind_info)
unw_proc_info_t *pip)
{
unw_word_t addr = *addrp;

Expand Down Expand Up @@ -917,35 +916,6 @@ ExtractProcInfoFromFde(
return false;
}

// Now fill out the proc info if requested
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing this code breaks a fallback path if HAVE_GET_PROC_INFO_IN_RANGE is not set and not HOST_UNIX. I'm not sure exactly what platforms still will use that path (see line 2398).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAVE_GET_PROC_INFO_IN_RANGE is not defined on Unix when libunwind lacks support for unw_get_proc_info_in_range. This function is only available in HP libunwind version 1.6 and later. To ensure compatibility, we need to handle this for LLVM libunwind and HP libunwind versions earlier than 1.6.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a fundamental misunderstanding of what this PR does (or I am missing something).

This code is Apple specific. It was called explicitly on Apple platforms already without any libunwind involvement (aside from operating on the same structures). I completely removed the code path that goes to (HP) libunwind on Apple platforms because there are only two unwinding encodings (compact unwinding and DWARF; with the DWARF always using compact unwinding as index because the linker enforces that) and both are already handled by the special Apple code path.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExtractProcInfoFromFde is not Apple specific. Yes, most of the calls are in Apple specific except here:

runtime/src/coreclr/pal/src/exception/remote-unwind.cpp

Line 2398 in 342936c

if (!ExtractProcInfoFromFde(info, &fdeAddr, pip, need_unwind_info)) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think that code path never takes place with the HP libunwind fork that we ship. It may be relevant for platforms that use system libunwind (FreeBSD or Haiku). I can add that code path back...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, add it back to make sure ever platform is covered.

if (need_unwind_info)
{
if (dci.have_abi_marker)
{
if (!ReadValue16(info, &addr, &dci.abi)) {
return false;
}
if (!ReadValue16(info, &addr, &dci.tag)) {
return false;
}
}
if (dci.sized_augmentation) {
dci.fde_instr_start = augmentationEndAddr;
}
else {
dci.fde_instr_start = addr;
}
dci.fde_instr_end = fdeEndAddr;

pip->format = UNW_INFO_FORMAT_TABLE;
pip->unwind_info_size = sizeof(dci);
pip->unwind_info = malloc(sizeof(dci));
if (pip->unwind_info == nullptr) {
return -UNW_ENOMEM;
}
memcpy(pip->unwind_info, &dci, sizeof(dci));
}

return true;
}

Expand Down Expand Up @@ -1157,8 +1127,7 @@ SearchDwarfSection(
unw_word_t dwarfSectionAddr,
unw_word_t dwarfSectionSize,
uint32_t fdeSectionHint,
unw_proc_info_t *pip,
int need_unwind_info)
unw_proc_info_t *pip)
{
unw_word_t addr = dwarfSectionAddr + fdeSectionHint;
unw_word_t fdeAddr;
Expand All @@ -1175,7 +1144,7 @@ SearchDwarfSection(
}

if (ip >= ipStart && ip < ipEnd) {
if (!ExtractProcInfoFromFde(info, &fdeAddr, pip, need_unwind_info)) {
if (!ExtractProcInfoFromFde(info, &fdeAddr, pip)) {
ERROR("ExtractProcInfoFromFde FAILED for ip %p\n", (void*)ip);
break;
}
Expand All @@ -1188,7 +1157,7 @@ SearchDwarfSection(


static bool
GetProcInfo(unw_word_t ip, unw_proc_info_t *pip, libunwindInfo* info, bool* step, int need_unwind_info)
GetProcInfo(unw_word_t ip, unw_proc_info_t *pip, libunwindInfo* info, bool* step)
{
memset(pip, 0, sizeof(*pip));
*step = false;
Expand Down Expand Up @@ -1278,7 +1247,7 @@ GetProcInfo(unw_word_t ip, unw_proc_info_t *pip, libunwindInfo* info, bool* step
#else
#error unsupported architecture
#endif
if (SearchDwarfSection(info, ip, ehframeSectionAddr, ehframeSectionSize, dwarfOffsetHint, pip, need_unwind_info)) {
if (SearchDwarfSection(info, ip, ehframeSectionAddr, ehframeSectionSize, dwarfOffsetHint, pip)) {
TRACE("SUCCESS: found in eh frame from compact hint for %p\n", (void*)ip);
return true;
}
Expand All @@ -1294,7 +1263,7 @@ GetProcInfo(unw_word_t ip, unw_proc_info_t *pip, libunwindInfo* info, bool* step
// Look in dwarf unwind info next
if (ehframeSectionAddr != 0)
{
if (SearchDwarfSection(info, ip, ehframeSectionAddr, ehframeSectionSize, 0, pip, need_unwind_info)) {
if (SearchDwarfSection(info, ip, ehframeSectionAddr, ehframeSectionSize, 0, pip)) {
TRACE("SUCCESS: found in eh frame for %p\n", (void*)ip);
return true;
}
Expand Down Expand Up @@ -1495,7 +1464,7 @@ StepWithCompactEncodingFrameless(const libunwindInfo* info, compact_unwind_encod
}
}

uint64_t savedRegisters = context->Rsp + stack_size - 8 - (8 * register_count);
unw_word_t savedRegisters = context->Rsp + stack_size - 8 - (8 * register_count);
for (int i = 0; i < register_count; i++)
{
uint64_t reg;
Expand Down Expand Up @@ -2033,6 +2002,7 @@ static void UnwindContextToContext(unw_cursor_t *cursor, CONTEXT *winContext)
#endif
}

#ifndef __APPLE__
static int
get_dyn_info_list_addr(unw_addr_space_t as, unw_word_t *dilap, void *arg)
{
Expand Down Expand Up @@ -2217,14 +2187,6 @@ static int
find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pip, int need_unwind_info, void *arg)
{
auto *info = (libunwindInfo*)arg;
#ifdef __APPLE__
mikem8361 marked this conversation as resolved.
Show resolved Hide resolved
bool step;
if (!GetProcInfo(ip, pip, info, &step, need_unwind_info)) {
return -UNW_EINVAL;
}
_ASSERTE(!step);
return UNW_ESUCCESS;
#else
memset(pip, 0, sizeof(*pip));

Ehdr ehdr;
Expand Down Expand Up @@ -2395,7 +2357,7 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pip, int nee
}

// Now get the unwind info
if (!ExtractProcInfoFromFde(info, &fdeAddr, pip, need_unwind_info)) {
if (!ExtractProcInfoFromFde(info, &fdeAddr, pip)) {
ERROR("ExtractProcInfoFromFde\n");
return -UNW_EINVAL;
}
Expand All @@ -2407,8 +2369,6 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pip, int nee
info->FunctionStart = pip->start_ip;
return UNW_ESUCCESS;
#endif // HAVE_GET_PROC_INFO_IN_RANGE || !defined(HOST_UNIX)

#endif // __APPLE__
}

static void
Expand Down Expand Up @@ -2439,6 +2399,7 @@ static unw_accessors_t init_unwind_accessors()
};

static unw_accessors_t unwind_accessors = init_unwind_accessors();
#endif // __APPLE__

/*++
Function:
Expand Down Expand Up @@ -2476,10 +2437,10 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont
bool step;
#if defined(TARGET_AMD64)
TRACE("Unwind: rip %p rsp %p rbp %p\n", (void*)context->Rip, (void*)context->Rsp, (void*)context->Rbp);
result = GetProcInfo(context->Rip, &procInfo, &info, &step, false);
result = GetProcInfo(context->Rip, &procInfo, &info, &step);
#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);
result = GetProcInfo(context->Pc, &procInfo, &info, &step);
if (result && step)
{
// If the PC is at the start of the function, the previous instruction is BL and the unwind encoding is frameless
Expand All @@ -2498,7 +2459,7 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont
(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);
result = GetProcInfo(context->Pc - 1, &procInfo, &info, &step);
}
else
{
Expand All @@ -2519,8 +2480,7 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont
result = StepWithCompactEncoding(&info, procInfo.format, procInfo.start_ip);
goto exit;
}
#endif

#else
addrSpace = unw_create_addr_space(&unwind_accessors, 0);

st = unw_init_remote(&cursor, addrSpace, &info);
Expand All @@ -2544,16 +2504,19 @@ PAL_VirtualUnwindOutOfProc(CONTEXT *context, KNONVOLATILE_CONTEXT_POINTERS *cont
GetContextPointers(&cursor, NULL, contextPointers);
}
result = TRUE;
#endif // __APPLE__

exit:
if (functionStart)
{
*functionStart = info.FunctionStart;
}
#ifndef __APPLE__
if (addrSpace != 0)
{
unw_destroy_addr_space(addrSpace);
}
#endif // !__APPLE__
return result;
}

Expand Down
Loading