This is an archive of the discontinued LLVM Phabricator instance.

Differential D118320

[libunwind] build fix for GCC on PPC32
ClosedPublic

Authored by thesamesam on Jan 26 2022, 11:36 PM.

Details

Reviewers
MaskRay
mgorny
Group Reviewers
Restricted Project
Commits
rGcd20e579df07: [unwind] fix build with GCC on PPC32
Summary

Originally reported downstream in Gentoo here.

/var/tmp/portage/sys-libs/llvm-libunwind-13.0.0/work/libunwind/src/libunwind.cpp:77:3: error: #error Architecture not supported
   77 | # error Architecture not supported
      |   ^~~~~
[...]
/var/tmp/portage/sys-libs/llvm-libunwind-13.0.0/work/libunwind/src/libunwind.cpp: In function ‘int __unw_init_local(unw_cursor_t*, unw_context_t*)’:
/var/tmp/portage/sys-libs/llvm-libunwind-13.0.0/work/libunwind/src/libunwind.cpp:80:57: error: ‘REGISTER_KIND’ was not declared in this scope
   80 |   new (reinterpret_cast<UnwindCursor<LocalAddressSpace, REGISTER_KIND> *>(cursor))
      |                                                         ^~~~~~~~~~~~~
[...]

PPC is actually a supported architecture, but GCC (tested with 11.2.0) on powerpc32 seems to only define:
__PPC__, _ARCH_PPC, __PPC, __powerpc and not __ppc__.

This instead uses __powerpc__ which should be around on PPC32 and PPC64 (but we check it after PPC64, so it's fine).

Diff Detail

Event Timeline

thesamesam created this revision.Jan 26 2022, 11:36 PM
Herald added subscribers: steven.zhang, shchenz. · View Herald TranscriptJan 26 2022, 11:36 PM
thesamesam requested review of this revision.Jan 26 2022, 11:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2022, 11:36 PM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B145928: Diff 403512.Jan 26 2022, 11:36 PM
thesamesam updated this revision to Diff 403515.Jan 26 2022, 11:46 PM
Arfrever added a subscriber: Arfrever.Jan 27 2022, 12:20 AM
Arfrever added inline comments.
libunwind/include/__libunwind_config.h
58

It would probably suffice:

# elif defined(__powerpc__)

Similarly in other places.

libunwind/src/assembly.h
219
#if defined(__powerpc__) || defined(__powerpc64__)
MaskRay added a comment.Jan 27 2022, 12:48 AM

In the future, please include more context in the diff :)

https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface "To make reviews easier, please always include as much context as possible with your diff!"

libunwind/include/__libunwind_config.h
58

__powerpc__ sounds good.

Both ppc32 and ppc64 define __powerpc__ and __PPC__. Can you also check whether this works for powerpc32?

MaskRay retitled this revision from llvm-libunwind build fix for GCC on PPC32 to [libunwind] build fix for GCC on PPC32.EditedJan 27 2022, 12:49 AM

__PPC__, _ARCH_PPC, __PPC, __powerpc and not __ppc__.

WoW, this looks like a Clang problem...

In GCC, __ppc__ is defined for FreeBSD/Darwin/etc but not Linux. Clang makes it apply to other systems.

Harbormaster completed remote builds in B145929: Diff 403515.Jan 27 2022, 8:42 AM
thesamesam updated this revision to Diff 403752.Jan 27 2022, 12:17 PM
thesamesam edited the summary of this revision. (Show Details)
thesamesam marked 3 inline comments as done.
MaskRay accepted this revision.Jan 27 2022, 12:22 PM

Thanks!

This revision is now accepted and ready to land.Jan 27 2022, 12:22 PM
thesamesam updated this revision to Diff 403755.Jan 27 2022, 12:30 PM
Harbormaster completed remote builds in B146105: Diff 403755.Jan 27 2022, 12:31 PM
mgorny accepted this revision.Jan 27 2022, 1:13 PM
mgorny added a subscriber: mgorny.

