[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info #113945

kovdan01 · 2024-10-28T18:42:29Z

Assume PAC instructions being supported with PAuth core info different from (0,0). The (0,0) value means that an ELF file is incompatible with PAuth - see https://github.com/ARM-software/abi-aa/blob/2024Q3/pauthabielf64/pauthabielf64.rst#core-information. With PAC non-hint instructions supported, autia1716; br x17 can be replaced with braa x17, x16; nop, where braa is an authenticated branch instruction using IA key, discriminator from x16 and signed target address from x17.

kovdan01 · 2024-10-28T18:42:48Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

llvmbot · 2024-10-28T18:43:49Z

@llvm/pr-subscribers-lld

Author: Daniil Kovalev (kovdan01)

Changes

Assume PAC instructions being supported with PAuth core info different
from (0,0). Given that, autia1716; br x17 can be replaced with
braa x17, x16; nop.

Full diff: https://github.com/llvm/llvm-project/pull/113945.diff

2 Files Affected:

(modified) lld/ELF/Arch/AArch64.cpp (+15-4)
(modified) lld/test/ELF/aarch64-feature-pauth.s (+6-4)

diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 260307ac4c3dcb..c76f226bc5511c 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -999,7 +999,9 @@ class AArch64BtiPac final : public AArch64 {
 
 private:
   bool btiHeader; // bti instruction needed in PLT Header and Entry
-  bool pacEntry;  // autia1716 instruction needed in PLT Entry
+  bool pacEntry;  // Authenticated branch needed in PLT Entry
+  bool pacUseHint =
+      true; // Use hint space instructions for authenticated branch in PLT entry
 };
 } // namespace
 
