This is an archive of the discontinued LLVM Phabricator instance.

[libunwind][PPC64] Port to ppc64le - initial version
ClosedPublic

Authored by luporl on Dec 19 2017, 3:55 AM.

Details

Summary

Initial working version of libunwind for PowerPC 64. Tested on little-endian ppc64 host only.
Based on the existing PowerPC 32 code.

It supports:

  • context save/restore (unw_getcontext, unw_init_local, unw_resume)
  • read/write from/to saved registers
  • backtrace (unw_step)

The main changes were:

  • implement ppc64 registers
  • add save/restore ppc64 registers code, except for vector ones (this is the next planned step)
  • change register names from rXX to %rXX. The built-in assembler of clang, on powerpc64le on Linux, doesn't seem to recognize the former format. Checked that gcc/as also understand this format

This is the continuation of the work started here:
https://github.com/rgdoliveira/libunwind

Note: The commits were squashed to remove test and changed code and present a clean history.

Diff Detail

Repository
rL LLVM

Event Timeline

luporl created this revision.Dec 19 2017, 3:55 AM
luporl edited the summary of this revision. (Show Details)
luporl edited the summary of this revision. (Show Details)
mstorsjo added inline comments.Dec 19 2017, 5:42 AM
include/__libunwind_config.h
46 ↗(On Diff #127481)

Don't hardcode a number here; add a define _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC64 further above like the other architectures

src/Registers.hpp
1128 ↗(On Diff #127481)

... and use _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC64 here instead of a hardcoded number

src/libunwind.cpp
84 ↗(On Diff #127481)

Why this comment here? Remote unwinding is unimplemented in libunwind, and I don't see how a ppc64 specific comment is needed here?

luporl updated this revision to Diff 127516.Dec 19 2017, 6:11 AM

Addressed review comments.

No further comments from my side, but it'd be good if @compnerd could have a look as well.

luporl marked 3 inline comments as done.Dec 19 2017, 6:20 AM
luporl added inline comments.
src/libunwind.cpp
84 ↗(On Diff #127481)

Right, I thought that maybe only ppc64 had to be added here, but as remote unwinding is unimplemented there is really no need for a ppc64 specific comment here.

compnerd accepted this revision.Dec 19 2017, 11:28 AM
This revision is now accepted and ready to land.Dec 19 2017, 11:28 AM
luporl marked an inline comment as done.Jan 2 2018, 5:03 AM

@compnerd, @mstorsjo, I don't have permission to commit to the repository. Could you check in the changes for me?

This revision was automatically updated to reflect the committed changes.
timshen added a subscriber: timshen.Jan 2 2018, 1:57 PM

The static_assert on line 1189 fails with the following build on x86:

cmake -GNinja -DLIBUNWIND_ENABLE_CROSS_UNWINDING=ON -DLLVM_PATH=$PATH_TO_LLVM $PATH_TO_LIBUNWIND && ninja libunwind.a

It seems that 136 in # define _LIBUNWIND_CONTEXT_SIZE 136 is too small. Reverting the patch for now.

The static_assert on line 1189 fails with the following build on x86:

cmake -GNinja -DLIBUNWIND_ENABLE_CROSS_UNWINDING=ON -DLLVM_PATH=$PATH_TO_LLVM $PATH_TO_LIBUNWIND && ninja libunwind.a

It seems that 136 in # define _LIBUNWIND_CONTEXT_SIZE 136 is too small. Reverting the patch for now.

Actually, I can fix this directly without having to revert, if you give me a couple minutes.

The static_assert on line 1189 fails with the following build on x86:

cmake -GNinja -DLIBUNWIND_ENABLE_CROSS_UNWINDING=ON -DLLVM_PATH=$PATH_TO_LLVM $PATH_TO_LIBUNWIND && ninja libunwind.a

It seems that 136 in # define _LIBUNWIND_CONTEXT_SIZE 136 is too small. Reverting the patch for now.

Actually, I can fix this directly without having to revert, if you give me a couple minutes.

Ah sorry, didn't see this earlier. It's already reverted. :(