LGTM. Thanks.

This revision was landed with ongoing or failed builds.Jan 27 2022, 2:05 PM
Closed by commit rGcd20e579df07: [unwind] fix build with GCC on PPC32 (authored by thesamesam, committed by mgorny). · Explain Why
This revision was automatically updated to reflect the committed changes.
mgorny added a commit: rGcd20e579df07: [unwind] fix build with GCC on PPC32.
Herald added a project: Restricted Project. · View Herald TranscriptJan 27 2022, 2:05 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
ldionne added a subscriber: ldionne.Jan 27 2022, 2:50 PM

@mgorny @MaskRay Please consider adding yourselves to the libunwind review groups if you aren't, and accepting as the group when a patch can be shipped. There would be a lot of value in having folks "own" libunwind more officially. Cheers and thanks for reviewing.

MaskRay added a comment.Jan 27 2022, 3:00 PM
In D118320#3277666, @ldionne wrote:

@mgorny @MaskRay Please consider adding yourselves to the libunwind review groups if you aren't, and accepting as the group when a patch can be shipped. There would be a lot of value in having folks "own" libunwind more officially. Cheers and thanks for reviewing.

Thanks. I am in #libunwind:) I have should added #libunwind when I saw this.

Arfrever mentioned this in D119068: [libunwind] Define _Unwind_Backtrace for powerpc, sparc.Feb 5 2022, 10:39 PM
MaskRay mentioned this in rG022011078054: [libunwind] Define _Unwind_Backtrace for powerpc, sparc.Feb 7 2022, 9:37 AM

Revision Contents

PathSize
libunwind/
include/
2 lines
src/
2 lines
4 lines
2 lines
2 lines
2 lines

Diff 403794

libunwind/include/__libunwind_config.h

Show First 20 Lines • Show All 49 Lines • ▼ Show 20 Lines
# define _LIBUNWIND_CURSOR_SIZE 33 # define _LIBUNWIND_CURSOR_SIZE 33
# endif # endif
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86_64 # define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_X86_64
# elif defined(__powerpc64__) # elif defined(__powerpc64__)
# define _LIBUNWIND_TARGET_PPC64 1 # define _LIBUNWIND_TARGET_PPC64 1
# define _LIBUNWIND_CONTEXT_SIZE 167 # define _LIBUNWIND_CONTEXT_SIZE 167
# define _LIBUNWIND_CURSOR_SIZE 179 # define _LIBUNWIND_CURSOR_SIZE 179
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC64 # define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC64
# elif defined(__ppc__) # elif defined(__powerpc__)
ArfreverUnsubmitted
Done
ReplyInline Actions

It would probably suffice:

# elif defined(__powerpc__)

Similarly in other places.

Arfrever: It would probably suffice: ``` # elif defined(__powerpc__) ``` Similarly in other places.
MaskRayUnsubmitted
Done
ReplyInline Actions

__powerpc__ sounds good.

Both ppc32 and ppc64 define __powerpc__ and __PPC__. Can you also check whether this works for powerpc32?

MaskRay: `__powerpc__` sounds good. Both ppc32 and ppc64 define `__powerpc__` and `__PPC__`. Can you…
# define _LIBUNWIND_TARGET_PPC 1 # define _LIBUNWIND_TARGET_PPC 1
# define _LIBUNWIND_CONTEXT_SIZE 117 # define _LIBUNWIND_CONTEXT_SIZE 117
# define _LIBUNWIND_CURSOR_SIZE 124 # define _LIBUNWIND_CURSOR_SIZE 124
# define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC # define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
# elif defined(__aarch64__) # elif defined(__aarch64__)
# define _LIBUNWIND_TARGET_AARCH64 1 # define _LIBUNWIND_TARGET_AARCH64 1
# define _LIBUNWIND_CONTEXT_SIZE 66 # define _LIBUNWIND_CONTEXT_SIZE 66
# if defined(__SEH__) # if defined(__SEH__)
▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines

