Page MenuHomePhabricator

[libunwind] AARCH64 use inline assembly for pointer authentication
AcceptedPublic

Authored by kitaisreal on May 21 2021, 3:48 AM.

Details

Reviewers
danielkiss
compnerd
Group Reviewers
Restricted Project
Summary

Clang does not support local register variables.

Diff Detail

Event Timeline

kitaisreal created this revision.May 21 2021, 3:48 AM
kitaisreal requested review of this revision.May 21 2021, 3:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 3:48 AM

Formatted code using clang-format

Clang does not support local register variables.

LGTM just please run once more git clang-format HEAD^1 on the patch.

libunwind/src/DwarfInstructions.hpp
225–247

the code style is that { in the same line with the if statement.

compnerd added inline comments.
libunwind/src/DwarfInstructions.hpp
228

Could you use \n\t instead of the ; for the inline assembly please?

230

I wonder if this would be easier to spot with a constant being passed in as:

static const uint16_t autia1716 = 0xc;
static const uint16_t autib1716 = 0xe;
__asm__ __volatile__ ("mov x17, %x0\n\t"
                      "mov x16, %x1\n\t"
                      "hint %[key]\n\t"
                      "mov %0, x17\n\t"
                      : "+r" (ra)
                      : "r" (cfa), [key] "i" (autib1716)
                      : "x16", "x17");

Fixed code review issues.

kitaisreal marked 3 inline comments as done.May 21 2021, 2:31 PM

Clang does not support local register variables.

https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html is supported but I am sure there can be weak points (especially for some older releases). So avoiding the feature sounds good for portability.

libunwind/src/DwarfInstructions.hpp
242

Just use hint 0xe?

pcc added a subscriber: pcc.May 21 2021, 4:44 PM

Which version of Clang are you using? I'm reasonably sure that we're building libunwind on Android with Clang, and the pointer authentication code works there.

Which version of Clang are you using? I'm reasonably sure that we're building libunwind on Android with Clang, and the pointer authentication code works there.

I tried to make native AARCH64 build with clang-11.
I am using clang-11, native build on AARCH64, there was a crash during O0 build. Let me show example:

#include <stdint.h>

uintptr_t __attribute__ ((noinline)) authenticatePtrKeyB(uintptr_t pointer, uintptr_t modifier)
{
    register unsigned long long x17 __asm("x17") = pointer;
    register unsigned long long x16 __asm("x16") = modifier;

    asm volatile("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
    pointer = x17;

    return pointer;
}

int main(int argc, char **argv)
{
    uintptr_t value_to_take = 5;
    uintptr_t ptr_to_value = reinterpret_cast<uintptr_t>(&value_to_take);
    authenticatePtrKeyB(ptr_to_value, 3);

    return 0;
}
/usr/bin/clang-11 -O0 register_example.cpp -o register_example

Stack dump:
0.	Program arguments: /usr/lib/llvm-11/bin/clang -cc1 -triple aarch64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name register_example.cpp -mrelocation-model static -mframe-pointer=non-leaf -fmath-errno -fno-rounding-math -mconstructor-aliases -target-cpu generic -target-feature +neon -target-abi aapcs -fallow-half-arguments-and-returns -fno-split-dwarf-inlining -debugger-tuning=gdb -resource-dir /usr/lib/llvm-11/lib/clang/11.0.0 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/10/../../../../include/c++/10 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/10/../../../../include/aarch64-linux-gnu/c++/10 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/10/../../../../include/aarch64-linux-gnu/c++/10 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/10/../../../../include/c++/10/backward -internal-isystem /usr/local/include -internal-isystem /usr/lib/llvm-11/lib/clang/11.0.0/include -internal-externc-isystem /usr/include/aarch64-linux-gnu -internal-externc-isystem /include -internal-externc-isystem /usr/include -O0 -fdeprecated-macro -fdebug-compilation-dir /home/ubuntu/test -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -faddrsig -o /tmp/register_example-0ba515.o -x c++ register_example.cpp
1.	<eof> parser at end of file
2.	Code generation
3.	Running pass 'Function Pass Manager' on module 'register_example.cpp'.
4.	Running pass 'RegBankSelect' on function '@_Z19authenticatePtrKeyBmm'
#0 0x0000ffff8f9809fc llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/lib/aarch64-linux-gnu/libLLVM-11.so.1+0xa5c9fc)

If I compile this file using O2 it will work and in objdump code for function is correctly generated.

0000000000400544 <_Z19authenticatePtrKeyBmm>:
  400544:       aa0003f1        mov     x17, x0
  400548:       aa0103f0        mov     x16, x1
  40054c:       d50321df        autib1716
  400550:       aa1103e0        mov     x0, x17
  400554:       d65f03c0        ret
/usr/bin/clang-11 --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

I checked following links:

  1. https://clang.llvm.org/docs/UsersManual.html#gcc-extensions-not-implemented-yet.
  2. It is for other libunwind implementation https://github.com/libunwind/libunwind/issues/215

Seems like it is better to not use partially supported gcc extensions.

libunwind/src/DwarfInstructions.hpp
242

Discussed in previous comments that it make sense to be more specific and use constant.

kitaisreal marked an inline comment as done.May 24 2021, 2:36 AM
compnerd accepted this revision.May 26 2021, 8:38 AM
This revision is now accepted and ready to land.May 26 2021, 8:38 AM
pcc added inline comments.May 26 2021, 10:36 AM
libunwind/src/DwarfInstructions.hpp
242

(I would vote for the way you had this originally, FWIW.)

MaskRay added inline comments.May 26 2021, 11:03 AM
libunwind/src/DwarfInstructions.hpp
242

"hint 0xe;" // autib1716

Updated assembly hints to previous revision.

kitaisreal marked 2 inline comments as done.May 26 2021, 1:53 PM
kitaisreal added inline comments.
libunwind/src/DwarfInstructions.hpp
242

Updated to previous revision.

242

Updated to previous revision.

/usr/bin/clang-11 --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1

Seems it is fixed in 11.0.1 and trunk or at least I was not able to reproduce the crash, codegen looks fine.

kitaisreal marked an inline comment as done.May 28 2021, 12:54 PM

/usr/bin/clang-11 --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1

Seems it is fixed in 11.0.1 and trunk or at least I was not able to reproduce the crash, codegen looks fine.

Yes also could not reproduce it.