Skip to content

Reapply "compiler-rt: Introduce runtime functions for emulated PAC." #148094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

pcc
Copy link
Contributor

@pcc pcc commented Jul 11, 2025

This reverts commit 0c0aa56.

This time with the following fixes for buildbot failures:

  • Add underscore prefixes to symbol names on Apple platforms.
  • Modify the test so that it skips the crash tests on platforms where
    they are not expected to pass:
    • Platforms that implement FEAT_PAuth but not FEAT_FPAC (e.g.
      Apple M1, Cortex-A78C)
    • Platforms where DA key is disabled (e.g. older Linux kernels,
      Linux kernels with PAC disabled, likely Windows)

Original commit message follows:

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
Copy link

github-actions bot commented Jul 11, 2025

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

Created using spr 1.3.6-beta.1
@@ -26,6 +26,7 @@ builtin_check_c_compiler_flag("-Xclang -mcode-object-version=none" COMPILER_RT_H
builtin_check_c_compiler_flag(-Wbuiltin-declaration-mismatch COMPILER_RT_HAS_WBUILTIN_DECLARATION_MISMATCH_FLAG)
builtin_check_c_compiler_flag(/Zl COMPILER_RT_HAS_ZL_FLAG)
builtin_check_c_compiler_flag(-fcf-protection=full COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
builtin_check_c_compiler_flag(-nostdinc++ COMPILER_RT_HAS_NOSTDINCXX_FLAG)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, I wonder if builtin_check_c_compiler_flag handles C++ flags correctly. It may depend on the compiler, I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both gcc and clang (i.e. as opposed to g++ and clang++) accept this flag.

@@ -836,7 +839,7 @@ else ()
append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full BUILTIN_CFLAGS)
endif()

append_list_if(COMPILER_RT_HAS_STD_C11_FLAG -std=c11 BUILTIN_CFLAGS)
append_list_if(COMPILER_RT_HAS_NOSTDINCXX_FLAG -nostdinc++ BUILTIN_CFLAGS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] If a separate builtin_check_cxx_compiler_flag function would be required for any C++ compiler, BUILTIN_CFLAGS may have to be updated here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted in the other comment, it doesn't seem to be a problem to pass this to the C compiler as well.

Comment on lines +60 to +68
// This asm snippet is used to force the creation of a frame record when
// calling the EmuPAC functions. This is important because the EmuPAC functions
// may crash if an auth failure is detected and may be unwound past using a
// frame pointer based unwinder.
#ifdef __GCC_HAVE_DWARF2_CFI_ASM
#define CFI_INST(inst) inst
#else
#define CFI_INST(inst)
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is possible to emit WinCFI information as well by means of .seh_* directives, though I'm not sure if it is important here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with WinCFI so I'll leave that as a followup in case anyone is interested in adding it.

Comment on lines 123 to 124
__asm__(".globl __emupac_pacda\n"
"__emupac_pacda:\n" FRAME_POINTER_WRAP(__emupac_pacda_impl));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__emupac_pacda should probably be wrapped with FRAME_POINTER_WRAP on both lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you mean ASM_SYMBOL? Yes, done.

Comment on lines 145 to 146
__asm__(".globl __emupac_autda\n"
"__emupac_autda:\n" FRAME_POINTER_WRAP(__emupac_autda_impl));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for __emupac_autda.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mstorsjo
Copy link
Member

Thanks - this version no longer seems to break my macOS build, and no longer fails the test on windows of arm64.

Created using spr 1.3.6-beta.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants