-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions fixed #136868
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
[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions fixed #136868
Conversation
1) Checking opcodes specifically for LDGM and AUT opcode instruction variants. 2) Gracefully exiting with "<opcode-name> : Unsupported opcode: isPointerAuth/isUncheckedAccess" 3) Added corresponding test cases to check exit message.
- Replace hardcoded opcode ranges with AArch64::* enum values for pointer auth and unchecked access instructions. - Add required AArch64 headers and update tests. - Style changes for corresponding testcases.
…ctions. 1) Change location of isPointerAuth and isLoadTagMultiple function across file for correct implementation 2) Style nits changes in test cases.
…-exegesis-illegal-instr
- Included necessary headers for PR_PAC_* constants and modified the getIgnoredOpcodeReasonOrNull function to disabling pointer authentication. - Adjusted formatting.
…ication and load tag multiple - Moved isPointerAuth and isLoadTagMultiple functions to AArch64/Target.cpp for better organization. - Updated getIgnoredOpcodeReasonOrNull to handle pointer authentication and load tag multiple opcodes correctly. - Removed redundant definitions from Target.cpp and Target.h.
…Linux - Added platform-specific handling for pointer authentication instructions, enabling the disabling of PAC keys on Linux. - Ensured that unsupported opcodes return an appropriate message on non-Linux platforms.
This reverts commit dade502.
…ormatting changes.
To resolve build failure due to undefined PR_PAC_SET_ENABLED_KEYS. 1) Added stricter check for OS requirement. 2) Initialized variable used if still undefined.
…r functions reverted back. These changed got misplaced during last commit. Reverted back.
…tion if undefined.
@llvm/pr-subscribers-tools-llvm-exegesis Author: Lakshay Kumar (lakshayk-nv) Changes[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions. Last pull request #132346 got reviewed and merged but builder bot got failed. This was due to undefined This is followup to merge the changes with following changes to fixup the build failure. Changes:
Full diff: https://github.com/llvm/llvm-project/pull/136868.diff 2 Files Affected:
diff --git a/llvm/test/tools/llvm-exegesis/AArch64/skip_unsupported_instructions.s b/llvm/test/tools/llvm-exegesis/AArch64/skip_unsupported_instructions.s
new file mode 100644
index 0000000000000..927ee190e803f
--- /dev/null
+++ b/llvm/test/tools/llvm-exegesis/AArch64/skip_unsupported_instructions.s
@@ -0,0 +1,10 @@
+llvm/test/tools/llvm-exegesis/AArch64/skip_unsupported_instructions.s
+
+# REQUIRES: aarch64-registered-target
+
+# Check for skipping of illegal instruction errors (AUT and LDGM)
+# RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --opcode-name=AUTIA --benchmark-phase=assemble-measured-code 2>&1 | FileCheck %s --check-prefix=CHECK-AUTIA
+# CHECK-AUTIA-NOT: snippet crashed while running: Illegal instruction
+
+# RUN: llvm-exegesis -mcpu=neoverse-v2 -mode=latency --opcode-name=LDGM --benchmark-phase=assemble-measured-code 2>&1 | FileCheck %s --check-prefix=CHECK-LDGM
+# CHECK-LDGM: LDGM: Unsupported opcode: load tag multiple
\ No newline at end of file
diff --git a/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp b/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
index bf2b053003ce3..6d8e19e5820c7 100644
--- a/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/AArch64/Target.cpp
@@ -9,12 +9,64 @@
#include "AArch64.h"
#include "AArch64RegisterInfo.h"
+#if defined(__aarch64__) && defined(__linux__)
+#include <linux/prctl.h> // For PR_PAC_* constants
+#include <sys/prctl.h>
+#ifndef PR_PAC_SET_ENABLED_KEYS
+#define PR_PAC_SET_ENABLED_KEYS 60
+#endif
+#ifndef PR_PAC_GET_ENABLED_KEYS
+#define PR_PAC_GET_ENABLED_KEYS 61
+#endif
+#endif
+
#define GET_AVAILABLE_OPCODE_CHECKER
#include "AArch64GenInstrInfo.inc"
namespace llvm {
namespace exegesis {
+bool isPointerAuth(unsigned Opcode) {
+ switch (Opcode) {
+ default:
+ return false;
+
+ // FIXME: Pointer Authentication instructions.
+ // We would like to measure these instructions, but they can behave
+ // differently on different platforms, and maybe the snippets need to look
+ // different for these instructions,
+ // Platform-specific handling: On Linux, we disable authentication, may
+ // interfere with measurements. On non-Linux platforms, disable opcodes for
+ // now.
+ case AArch64::AUTDA:
+ case AArch64::AUTDB:
+ case AArch64::AUTDZA:
+ case AArch64::AUTDZB:
+ case AArch64::AUTIA:
+ case AArch64::AUTIA1716:
+ case AArch64::AUTIASP:
+ case AArch64::AUTIAZ:
+ case AArch64::AUTIB:
+ case AArch64::AUTIB1716:
+ case AArch64::AUTIBSP:
+ case AArch64::AUTIBZ:
+ case AArch64::AUTIZA:
+ case AArch64::AUTIZB:
+ return true;
+ }
+}
+
+bool isLoadTagMultiple(unsigned Opcode) {
+ switch (Opcode) {
+ default:
+ return false;
+
+ // Load tag multiple instruction
+ case AArch64::LDGM:
+ return true;
+ }
+}
+
static unsigned getLoadImmediateOpcode(unsigned RegBitWidth) {
switch (RegBitWidth) {
case 32:
@@ -134,6 +186,35 @@ class ExegesisAArch64Target : public ExegesisTarget {
// Function return is a pseudo-instruction that needs to be expanded
PM.add(createAArch64ExpandPseudoPass());
}
+
+ const char *getIgnoredOpcodeReasonOrNull(const LLVMState &State,
+ unsigned Opcode) const override {
+ if (const char *Reason =
+ ExegesisTarget::getIgnoredOpcodeReasonOrNull(State, Opcode))
+ return Reason;
+
+ if (isPointerAuth(Opcode)) {
+#if defined(__aarch64__) && defined(__linux__)
+ // Disable all PAC keys. Note that while we expect the measurements to
+ // be the same with PAC keys disabled, they could potentially be lower
+ // since authentication checks are bypassed.
+ if (prctl(PR_PAC_SET_ENABLED_KEYS,
+ PR_PAC_APIAKEY | PR_PAC_APIBKEY | PR_PAC_APDAKEY |
+ PR_PAC_APDBKEY, // all keys
+ 0, // disable all
+ 0, 0) < 0) {
+ return "Failed to disable PAC keys";
+ }
+#else
+ return "Unsupported opcode: isPointerAuth";
+#endif
+ }
+
+ if (isLoadTagMultiple(Opcode))
+ return "Unsupported opcode: load tag multiple";
+
+ return nullptr;
+ }
};
} // namespace
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM to me still. We will have to see if this still has some issues with the buildbots, if we do then we will perhaps need to make it simpler or adjust it via a cmake option. Good luck!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (carrying over from the last patch).
Were you able to reproduce the issue locally?
Yes, We replicated build failure locally using a |
Sounds good. Seems perfectly reasonable to reland then. |
We started to see this new test crashing when running on real aarch64:
I've only seen this failing in CPUs where paca and pacg are available. |
A full build log with the error is available at: https://download.copr.fedorainfracloud.org/results/@fedora-llvm-team/llvm-snapshots-big-merge-20250505/fedora-rawhide-aarch64/08995642-llvm/builder-live.log.gz This is the cpu info:
|
Looking at the error log, it seems that it is disabling pac (on some systems) without checking if the keys are present/accessible through prctl() . A solution can be to check prior if pac is available in the system and then to disable it . |
…ctions fixed (llvm#136868) [llvm-exegesis][AArch64] Recommit: Disable pauth and ldgm as unsupported instructions. Skipping AUT and LDGM opcode variants which currently throws "illegal instruction". Last pull request [llvm#132346](llvm#132346) got reviewed and merged but builder bot got failed. This was due to undefined `PR_PAC_SET_ENABLED_KEYS` utilized were not defined in x86 arch, resulting in build failure. This is followup to merge the changes with following changes to fixup the build failure. Changes: - Fixed up the problem with arch specific check for `prctl` library import - Defining `PR_PAC_SET_ENABLED_KEYS` if undefined.
…ctions fixed (llvm#136868) [llvm-exegesis][AArch64] Recommit: Disable pauth and ldgm as unsupported instructions. Skipping AUT and LDGM opcode variants which currently throws "illegal instruction". Last pull request [llvm#132346](llvm#132346) got reviewed and merged but builder bot got failed. This was due to undefined `PR_PAC_SET_ENABLED_KEYS` utilized were not defined in x86 arch, resulting in build failure. This is followup to merge the changes with following changes to fixup the build failure. Changes: - Fixed up the problem with arch specific check for `prctl` library import - Defining `PR_PAC_SET_ENABLED_KEYS` if undefined.
…ctions fixed (llvm#136868) [llvm-exegesis][AArch64] Recommit: Disable pauth and ldgm as unsupported instructions. Skipping AUT and LDGM opcode variants which currently throws "illegal instruction". Last pull request [llvm#132346](llvm#132346) got reviewed and merged but builder bot got failed. This was due to undefined `PR_PAC_SET_ENABLED_KEYS` utilized were not defined in x86 arch, resulting in build failure. This is followup to merge the changes with following changes to fixup the build failure. Changes: - Fixed up the problem with arch specific check for `prctl` library import - Defining `PR_PAC_SET_ENABLED_KEYS` if undefined.
…ctions fixed (llvm#136868) [llvm-exegesis][AArch64] Recommit: Disable pauth and ldgm as unsupported instructions. Skipping AUT and LDGM opcode variants which currently throws "illegal instruction". Last pull request [llvm#132346](llvm#132346) got reviewed and merged but builder bot got failed. This was due to undefined `PR_PAC_SET_ENABLED_KEYS` utilized were not defined in x86 arch, resulting in build failure. This is followup to merge the changes with following changes to fixup the build failure. Changes: - Fixed up the problem with arch specific check for `prctl` library import - Defining `PR_PAC_SET_ENABLED_KEYS` if undefined.
I'm guessing that the issue is that this change is disabling the IA key while a stack frame built with return PAC has a signed return address. The AUTIASP will act as a NOP and the RET instruction will branch to the signed address, causing the segfault observed in #136868 (comment) . Also, if llvm-exegesis is run on a system that supports PAuth ABI, this prctl will lead to a crash on the next indirect or virtual call. PR_PAC_SET_ENABLED_KEYS isn't really meant to be used by normal user applications, so I'm not sure if there's a good way to make this work. Maybe you could prevent llvm-exegesis from generating AUT instructions for now? |
I've disabled the test for now while this discussion is ongoing. I thought some of the fixes would be moving a little bit faster. |
We can put up a patch for this, let me know :) |
[llvm-exegesis][AArch64] Disable pauth and ldgm as unsupported instructions.
Skipping AUT and LDGM opcode variants which currently throws "illegal instruction".
Last pull request #132346 got reviewed and merged but builder bot got failed. This was due to undefined
PR_PAC_SET_ENABLED_KEYS
utilized were not defined in x86 arch, resulting in build failure.This is followup to merge the changes with following changes to fixup the build failure.
Changes:
prctl
library importPR_PAC_SET_ENABLED_KEYS
if undefined.