[clang][AArch64] Use B-key for -msign-return-address= on Windows - #220122
Conversation
-mbranch-protection= already defaults to B-key on AArch64 Windows, in ARM::parseBranchProtection(), but the -msign-return-address= spelling in CollectARMPACBTIOptions() hard-codes a_key for every target. That spelling has no syntax for a key, so on AArch64 Windows it selected a key the target does not support. Before LLVM 23 this silently emitted A-key signing (paciasp/autiasp) on Windows. LLVM 23 added a check in ShouldSignWithBKey() that turns it into a hard error: error: A-key return address signing is unsupported on AArch64 Windows so -msign-return-address= became unusable there. Default the key to b_key on AArch64 Windows, matching parseBranchProtection(). Windows now emits pacibsp/autibsp (hint llvm#27 / llvm#31) with .seh_pac_sign_lr for every spelling; other targets keep a_key unless b-key is requested.
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Daniel Paoliello (dpaoliello) Changes
After #203989 trying to use The fix is to default the key to Assisted-by: Claude Opus 5 (via VS Code) Full diff: https://github.com/llvm/llvm-project/pull/220122.diff 2 Files Affected:
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 072664e6040f3..b3923c0c1e981 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1413,7 +1413,9 @@ static void CollectARMPACBTIOptions(const ToolChain &TC, const ArgList &Args,
if (Scope != "none" && Scope != "non-leaf" && Scope != "all")
D.Diag(diag::err_drv_unsupported_option_argument)
<< A->getSpelling() << Scope;
- Key = "a_key";
+ // This spelling cannot express a key, and AArch64 Windows only supports
+ // B-key, so default to it there as parseBranchProtection() does.
+ Key = isAArch64 && Triple.isOSWindows() ? "b_key" : "a_key";
IndirectBranches =
(Triple.isOSOpenBSD() || Triple.isAndroid()) && isAArch64;
BranchProtectionPAuthLR = false;
diff --git a/clang/test/Driver/aarch64-security-options.c b/clang/test/Driver/aarch64-security-options.c
index 125e90b2b51fb..087b4b247bd82 100644
--- a/clang/test/Driver/aarch64-security-options.c
+++ b/clang/test/Driver/aarch64-security-options.c
@@ -9,6 +9,14 @@
// RUN: %clang --target=aarch64 -c %s -### -msign-return-address=all 2>&1 | \
// RUN: FileCheck %s --check-prefix=RA-ALL --check-prefix=KEY-A --check-prefix=BTE-OFF --check-prefix=GCS-OFF --check-prefix=WARN
+// This spelling cannot express a key, so it has to pick the one the target
+// supports. AArch64 Windows only supports B-key.
+// RUN: %clang --target=aarch64-windows-msvc -c %s -### -msign-return-address=non-leaf 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B --check-prefix=BTE-OFF --check-prefix=GCS-OFF --check-prefix=WARN
+
+// RUN: %clang --target=aarch64-windows-msvc -c %s -### -msign-return-address=all 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL --check-prefix=KEY-B --check-prefix=BTE-OFF --check-prefix=GCS-OFF --check-prefix=WARN
+
// -mbranch-protection with standard
// RUN: %clang --target=aarch64 -c %s -### -mbranch-protection=standard 2>&1 | \
// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A --check-prefix=BTE-ON --check-prefix=GCS-ON --check-prefix=WARN
|
mstorsjo
left a comment
There was a problem hiding this comment.
Looks reasonable to me in general, but let's see if @efriedma-quic or @DanielKristofKiss have anything more to add - otherwise it's probably fine to merge in a day or two.
…m#220122) `-mbranch-protection=` defaults to B-key on AArch64 Windows but the `-msign-return-address=` flag hard-codes `a_key` for every target. That flag has no syntax for selecting the key, so on AArch64 Windows it used an unsupported key. After llvm#203989 trying to use `-msign-return-address=` on AArch64 Windows became a hard error: ``` error: A-key return address signing is unsupported on AArch64 Windows ``` The fix is to default the key to `b_key` on AArch64 Windows, matching `parseBranchProtection()`. Windows now emits `pacibsp`/`autibsp` (hint `llvm#27` / `llvm#31`) with `.seh_pac_sign_lr` for every spelling; other targets keep `a_key` unless `b_key` is requested. Assisted-by: Claude Opus 5 (via VS Code)
…m#220122) `-mbranch-protection=` defaults to B-key on AArch64 Windows but the `-msign-return-address=` flag hard-codes `a_key` for every target. That flag has no syntax for selecting the key, so on AArch64 Windows it used an unsupported key. After llvm#203989 trying to use `-msign-return-address=` on AArch64 Windows became a hard error: ``` error: A-key return address signing is unsupported on AArch64 Windows ``` The fix is to default the key to `b_key` on AArch64 Windows, matching `parseBranchProtection()`. Windows now emits `pacibsp`/`autibsp` (hint `llvm#27` / `llvm#31`) with `.seh_pac_sign_lr` for every spelling; other targets keep `a_key` unless `b_key` is requested. Assisted-by: Claude Opus 5 (via VS Code)
-mbranch-protection=defaults to B-key on AArch64 Windows but the-msign-return-address=flag hard-codesa_keyfor every target. That flag has no syntax for selecting the key, so on AArch64 Windows it used an unsupported key.After #203989 trying to use
-msign-return-address=on AArch64 Windows became a hard error:The fix is to default the key to
b_keyon AArch64 Windows, matchingparseBranchProtection(). Windows now emitspacibsp/autibsp(hint#27/#31) with.seh_pac_sign_lrfor every spelling; other targets keepa_keyunlessb_keyis requested.Assisted-by: Claude Opus 5 (via VS Code)