libunwind/src/UnwindRegistersRestore.S

Show First 20 Lines • Show All 388 Lines • ▼ Show 20 LinesLnovec:
PPC64_LR(0) PPC64_LR(0)
PPC64_LR(5) PPC64_LR(5)
PPC64_LR(4) PPC64_LR(4)
PPC64_LR(1) PPC64_LR(1)
PPC64_LR(3) PPC64_LR(3)
bctr bctr
#elif defined(__ppc__) #elif defined(__powerpc__)
DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind13Registers_ppc6jumptoEv) DEFINE_LIBUNWIND_FUNCTION(_ZN9libunwind13Registers_ppc6jumptoEv)
// //
// void libunwind::Registers_ppc::jumpto() // void libunwind::Registers_ppc::jumpto()
// //
// On entry: // On entry:
// thread_state pointer is in r3 // thread_state pointer is in r3
// //
▲ Show 20 Lines • Show All 768 Lines • Show Last 20 Lines

libunwind/src/UnwindRegistersSave.S

Show First 20 Lines • Show All 320 Lines • ▼ Show 20 Lines
# #
# extern int __unw_getcontext(unw_context_t* thread_state) # extern int __unw_getcontext(unw_context_t* thread_state)
# #
# Just trap for the time being. # Just trap for the time being.
DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext) DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
teq $0, $0 teq $0, $0
#elif defined(__powerpc64__) #elif defined(__powerpc__)
// //
// extern int __unw_getcontext(unw_context_t* thread_state) // extern int __unw_getcontext(unw_context_t* thread_state)
// //
// On entry: // On entry:
// thread_state pointer is in r3 // thread_state pointer is in r3
// //
DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext) DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
▲ Show 20 Lines • Show All 216 Lines • ▼ Show 20 Lines
#endif #endif
#endif #endif
li 3, 0 // return UNW_ESUCCESS li 3, 0 // return UNW_ESUCCESS
blr blr
#elif defined(__ppc__) #elif defined(__powerpc__)
// //
// extern int unw_getcontext(unw_context_t* thread_state) // extern int unw_getcontext(unw_context_t* thread_state)
// //
// On entry: // On entry:
// thread_state pointer is in r3 // thread_state pointer is in r3
// //
DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext) DEFINE_LIBUNWIND_FUNCTION(__unw_getcontext)
▲ Show 20 Lines • Show All 547 Lines • Show Last 20 Lines

libunwind/src/assembly.h

Show First 20 Lines • Show All 210 Lines • ▼ Show 20 Lines
#define DEFINE_LIBUNWIND_FUNCTION(name) \ #define DEFINE_LIBUNWIND_FUNCTION(name) \
.globl SYMBOL_NAME(name) SEPARATOR \ .globl SYMBOL_NAME(name) SEPARATOR \
HIDDEN_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \ HIDDEN_SYMBOL(SYMBOL_NAME(name)) SEPARATOR \
SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \ SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR \
PPC64_OPD1 \ PPC64_OPD1 \
SYMBOL_NAME(name): \ SYMBOL_NAME(name): \
PPC64_OPD2 \ PPC64_OPD2 \
AARCH64_BTI AARCH64_BTI
ArfreverUnsubmitted
Done
ReplyInline Actions
#if defined(__powerpc__) || defined(__powerpc64__)
Arfrever: ``` #if defined(__powerpc__) || defined(__powerpc64__) ```
#if defined(__arm__) #if defined(__arm__)
#if !defined(__ARM_ARCH) #if !defined(__ARM_ARCH)
#define __ARM_ARCH 4 #define __ARM_ARCH 4
#endif #endif
#if defined(__ARM_ARCH_4T__) || __ARM_ARCH >= 5 #if defined(__ARM_ARCH_4T__) || __ARM_ARCH >= 5
#define ARM_HAS_BX #define ARM_HAS_BX
#endif #endif
#ifdef ARM_HAS_BX #ifdef ARM_HAS_BX
#define JMP(r) bx r #define JMP(r) bx r
#else #else
#define JMP(r) mov pc, r #define JMP(r) mov pc, r
#endif #endif
#endif /* __arm__ */ #endif /* __arm__ */
#if defined(__ppc__) || defined(__powerpc64__) #if defined(__powerpc__)
#define PPC_LEFT_SHIFT(index) << (index) #define PPC_LEFT_SHIFT(index) << (index)
#endif #endif
#endif /* UNWIND_ASSEMBLY_H */ #endif /* UNWIND_ASSEMBLY_H */

