compiler-rt: Introduce runtime functions for emulated PAC. #133530

pcc · 2025-03-28T22:33:12Z

The emulated PAC runtime functions emulate the ARMv8.3a pointer
authentication instructions and are intended for use in heterogeneous
testing environments. For more information, see the associated RFC:
https://discourse.llvm.org/t/rfc-emulated-pac/85557

Created using spr 1.3.6-beta.1

github-actions · 2025-03-28T22:38:04Z

✅ With the latest revision this PR passed the undef deprecator.

kbeyls · 2025-03-29T06:56:45Z

compiler-rt/lib/builtins/xxhash.h

+ * Header File
+ * Copyright (C) 2012-2023 Yann Collet
+ *
+ * BSD 2-Clause License (https://www.opensource.org/licenses/bsd-license.php)


This is a license different from Apache-2.0 WITH LLVM-exception.
Therefore, the process described at https://llvm.org/docs/DeveloperPolicy.html#copyright-license-and-patents should be followed to check whether this is acceptable in this specific case.

That being said, xxhash is already present under llvm/lib/Support/xxhash.cpp, as you pointed out in the RFC.

Making sure we don't have multiple copies of non-Apache-2.0 WITH LLVM-exception code would be preferable. I'll tag @beanz, as he had ideas about how to better structure vendored third party code in LLVM.

I'm not sure if moving non-Apache-2.0 WITH LLVM-exception licensed code to a run-time library (for the first time?) triggers new concerns.

I'll note that other hashing algorithms, such as Blake3 and SipHash, which are available under the Apache-2.0 WITH LLVM-exception, are also already present in the LLVM Support library.
Would one of these preferably-licensed hashing algorithms be a good fit for the hashing functionality needed for this use case?

Okay, I will contact [email protected] about this new usage.

I don't have a strong opinion about the hash algorithm. XXH3_64 was chosen for these reasons:

It was the easiest to incorporate into compiler-rt as it was already available as a header-only library without dependencies.

It was already being used in LLVM, so I imagined that licensing wouldn't be as much of a concern.

It was faster than the other algorithms according to the xxhash homepage. BLAKE3 isn't listed there but we can extrapolate from BLAKE3's claim of being ~7x faster than BLAKE2. (I know I said elsewhere that performance is less of a concern for these functions, but all other things being equal, we may as well go with the algorithm with the best performance.)

If the board decides not to allow use in compiler-rt it should be possible to switch to one of the other algorithms after changing its code as needed to remove dependencies.

The introduction of BSD-licensed code to compiler-rt (and especially to compiler-rt's builtins library) poses significant licensing difficulty.

The BSD 2-clause license requires attribution when code is distributed in object form as well as source form, and since the compiler-rt builtins are linked into binaries built with LLVM (not just LLVM itself), the impact of the license here would be transitive to products built using LLVM-based compilers.

This is extremely undesirable, and as such this change will need to replace the xxhash with an alternative hash (ideally under the LLVM license) in order to be integrated.

Understood, I've uploaded a new change that replaces the hash with SipHash.

Unfortunately SipHash is subtantially slower than xxhash and led to a ~3.5x slowdown (vs native PAC instructions) on the benchmark cited in my original RFC (and BLAKE3 isn't header only which would make it substantially harder to integrate here). Hopefully the performance hit doesn't turn out to be a problem here.

kbeyls · 2025-03-29T06:56:55Z

compiler-rt/lib/builtins/aarch64/emupac.c

@@ -0,0 +1,115 @@
+#include <stdint.h>


This new file needs a license header, I guess?

Correct, I forgot to add one. Will add.

jroelofs · 2025-03-31T16:11:26Z

compiler-rt/lib/builtins/aarch64/emupac.c

+}
+
+uint64_t __emupac_autda_impl(uint64_t ptr, uint64_t disc) {
+  if (pac_supported()) {


Might be worth writing a lazy ifunc-like resolver for this so it doesn't have to be checked on every call. Something like:

typedef BOOL (*fptr_ty)(void); static BOOL resolver(void); // Should be signed with address diversity fptr_ty __emupac_autda_impl = resolver; static BOOL resolver(void) { if (pac_supported()) __emupac_autda_impl = __emupac_autda_pac; else __emupac_autda_impl = __emupac_autda_nopac; return __emupac_autda_impl(); }

If we had FMV support for PAC I'd suggest __attribute__((target_version("pac"))). cc @labrinea

You might also want to consider giving __emupac_autda a preserves-none calling convention, so you don't have to save/restore around them.

The problem with using ifuncs here is that this function will itself be called from ifunc resolvers (see this), so that could lead to order of initialization issues.

Since performance is not that important for this function's use case I didn't try to avoid checking every time or experiment with other calling conventions. Also, if compiler-rt needs to be buildable with non-Clang compilers, I'm not sure that we can use the other calling conventions anyway.

The choice of calling convention to use will probably affect register allocation significantly in the caller functions. I wonder if it could hide some subtle codegen issues. On the other hand, it may uncover other ones :)

We haven't noticed codegen issues like this in our internal testing (other than the performance hit which is expected).

llvm-beanz

We cannot introduce BSD-licensed code to compiler-rt. Please see my comment for more details.

Created using spr 1.3.6-beta.1

github-actions · 2025-04-03T04:37:52Z

✅ With the latest revision this PR passed the C/C++ code formatter.

pcc · 2025-04-03T04:43:19Z

We cannot introduce BSD-licensed code to compiler-rt. Please see my comment for more details.

Thanks, resolved by switching to SipHash. #134197 extracts the existing SipHash implementation into a header that can be included here.

The emulated PAC runtime functions emulate the ARMv8.3a pointer authentication instructions and are intended for use in heterogeneous testing environments. For more information, see the associated RFC: https://discourse.llvm.org/t/rfc-emulated-pac/85557 TODO: - Add tests. - Figure out how to get emupac.cpp to build with CMake. For some reason any .cpp files added to the builtins library are ignored. So for now only the gn build works. Pull Request: llvm#133530

Created using spr 1.3.6-beta.1

The emulated PAC runtime functions emulate the ARMv8.3a pointer authentication instructions and are intended for use in heterogeneous testing environments. For more information, see the associated RFC: https://discourse.llvm.org/t/rfc-emulated-pac/85557 TODO: - Add tests. Pull Request: llvm#133530

Created using spr 1.3.6-beta.1

The emulated PAC runtime functions emulate the ARMv8.3a pointer authentication instructions and are intended for use in heterogeneous testing environments. For more information, see the associated RFC: https://discourse.llvm.org/t/rfc-emulated-pac/85557 Pull Request: llvm#133530

atrosinenko · 2025-05-26T11:55:39Z

compiler-rt/cmake/Modules/AddCompilerRT.cmake

@@ -172,7 +172,7 @@ function(add_compiler_rt_runtime name type)
  cmake_parse_arguments(LIB
    ""
    "PARENT_TARGET"
-    "OS;ARCHS;SOURCES;CFLAGS;LINK_FLAGS;DEFS;DEPS;LINK_LIBS;OBJECT_LIBS;ADDITIONAL_HEADERS;EXTENSIONS"
+    "OS;ARCHS;SOURCES;CFLAGS;LINK_FLAGS;DEFS;DEPS;LINK_LIBS;OBJECT_LIBS;ADDITIONAL_HEADERS;EXTENSIONS;C_STANDARD;CXX_STANDARD"


(C|CXX)_STANDARD should probably go to <one_value_keywords> argument of cmake_parse_arguments, same as PARENT_TARGET.

[nit] The new options are not mentioned in the above comment (the description of add_compiler_rt_runtime).

atrosinenko · 2025-05-26T13:30:15Z

compiler-rt/lib/builtins/aarch64/emupac.cpp

+const uint64_t kMaxVASize = 48;
+const uint64_t kPACMask = ((1ULL << 55) - 1) & ~((1ULL << kMaxVASize) - 1);
+const uint64_t kTTBR1Mask = 1ULL << 55;


[nit] According to LLVM naming convention, this probably should be simply MaxVASize, PACMask, TTBR1Mask.

atrosinenko · 2025-05-26T13:48:45Z

compiler-rt/lib/builtins/aarch64/emupac.cpp

+// Determine whether PAC is supported without accessing memory. This utilizes
+// the XPACLRI instruction which will copy bit 55 of x30 into at least bit 54 if
+// PAC is supported and acts as a NOP if PAC is not supported.
+static bool pac_supported() {


[nit] It looks like functions under compiler-rt/lib/builtins which are not part of the public interface generally follow the standard naming conventions.

Suggested change

static bool pac_supported() {

static bool pacSupported() {

atrosinenko · 2025-05-26T14:12:34Z

compiler-rt/lib/builtins/aarch64/emupac.cpp

+static const uint8_t K[16] = {0xb5, 0xd4, 0xc9, 0xeb, 0x79, 0x10, 0x4a, 0x79,
+                              0x6f, 0xec, 0x8b, 0x1b, 0x42, 0x87, 0x81, 0xd4};


This probably deserves a comment, something like "emulated DA key value".

atrosinenko · 2025-05-26T14:20:55Z

compiler-rt/lib/builtins/aarch64/emupac.cpp

+                         : "r"(disc));
+    return ptr;
+  }
+  uint64_t ptr_without_pac =


[nit] According to standard LLVM naming conventions, it should be PtrWithoutPac, though I'm not sure whether naming conventions are strict for compiler-rt. The same applies to other local variables and function arguments.

atrosinenko · 2025-05-26T14:28:15Z

compiler-rt/test/builtins/Unit/aarch64/emupac.c

+
+int main(int argc, char **argv) {
+  char stack_object1;
+  uint64_t ptr1 = (uint64_t)stack_object1;


(uint64_t)&stack_object1 was probably intended (the same for stack_object2).

atrosinenko · 2025-05-26T14:56:18Z

compiler-rt/lib/builtins/aarch64/emupac.cpp

+// The purpose of the emulation is to allow programs to be built to be portable
+// to machines without PAC support, with some performance loss and increased
+// probability of false positives (due to not being able to portably determine
+// the VA size), while being functionally almost equivalent to running on a
+// machine with PAC support. One example of a use case is if PAC is used in
+// production as a security mitigation, but the testing environment is
+// heterogeneous (i.e. some machines lack PAC support). In this case we would
+// like the testing machines to be able to detect issues resulting
+// from the use of PAC instructions that would affect production by running
+// tests. This can be achieved by building test binaries with EmuPAC and
+// production binaries with real PAC.


I wonder if it should be explicitly stated that EmuPAC is not intended to be used in production (something like it is done for Address Sanitizer: https://clang.llvm.org/docs/AddressSanitizer.html#security-considerations).

atrosinenko · 2025-05-26T14:59:31Z

Since this PR enables another language (CXX in addition to C and ASM) in the builtins library, it is probably worth reviewing by someone familiar with CMake build descriptions used in LLVM.

[𝘀𝗽𝗿] initial version

2397a45

Created using spr 1.3.6-beta.1

llvmbot added compiler-rt compiler-rt:builtins labels Mar 28, 2025

kbeyls reviewed Mar 29, 2025

View reviewed changes

jroelofs reviewed Mar 31, 2025

View reviewed changes

llvm-beanz requested changes Apr 1, 2025

View reviewed changes

Switch to SipHash

84a0613

Created using spr 1.3.6-beta.1

pcc added this to Structure Protection Apr 3, 2025

Fix CMake build

b37a44f

Created using spr 1.3.6-beta.1

pcc added 3 commits April 7, 2025 20:45

Add test

66b7ed2

Created using spr 1.3.6-beta.1

Rebase

48031d4

Created using spr 1.3.6-beta.1

Rebase

b4a4147

Created using spr 1.3.6-beta.1

llvm-beanz self-requested a review April 28, 2025 01:08

pcc added 2 commits May 12, 2025 21:36

Rebase

f3d5547

Created using spr 1.3.6-beta.1

Rebase

7915d67

Created using spr 1.3.6-beta.1

atrosinenko reviewed May 26, 2025

View reviewed changes

		static const uint8_t K[16] = {0xb5, 0xd4, 0xc9, 0xeb, 0x79, 0x10, 0x4a, 0x79,
		0x6f, 0xec, 0x8b, 0x1b, 0x42, 0x87, 0x81, 0xd4};

compiler-rt: Introduce runtime functions for emulated PAC. #133530

Are you sure you want to change the base?

compiler-rt: Introduce runtime functions for emulated PAC. #133530

Conversation

pcc commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Mar 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llvm-beanz left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pcc commented Apr 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atrosinenko commented May 26, 2025

Uh oh!

Uh oh!

pcc commented Mar 28, 2025 •

edited

Loading

github-actions bot commented Mar 28, 2025 •

edited

Loading

github-actions bot commented Apr 3, 2025 •

edited

Loading