[libunwind] AARCH64 use inline assembly for pointer authentication

[kitaisreal]

Summary

Clang does not support local register variables.

[kitaisreal]

Formatted code using clang-format

[kitaisreal]

Clang does not support local register variables.

[danielkiss]

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

libunwind/src/DwarfInstructions.hpp

225–246

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

[compnerd]

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");

[kitaisreal]

Fixed code review issues.

[MaskRay]

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]

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.

[kitaisreal]

In D102912#2775036, @pcc wrote:

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.

[pcc]

libunwind/src/DwarfInstructions.hpp

242

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

[MaskRay]

libunwind/src/DwarfInstructions.hpp

242

"hint 0xe;" // autib1716

[kitaisreal]

Updated assembly hints to previous revision.

[kitaisreal]

libunwind/src/DwarfInstructions.hpp

242

Updated to previous revision.

242

Updated to previous revision.

[danielkiss]

In D102912#2775303, @kitaisreal wrote:

/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]

In D102912#2785569, @danielkiss wrote:

In D102912#2775303, @kitaisreal wrote:

/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.

[danielkiss]

I think we can drop this change.

[kitaisreal]

@danielkiss it seems that this patch fixes libunwind build for arm for old llvm versions, and also removes dependency on local register variables feature. Should not we apply it ?
Reproducible crash scenario for clang-11.

cat /etc/os-release
NAME="Ubuntu"
VERSION="20.04.4 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.4 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal

uname -a
Linux ip-172-31-37-142 5.13.0-1022-aws #24~20.04.1-Ubuntu SMP Thu Apr 7 22:14:11 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux

arch
aarch64

Download clang-11 using llvm.sh

wget https://apt.llvm.org/llvm.sh
chmod +x llvm.sh
sudo ./llvm.sh 11

clang version

clang++-11 --version
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

/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

Now lets reproduce bug

#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;
}

Compile with clang++-11

/usr/bin/clang++-11 -O0 register_example.cpp -o register_example
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
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/9/../../../../include/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/aarch64-linux-gnu/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/aarch64-linux-gnu/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/c++/9/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 -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -faddrsig -o /tmp/register_example-8aae67.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 0x0000ffff9775a9fc llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/lib/aarch64-linux-gnu/libLLVM-11.so.1+0xa5c9fc)
clang: error: unable to execute command: Segmentation fault (core dumped)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: /tmp/register_example-814e11.cpp
clang: note: diagnostic msg: /tmp/register_example-814e11.sh
clang: note: diagnostic msg: 

********************

Compile with clang-11

/usr/bin/clang-11 -O0 register_example.cpp -o register_example
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script.
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/9/../../../../include/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/aarch64-linux-gnu/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/aarch64-linux-gnu/c++/9 -internal-isystem /usr/bin/../lib/gcc/aarch64-linux-gnu/9/../../../../include/c++/9/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 -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fcxx-exceptions -fexceptions -fcolor-diagnostics -faddrsig -o /tmp/register_example-035369.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 0x0000ffffb54469fc llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/lib/aarch64-linux-gnu/libLLVM-11.so.1+0xa5c9fc)
clang: error: unable to execute command: Segmentation fault (core dumped)
clang: error: clang frontend command failed due to signal (use -v to see invocation)
Ubuntu clang version 11.0.0-2~ubuntu20.04.1
Target: aarch64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: /tmp/register_example-afe67d.cpp
clang: note: diagnostic msg: /tmp/register_example-afe67d.sh
clang: note: diagnostic msg: 

********************

[kitaisreal]

Address code review comments.

[danielkiss]

Patch is only needed due to a clang bug which is fixed already in 11.x releases and TOT. For me the original code looks better and I don't see the need for the patch but I let other from the @libunwind group to disagree with me.

[tschuett]

Could CMake catch this problem? The minimal supported Clang is 12.0?

[ldionne]

Indeed, if this only catches issues on a now-unsupported version of Clang, I think this should be abandoned.

libunwind/src/DwarfInstructions.hpp

static v128 getSavedVectorRegister(A &addressSpace, const R &registers,
                                   pint_t cfa, const RegisterLocation &savedReg);
 
   static pint_t getCFA(A &addressSpace, const PrologInfo &prolog,
                        const R &registers) {
     if (prolog.cfaRegister != 0)
       return (pint_t)((sint_t)registers.getRegister((int)prolog.cfaRegister) +
              prolog.cfaRegisterOffset);
     if (prolog.cfaExpression != 0)
-      return evaluateExpression((pint_t)prolog.cfaExpression, addressSpace, 
+      return evaluateExpression((pint_t)prolog.cfaExpression, addressSpace,
                                 registers, 0);
     assert(0 && "getCFA(): unknown location");
     __builtin_unreachable();
   }
 };
 
 
 template <typename A, typename R>
#if defined(_LIBUNWIND_TARGET_AARCH64)
       // restored. autia1716 is used instead of autia as autia1716 assembles
       // to a NOP on pre-v8.3a architectures.
       if ((R::getArch() == REGISTERS_ARM64) &&
           prolog.savedRegisters[UNW_ARM64_RA_SIGN_STATE].value &&
           returnAddress != 0) {
 #if !defined(_LIBUNWIND_IS_NATIVE_ONLY)
         return UNW_ECROSSRASIGNING;
 #else
-        register unsigned long long x17 __asm("x17") = returnAddress;
-        register unsigned long long x16 __asm("x16") = cfa;
-
         // These are the autia1716/autib1716 instructions. The hint instructions
         // are used here as gcc does not assemble autia1716/autib1716 for pre
         // armv8.3a targets.
+
         if (cieInfo.addressesSignedWithBKey)
-          asm("hint 0xe" : "+r"(x17) : "r"(x16)); // autib1716
+        {
+            asm volatile(
+                "mov x17, %x0;"
+                "mov x16, %x1;"
+                "hint 0xe;" // autib1716
+                "mov %0, x17"
+                : "+r"(returnAddress)
+                : "r"(cfa)
+                : "x16", "x17");
+        }
         else
-          asm("hint 0xc" : "+r"(x17) : "r"(x16)); // autia1716
-        returnAddress = x17;
+        {
+            asm volatile(
+                "mov x17, %x0;"
+                "mov x16, %x1;"
+                "hint 0xc;" // autia1716
+                "mov %0, x17"
+                : "+r"(returnAddress)
+                : "r"(cfa)
+                : "x16", "x17");
+        }
 #endif
       }
 #endif
 
 #if defined(_LIBUNWIND_TARGET_SPARC)
       if (R::getArch() == REGISTERS_SPARC) {
         // Skip call site instruction and delay slot
         returnAddress += 8;
#endif
 
       // Return address is address after call site instruction, so setting IP to
       // that does simualates a return.
       newRegisters.setIP(returnAddress);
 
       // Simulate the step by replacing the register set with the new ones.
       registers = newRegisters;
 
       return UNW_STEP_SUCCESS;
     }
   }
   return UNW_EBADFRAME;
 }
 
 template <typename A, typename R>
 typename A::pint_t
 DwarfInstructions<A, R>::evaluateExpression(pint_t expression, A &addressSpace,