libunwind/src/config.h

Show First 20 Lines • Show All 99 Lines • ▼ Show 20 Lines
#endif #endif
// Apple/armv7k defaults to DWARF/Compact unwinding, but its libunwind also // Apple/armv7k defaults to DWARF/Compact unwinding, but its libunwind also
// needs to include the SJLJ APIs. // needs to include the SJLJ APIs.
#if (defined(__APPLE__) && defined(__arm__)) || defined(__USING_SJLJ_EXCEPTIONS__) #if (defined(__APPLE__) && defined(__arm__)) || defined(__USING_SJLJ_EXCEPTIONS__)
#define _LIBUNWIND_BUILD_SJLJ_APIS #define _LIBUNWIND_BUILD_SJLJ_APIS
#endif #endif
#if defined(__i386__) || defined(__x86_64__) || defined(__ppc__) || defined(__ppc64__) || defined(__powerpc64__) #if defined(__i386__) || defined(__x86_64__) || defined(__powerpc__)
#define _LIBUNWIND_SUPPORT_FRAME_APIS #define _LIBUNWIND_SUPPORT_FRAME_APIS
#endif #endif
#if defined(__i386__) || defined(__x86_64__) || \ #if defined(__i386__) || defined(__x86_64__) || \
defined(__ppc__) || defined(__ppc64__) || defined(__powerpc64__) || \ defined(__ppc__) || defined(__ppc64__) || defined(__powerpc64__) || \
(!defined(__APPLE__) && defined(__arm__)) || \ (!defined(__APPLE__) && defined(__arm__)) || \
defined(__aarch64__) || \ defined(__aarch64__) || \
defined(__mips__) || \ defined(__mips__) || \
▲ Show 20 Lines • Show All 125 Lines • Show Last 20 Lines

libunwind/src/libunwind.cpp

Show First 20 Lines • Show All 45 Lines • ▼ Show 20 Lines _LIBUNWIND_TRACE_API("__unw_init_local(cursor=%p, context=%p)",
static_cast<void *>(cursor), static_cast<void *>(cursor),
static_cast<void *>(context)); static_cast<void *>(context));
#if defined(__i386__) #if defined(__i386__)
# define REGISTER_KIND Registers_x86 # define REGISTER_KIND Registers_x86
#elif defined(__x86_64__) #elif defined(__x86_64__)
# define REGISTER_KIND Registers_x86_64 # define REGISTER_KIND Registers_x86_64
#elif defined(__powerpc64__) #elif defined(__powerpc64__)
# define REGISTER_KIND Registers_ppc64 # define REGISTER_KIND Registers_ppc64
#elif defined(__ppc__) #elif defined(__powerpc__)
# define REGISTER_KIND Registers_ppc # define REGISTER_KIND Registers_ppc
#elif defined(__aarch64__) #elif defined(__aarch64__)
# define REGISTER_KIND Registers_arm64 # define REGISTER_KIND Registers_arm64
#elif defined(__arm__) #elif defined(__arm__)
# define REGISTER_KIND Registers_arm # define REGISTER_KIND Registers_arm
#elif defined(__or1k__) #elif defined(__or1k__)
# define REGISTER_KIND Registers_or1k # define REGISTER_KIND Registers_or1k
#elif defined(__hexagon__) #elif defined(__hexagon__)
▲ Show 20 Lines • Show All 308 Lines • Show Last 20 Lines