Page MenuHomePhabricator

[AArch64] Add v8.5 Branch Target Identification support.
Needs ReviewPublic

Authored by danielkiss on Apr 9 2020, 2:34 AM.

Details

Reviewers
phosek
tamas.petz
chill
olista01
miyuki
Group Reviewers
Restricted Project
Summary

The .note.gnu.property must be in the assembly file to indicate the
support for BTI otherwise BTI will be disabled for the whole library.
__unw_getcontext and libunwind::Registers_arm64::jumpto() may be called
indirectly therefore they should start with a landing pad.

Diff Detail

Event Timeline

danielkiss created this revision.Apr 9 2020, 2:34 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptApr 9 2020, 2:34 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Please help me how to add a test for it, in the current state this patch won't do anything because the standard build for libunwind does not add -mbranch-protection=standard (or bti).

chill added inline comments.Apr 9 2020, 2:53 AM
libunwind/src/assembly.h
47

This will be emitted for all functions, but we need only one per object file, isn't it ?

Also, could you, please, rename PPC64OPD{1,2} to TARGET_OPD{1,2}, or {ARCH ...}, something like that.

miyuki added a comment.Apr 9 2020, 4:23 AM

Please help me how to add a test for it, in the current state this patch won't do anything because the standard build for libunwind does not add -mbranch-protection=standard (or bti).

I suggest adding a CMake variable to configure the branch protection option used when building the C++ libraries

scw added a subscriber: scw.Apr 9 2020, 4:11 PM
MaskRay added a subscriber: MaskRay.Apr 9 2020, 5:27 PM
MaskRay added inline comments.
libunwind/src/assembly.h
46

Just use defined(__aarch64__). I deleted __arm64__ in D77829

48

I am worried that we may want to change the section type for .note.gnu.property

https://sourceware.org/pipermail/binutils/2020-April/000372.html

danielkiss marked 4 inline comments as done.Apr 14 2020, 3:10 PM

Please help me how to add a test for it, in the current state this patch won't do anything because the standard build for libunwind does not add -mbranch-protection=standard (or bti).

I suggest adding a CMake variable to configure the branch protection option used when building the C++ libraries

Thanks, it will be a new patch.

libunwind/src/assembly.h
47

Yes, only one function per file today, but let's add the section per include.

Rename is done.

48

Thanks for the heads-up, I'm not on that mailing list...

Mostly good apart from the renaming issue and a nit about comments. I have some patches in the BTI/PAC area, so I am confident.

libunwind/src/assembly.h
47

@chill
Please don't rename OPD{1,2}. They are really specific to PowerPC ELFv1 function descriptors. No other architecture needs them.

53

The comments can be added right after .long

MaskRay added inline comments.Apr 14 2020, 3:34 PM
libunwind/src/assembly.h
62

This really needs to use a new macro.

I expect a __ARM_FEATURE_BTI_DEFAULT toolchain capable of assembling bti c

chill added inline comments.Apr 15 2020, 1:10 AM
libunwind/src/assembly.h
163

All right, of these are to stay named PPC64_OPD, then just add a new line with, say, AARCH64_BTI, e..g

#define DEFINE_LIBUNWIND_FUNCTION(name)                                        \
.globl SYMBOL_NAME(name) SEPARATOR                                           \
HIDDEN_SYMBOL(SYMBOL_NAME(name)) SEPARATOR                                   \
SYMBOL_IS_FUNC(SYMBOL_NAME(name)) SEPARATOR                                  \
PPC64_OPD1                                                                    \ 
SYMBOL_NAME(name):                                                           \
PPC64_OPD2                                                                   \
AARCH64_BTI
danielkiss marked an inline comment as done.
danielkiss marked 3 inline comments as done.Apr 15 2020, 8:58 AM
danielkiss added inline comments.
libunwind/src/assembly.h
62

Expectation is right, D71658 broke it, I will take a look.

danielkiss marked an inline comment as done.
danielkiss marked 2 inline comments as done.Apr 15 2020, 9:03 AM
danielkiss added inline comments.
libunwind/src/assembly.h
53

Missed this comment in the previous version.