@@ -1016,6 +1018,10 @@ AArch64BtiPac::AArch64BtiPac(Ctx &ctx) : AArch64(ctx) {
   // from properties in the objects, so we use the command line flag.
   pacEntry = ctx.arg.zPacPlt;
 
+  if (llvm::any_of(ctx.aarch64PauthAbiCoreInfo,
+                   [](uint8_t c) { return c != 0; }))
+    pacUseHint = false;
+
   if (btiHeader || pacEntry) {
     pltEntrySize = 24;
     ipltEntrySize = 24;
@@ -1066,9 +1072,13 @@ void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol &sym,
       0x11, 0x02, 0x40, 0xf9,  // ldr  x17, [x16, Offset(&(.got.plt[n]))]
       0x10, 0x02, 0x00, 0x91   // add  x16, x16, Offset(&(.got.plt[n]))
   };
+  const uint8_t pacHintBr[] = {
+      0x9f, 0x21, 0x03, 0xd5, // autia1716
+      0x20, 0x02, 0x1f, 0xd6  // br   x17
+  };
   const uint8_t pacBr[] = {
-      0x9f, 0x21, 0x03, 0xd5,  // autia1716
-      0x20, 0x02, 0x1f, 0xd6   // br   x17
+      0x30, 0x0a, 0x1f, 0xd7, // braa x17, x16
+      0x1f, 0x20, 0x03, 0xd5  // nop
   };
   const uint8_t stdBr[] = {
       0x20, 0x02, 0x1f, 0xd6,  // br   x17
@@ -1097,7 +1107,8 @@ void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol &sym,
   relocateNoSym(buf + 8, R_AARCH64_ADD_ABS_LO12_NC, gotPltEntryAddr);
 
   if (pacEntry)
-    memcpy(buf + sizeof(addrInst), pacBr, sizeof(pacBr));
+    memcpy(buf + sizeof(addrInst), (pacUseHint ? pacHintBr : pacBr),
+           sizeof(pacUseHint ? pacHintBr : pacBr));
   else
     memcpy(buf + sizeof(addrInst), stdBr, sizeof(stdBr));
   if (!hasBti)
diff --git a/lld/test/ELF/aarch64-feature-pauth.s b/lld/test/ELF/aarch64-feature-pauth.s
index c11073dba86f24..34f2f2698a26b8 100644
--- a/lld/test/ELF/aarch64-feature-pauth.s
+++ b/lld/test/ELF/aarch64-feature-pauth.s
@@ -56,8 +56,8 @@
 
 # PACPLTTAG:      0x0000000070000003 (AARCH64_PAC_PLT)
 
-# RUN: llvm-objdump -d pacplt-nowarn | FileCheck --check-prefix PACPLT -DA=10380 -DB=478 -DC=480 %s
-# RUN: llvm-objdump -d pacplt-warn   | FileCheck --check-prefix PACPLT -DA=10390 -DB=488 -DC=490 %s
+# RUN: llvm-objdump -d pacplt-nowarn | FileCheck --check-prefixes=PACPLT,NOHINT -DA=10380 -DB=478 -DC=480 %s
+# RUN: llvm-objdump -d pacplt-warn   | FileCheck --check-prefixes=PACPLT,HINT   -DA=10390 -DB=488 -DC=490 %s
 
 # PACPLT: Disassembly of section .text:
 # PACPLT:      <func2>:
@@ -77,8 +77,10 @@
 # PACPLT-NEXT:     adrp    x16, 0x30000 <func3+0x30000>
 # PACPLT-NEXT:     ldr     x17, [x16, #0x[[C]]]
 # PACPLT-NEXT:     add     x16, x16, #0x[[C]]
-# PACPLT-NEXT:     autia1716
-# PACPLT-NEXT:     br      x17
+# NOHINT-NEXT:     braa    x17, x16
+# NOHINT-NEXT:     nop
+# HINT-NEXT:       autia1716
+# HINT-NEXT:       br      x17
 # PACPLT-NEXT:     nop
 
 #--- abi-tag-short.s

llvmbot · 2024-10-28T18:43:49Z

@llvm/pr-subscribers-lld-elf

Author: Daniil Kovalev (kovdan01)

Changes

Assume PAC instructions being supported with PAuth core info different
from (0,0). Given that, autia1716; br x17 can be replaced with
braa x17, x16; nop.

Full diff: https://github.com/llvm/llvm-project/pull/113945.diff

2 Files Affected:

(modified) lld/ELF/Arch/AArch64.cpp (+15-4)
(modified) lld/test/ELF/aarch64-feature-pauth.s (+6-4)

diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 260307ac4c3dcb..c76f226bc5511c 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -999,7 +999,9 @@ class AArch64BtiPac final : public AArch64 {
 
 private:
   bool btiHeader; // bti instruction needed in PLT Header and Entry
-  bool pacEntry;  // autia1716 instruction needed in PLT Entry
+  bool pacEntry;  // Authenticated branch needed in PLT Entry
+  bool pacUseHint =
+      true; // Use hint space instructions for authenticated branch in PLT entry
 };
 } // namespace
 
@@ -1016,6 +1018,10 @@ AArch64BtiPac::AArch64BtiPac(Ctx &ctx) : AArch64(ctx) {
   // from properties in the objects, so we use the command line flag.
   pacEntry = ctx.arg.zPacPlt;
 
+  if (llvm::any_of(ctx.aarch64PauthAbiCoreInfo,
+                   [](uint8_t c) { return c != 0; }))
+    pacUseHint = false;
+
   if (btiHeader || pacEntry) {
     pltEntrySize = 24;
     ipltEntrySize = 24;
@@ -1066,9 +1072,13 @@ void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol &sym,
       0x11, 0x02, 0x40, 0xf9,  // ldr  x17, [x16, Offset(&(.got.plt[n]))]
       0x10, 0x02, 0x00, 0x91   // add  x16, x16, Offset(&(.got.plt[n]))
   };
+  const uint8_t pacHintBr[] = {
+      0x9f, 0x21, 0x03, 0xd5, // autia1716
+      0x20, 0x02, 0x1f, 0xd6  // br   x17
+  };
   const uint8_t pacBr[] = {
-      0x9f, 0x21, 0x03, 0xd5,  // autia1716
-      0x20, 0x02, 0x1f, 0xd6   // br   x17
+      0x30, 0x0a, 0x1f, 0xd7, // braa x17, x16
+      0x1f, 0x20, 0x03, 0xd5  // nop
   };
   const uint8_t stdBr[] = {
       0x20, 0x02, 0x1f, 0xd6,  // br   x17
@@ -1097,7 +1107,8 @@ void AArch64BtiPac::writePlt(uint8_t *buf, const Symbol &sym,
   relocateNoSym(buf + 8, R_AARCH64_ADD_ABS_LO12_NC, gotPltEntryAddr);
 
   if (pacEntry)
-    memcpy(buf + sizeof(addrInst), pacBr, sizeof(pacBr));
+    memcpy(buf + sizeof(addrInst), (pacUseHint ? pacHintBr : pacBr),
+           sizeof(pacUseHint ? pacHintBr : pacBr));
   else
     memcpy(buf + sizeof(addrInst), stdBr, sizeof(stdBr));
   if (!hasBti)
diff --git a/lld/test/ELF/aarch64-feature-pauth.s b/lld/test/ELF/aarch64-feature-pauth.s
index c11073dba86f24..34f2f2698a26b8 100644
--- a/lld/test/ELF/aarch64-feature-pauth.s
+++ b/lld/test/ELF/aarch64-feature-pauth.s
@@ -56,8 +56,8 @@
 
 # PACPLTTAG:      0x0000000070000003 (AARCH64_PAC_PLT)
 
-# RUN: llvm-objdump -d pacplt-nowarn | FileCheck --check-prefix PACPLT -DA=10380 -DB=478 -DC=480 %s
-# RUN: llvm-objdump -d pacplt-warn   | FileCheck --check-prefix PACPLT -DA=10390 -DB=488 -DC=490 %s
+# RUN: llvm-objdump -d pacplt-nowarn | FileCheck --check-prefixes=PACPLT,NOHINT -DA=10380 -DB=478 -DC=480 %s
+# RUN: llvm-objdump -d pacplt-warn   | FileCheck --check-prefixes=PACPLT,HINT   -DA=10390 -DB=488 -DC=490 %s
 
 # PACPLT: Disassembly of section .text:
 # PACPLT:      <func2>:
@@ -77,8 +77,10 @@
 # PACPLT-NEXT:     adrp    x16, 0x30000 <func3+0x30000>
 # PACPLT-NEXT:     ldr     x17, [x16, #0x[[C]]]
 # PACPLT-NEXT:     add     x16, x16, #0x[[C]]
-# PACPLT-NEXT:     autia1716
-# PACPLT-NEXT:     br      x17
+# NOHINT-NEXT:     braa    x17, x16
+# NOHINT-NEXT:     nop
+# HINT-NEXT:       autia1716
+# HINT-NEXT:       br      x17
 # PACPLT-NEXT:     nop
 
 #--- abi-tag-short.s

asl · 2024-10-29T08:20:33Z

lld/ELF/Arch/AArch64.cpp

@@ -999,7 +999,9 @@ class AArch64BtiPac final : public AArch64 {

 private:
  bool btiHeader; // bti instruction needed in PLT Header and Entry
-  bool pacEntry;  // autia1716 instruction needed in PLT Entry
+  bool pacEntry;  // Authenticated branch needed in PLT Entry


maybe we can just have enum here instead of pair of bools? E.g.

enum { Unsigned, SignedHint, // use autia1716 instruction in PLT entry Signed // can use non-hint braa instruction }

Changed to an enum, thanks! It might be worth using a switch statement instead of if's and ternary operators at the end of AArch64BtiPac::writePlt, but it looks like that it's readable enough right now, so I left that "as is" unless there is a request for changing that as well.

DanielKristofKiss · 2024-11-04T12:54:45Z

lld/ELF/Arch/AArch64.cpp

-      0x9f, 0x21, 0x03, 0xd5,  // autia1716
-      0x20, 0x02, 0x1f, 0xd6   // br   x17
+      0x30, 0x0a, 0x1f, 0xd7, // braa x17, x16
+      0x1f, 0x20, 0x03, 0xd5  // nop


maybe UDF instruction is a safer choice instead of NOP. also may not have any significance.

I think it's a nice suggestion, but I'll apply it in a separate PR not to mix different changes - nop is present both in pacBr and stdBr, while the current PR is intended to change only pacBr.

DanielKristofKiss

LGTM

smithp35

No objections from me. A small suggestion for the comment and a possible simplification.

smithp35 · 2024-11-04T17:28:46Z

lld/ELF/Arch/AArch64.cpp

@@ -1014,9 +1018,18 @@ AArch64BtiPac::AArch64BtiPac(Ctx &ctx) : AArch64(ctx) {
  // relocations.
  // The PAC PLT entries require dynamic loader support and this isn't known
  // from properties in the objects, so we use the command line flag.
-  pacEntry = ctx.arg.zPacPlt;


It could be worth adding to the comment here. Something like:

By default we only use hint-space instructions, but if we detect the PAuthABI, which requires v8.3-A we can use the non-hint space instructions.

Added, thanks, see b189f51

smithp35 · 2024-11-04T17:34:49Z

lld/ELF/Arch/AArch64.cpp

-    memcpy(buf + sizeof(addrInst), pacBr, sizeof(pacBr));
+  if (pacEntryKind != PEK_NoAuth)
+    memcpy(buf + sizeof(addrInst),
+           pacEntryKind == PEK_AuthHint ? pacHintBr : pacBr,


Could be a possibility to make pacHintBr and pacBr into an array indexed by pacEntryKind.

memcpy(buf + sizeof(addrInst), pacBrInstrs[pacEntryKind], sizeof(pacBrInstrs[pacEntryKind]));

Not sure if it is worth the trouble though.

Thanks for suggestion. I think I'll leave this "as is" w/o changes - it looks like that a pretty small decrease in code length here will require increase in complexity by introducing invariants on enum values and increase in code length somewhere else.

kovdan01 · 2024-11-10T16:55:25Z

@MaskRay Please let me know if there are any objections on merging this

Assume PAC instructions being supported with PAuth core info different from (0,0). Given that, `autia1716; br x17` can be replaced with `braa x17, x16; nop`.

MaskRay · 2024-11-17T20:29:51Z

Assume PAC instructions being supported with PAuth core info different from (0,0). Given that, autia1716; br x17 can be replaced with braa x17, x16; nop.

It is worth explaining what braa is.

"Assume PAC instructions being supported with PAuth core info different from (0,0)" seems difficult to follow for AArch64 PAuth outsiders

kovdan01 · 2024-11-18T05:05:14Z

Assume PAC instructions being supported with PAuth core info different from (0,0). Given that, autia1716; br x17 can be replaced with braa x17, x16; nop.

It is worth explaining what braa is.

"Assume PAC instructions being supported with PAuth core info different from (0,0)" seems difficult to follow for AArch64 PAuth outsiders

Thanks for suggestion! I changed the PR description to the following:

Assume PAC instructions being supported with PAuth core info different from (0,0). The (0,0) value means that an ELF file is incompatible with PAuth - see https://github.com/ARM-software/abi-aa/blob/2024Q3/pauthabielf64/pauthabielf64.rst#core-information. With PAC non-hint instructions supported, autia1716; br x17 can be replaced with braa x17, x16; nop, where braa is an authenticated branch instruction using IA key, discriminator from x16 and signed target address from x17.

kovdan01 · 2024-11-18T05:06:35Z

Merge activity

Nov 18, 12:06 AM EST: A user started a stack merge that includes this pull request via Graphite.
Nov 18, 12:07 AM EST: A user merged this pull request with Graphite.

llvm-ci · 2024-11-18T05:44:49Z

LLVM Buildbot has detected a new failure on builder openmp-offload-libc-amdgpu-runtime running on omp-vega20-1 while building lld at step 10 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/8730

Here is the relevant piece of the build log for the reference

Step 10 (Add check check-offload) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
...
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug50022.cpp (947 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug47654.cpp (948 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug53727.cpp (949 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/test_libc.cpp (950 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/wtime.c (951 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/bug49021.cpp (952 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu :: offloading/std_complex_arithmetic.cpp (953 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/complex_reduction.cpp (954 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/bug49021.cpp (955 of 960)
PASS: libomptarget :: x86_64-unknown-linux-gnu-LTO :: offloading/std_complex_arithmetic.cpp (956 of 960)
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-offload'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1237.782373

kovdan01 mentioned this pull request Oct 28, 2024

[PAC][lld] Test warning emitted for non-PAuth-marked files with -z pac-plt #112958

Merged

kovdan01 mentioned this pull request Oct 28, 2024

[PAC][lld] Do not emit warnings for -z pac-plt with valid PAuth core info #112959

Merged

kovdan01 marked this pull request as ready for review October 28, 2024 18:43

llvmbot added lld lld:ELF labels Oct 28, 2024

kovdan01 requested review from asl, MaskRay and smithp35 October 28, 2024 18:43

asl reviewed Oct 29, 2024

View reviewed changes

DanielKristofKiss reviewed Nov 4, 2024

View reviewed changes

DanielKristofKiss approved these changes Nov 4, 2024

View reviewed changes

smithp35 reviewed Nov 4, 2024

View reviewed changes

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-with-pauth-core-info branch from 35d5c86 to 7f1e0fc Compare November 10, 2024 11:55

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-braa branch from 026d7ca to 963794c Compare November 10, 2024 11:55

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-with-pauth-core-info branch from 7f1e0fc to 58b0ffa Compare November 10, 2024 11:59

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-braa branch from 963794c to 6b35745 Compare November 10, 2024 12:00

Base automatically changed from users/kovdan01/lld-pac-plt-with-pauth-core-info to main November 10, 2024 12:04

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-braa branch 2 times, most recently from 38b9ffd to b189f51 Compare November 10, 2024 16:53

kovdan01 requested a review from smithp35 November 10, 2024 16:54

kovdan01 added 3 commits November 15, 2024 11:02

[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info

cc6c7c4

Assume PAC instructions being supported with PAuth core info different from (0,0). Given that, `autia1716; br x17` can be replaced with `braa x17, x16; nop`.

Address review comments

13875e7

Add a comment

3d1670d

kovdan01 force-pushed the users/kovdan01/lld-pac-plt-braa branch from b189f51 to 3d1670d Compare November 15, 2024 08:04

kovdan01 mentioned this pull request Nov 15, 2024

[PAC][lld][ELF] Use PAC instructions in PLT header with -z pac-plt #116334

Open

MaskRay approved these changes Nov 17, 2024

View reviewed changes

kovdan01 merged commit 1c4f335 into main Nov 18, 2024
8 checks passed

kovdan01 deleted the users/kovdan01/lld-pac-plt-braa branch November 18, 2024 05:07

kovdan01 self-assigned this Nov 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info #113945

[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info #113945

kovdan01 commented Oct 28, 2024 •

edited

Loading

kovdan01 commented Oct 28, 2024 •

edited

Loading

llvmbot commented Oct 28, 2024

llvmbot commented Oct 28, 2024

asl Oct 29, 2024 •

edited

Loading

kovdan01 Nov 1, 2024

DanielKristofKiss Nov 4, 2024

kovdan01 Nov 10, 2024

DanielKristofKiss left a comment

smithp35 left a comment

smithp35 Nov 4, 2024

kovdan01 Nov 10, 2024

smithp35 Nov 4, 2024

kovdan01 Nov 10, 2024

kovdan01 commented Nov 10, 2024

MaskRay commented Nov 17, 2024 •

edited

Loading

kovdan01 commented Nov 18, 2024

kovdan01 commented Nov 18, 2024 •

edited

Loading

llvm-ci commented Nov 18, 2024

[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info #113945

[PAC][lld] Use braa instr in PAC PLT sequence with valid PAuth core info #113945

Conversation

kovdan01 commented Oct 28, 2024 • edited Loading

kovdan01 commented Oct 28, 2024 • edited Loading

llvmbot commented Oct 28, 2024

llvmbot commented Oct 28, 2024

asl Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

kovdan01 Nov 1, 2024

Choose a reason for hiding this comment

DanielKristofKiss Nov 4, 2024

Choose a reason for hiding this comment

kovdan01 Nov 10, 2024

Choose a reason for hiding this comment

DanielKristofKiss left a comment

Choose a reason for hiding this comment

smithp35 left a comment

Choose a reason for hiding this comment

smithp35 Nov 4, 2024

Choose a reason for hiding this comment

kovdan01 Nov 10, 2024

Choose a reason for hiding this comment

smithp35 Nov 4, 2024

Choose a reason for hiding this comment

kovdan01 Nov 10, 2024

Choose a reason for hiding this comment

kovdan01 commented Nov 10, 2024

MaskRay commented Nov 17, 2024 • edited Loading

kovdan01 commented Nov 18, 2024

kovdan01 commented Nov 18, 2024 • edited Loading

Merge activity

llvm-ci commented Nov 18, 2024

kovdan01 commented Oct 28, 2024 •

edited

Loading

kovdan01 commented Oct 28, 2024 •

edited

Loading

asl Oct 29, 2024 •

edited

Loading

MaskRay commented Nov 17, 2024 •

edited

Loading

kovdan01 commented Nov 18, 2024 •

edited

Loading