Skip to content
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-readobj][AArch64][ELF][PAC] Support ELF AUTH constants #72713

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

kovdan01
Copy link
Contributor

This patch adds llvm-readobj support for:

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 17, 2023

@llvm/pr-subscribers-objectyaml
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-aarch64

Author: Daniil Kovalev (kovdan01)

Changes

This patch adds llvm-readobj support for:


Patch is 30.88 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72713.diff

13 Files Affected:

  • (modified) llvm/include/llvm/BinaryFormat/DynamicTags.def (+6)
  • (modified) llvm/include/llvm/BinaryFormat/ELF.h (+8)
  • (modified) llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def (+2-1)
  • (modified) llvm/lib/Object/ELF.cpp (+1)
  • (modified) llvm/lib/ObjectYAML/ELFYAML.cpp (+1)
  • (modified) llvm/test/MC/AArch64/elf-reloc-ptrauth.s (+8-8)
  • (modified) llvm/test/tools/llvm-objdump/ELF/dynamic-section-machine-specific.test (+9)
  • (added) llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s (+98)
  • (modified) llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test (+50-10)
  • (modified) llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test (+22-10)
  • (modified) llvm/test/tools/llvm-readobj/ELF/relr-relocs.test (+20-2)
  • (modified) llvm/test/tools/llvm-readobj/ELF/section-types.test (+5)
  • (modified) llvm/tools/llvm-readobj/ELFDumper.cpp (+99-14)
diff --git a/llvm/include/llvm/BinaryFormat/DynamicTags.def b/llvm/include/llvm/BinaryFormat/DynamicTags.def
index f393b82406b41d9..1502d375f5c45da 100644
--- a/llvm/include/llvm/BinaryFormat/DynamicTags.def
+++ b/llvm/include/llvm/BinaryFormat/DynamicTags.def
@@ -132,6 +132,12 @@ AARCH64_DYNAMIC_TAG(AARCH64_MEMTAG_STACK, 0x7000000c)
 AARCH64_DYNAMIC_TAG(AARCH64_MEMTAG_GLOBALS, 0x7000000d)
 AARCH64_DYNAMIC_TAG(AARCH64_MEMTAG_GLOBALSSZ, 0x7000000f)
 
+// AArch64 specific dynamic table entries for RELR auth relocations as described here:
+// https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#dynamic-section
+AARCH64_DYNAMIC_TAG(AARCH64_AUTH_RELRSZ, 0x70000011)
+AARCH64_DYNAMIC_TAG(AARCH64_AUTH_RELR, 0x70000012)
+AARCH64_DYNAMIC_TAG(AARCH64_AUTH_RELRENT, 0x70000013)
+
 // Hexagon specific dynamic table entries
 HEXAGON_DYNAMIC_TAG(HEXAGON_SYMSZ, 0x70000000)
 HEXAGON_DYNAMIC_TAG(HEXAGON_VER, 0x70000001)
diff --git a/llvm/include/llvm/BinaryFormat/ELF.h b/llvm/include/llvm/BinaryFormat/ELF.h
index 3596174f74dde80..584c9efbf2b3f94 100644
--- a/llvm/include/llvm/BinaryFormat/ELF.h
+++ b/llvm/include/llvm/BinaryFormat/ELF.h
@@ -1056,6 +1056,9 @@ enum : unsigned {
   SHT_ARM_ATTRIBUTES = 0x70000003U,
   SHT_ARM_DEBUGOVERLAY = 0x70000004U,
   SHT_ARM_OVERLAYSECTION = 0x70000005U,
+  // Special aarch64-specific section for MTE support, as described in:
+  // https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#section-types
+  SHT_AARCH64_AUTH_RELR = 0x70000004U,
   // Special aarch64-specific sections for MTE support, as described in:
   // https://github.com/ARM-software/abi-aa/blob/main/memtagabielf64/memtagabielf64.rst#7section-types
   SHT_AARCH64_MEMTAG_GLOBALS_STATIC = 0x70000007U,
@@ -1643,6 +1646,11 @@ enum {
   NT_ANDROID_TYPE_MEMTAG = 4,
 };
 
+// ARM note types
+enum {
+  NT_ARM_TYPE_PAUTH_ABI_TAG = 1,
+};
+
 // Memory tagging values used in NT_ANDROID_TYPE_MEMTAG notes.
 enum {
   // Enumeration to determine the tagging mode. In Android-land, 'SYNC' means
diff --git a/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def b/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
index b507109b19e1b90..30375de420e3024 100644
--- a/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
+++ b/llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def
@@ -121,6 +121,7 @@ ELF_RELOC(R_AARCH64_TLSLE_LDST128_TPREL_LO12,        0x23a)
 ELF_RELOC(R_AARCH64_TLSLE_LDST128_TPREL_LO12_NC,     0x23b)
 ELF_RELOC(R_AARCH64_TLSLD_LDST128_DTPREL_LO12,       0x23c)
 ELF_RELOC(R_AARCH64_TLSLD_LDST128_DTPREL_LO12_NC,    0x23d)
+ELF_RELOC(R_AARCH64_AUTH_ABS64,                      0x244)
 // Dynamic relocations start
 ELF_RELOC(R_AARCH64_COPY,                            0x400)
 ELF_RELOC(R_AARCH64_GLOB_DAT,                        0x401)
@@ -134,7 +135,7 @@ ELF_RELOC(R_AARCH64_TLS_DTPREL64,                    0x405)
 ELF_RELOC(R_AARCH64_TLS_TPREL64,                     0x406)
 ELF_RELOC(R_AARCH64_TLSDESC,                         0x407)
 ELF_RELOC(R_AARCH64_IRELATIVE,                       0x408)
-ELF_RELOC(R_AARCH64_AUTH_ABS64,                      0xe100)
+ELF_RELOC(R_AARCH64_AUTH_RELATIVE,                   0x411)
 
 // ELF_RELOC(R_AARCH64_P32_NONE,                         0)
 ELF_RELOC(R_AARCH64_P32_ABS32,                       0x001)
diff --git a/llvm/lib/Object/ELF.cpp b/llvm/lib/Object/ELF.cpp
index 0d1862e573712f5..b717f34066e8f0a 100644
--- a/llvm/lib/Object/ELF.cpp
+++ b/llvm/lib/Object/ELF.cpp
@@ -299,6 +299,7 @@ StringRef llvm::object::getELFSectionTypeName(uint32_t Machine, unsigned Type) {
     STRINGIFY_ENUM_CASE(ELF, SHT_GROUP);
     STRINGIFY_ENUM_CASE(ELF, SHT_SYMTAB_SHNDX);
     STRINGIFY_ENUM_CASE(ELF, SHT_RELR);
+    STRINGIFY_ENUM_CASE(ELF, SHT_AARCH64_AUTH_RELR);
     STRINGIFY_ENUM_CASE(ELF, SHT_ANDROID_REL);
     STRINGIFY_ENUM_CASE(ELF, SHT_ANDROID_RELA);
     STRINGIFY_ENUM_CASE(ELF, SHT_ANDROID_RELR);
diff --git a/llvm/lib/ObjectYAML/ELFYAML.cpp b/llvm/lib/ObjectYAML/ELFYAML.cpp
index 872b89420a9e7a7..494819724f057fa 100644
--- a/llvm/lib/ObjectYAML/ELFYAML.cpp
+++ b/llvm/lib/ObjectYAML/ELFYAML.cpp
@@ -670,6 +670,7 @@ void ScalarEnumerationTraits<ELFYAML::ELF_SHT>::enumeration(
   ECase(SHT_GROUP);
   ECase(SHT_SYMTAB_SHNDX);
   ECase(SHT_RELR);
+  ECase(SHT_AARCH64_AUTH_RELR);
   ECase(SHT_ANDROID_REL);
   ECase(SHT_ANDROID_RELA);
   ECase(SHT_ANDROID_RELR);
diff --git a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
index 1ce008117ac3020..3bd8f5c19932ee9 100644
--- a/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
+++ b/llvm/test/MC/AArch64/elf-reloc-ptrauth.s
@@ -5,14 +5,14 @@
 
 // RELOC: Relocation section '.rela.test' at offset 0x230 contains 8 entries:
 // RELOC-NEXT:  Offset Info Type Symbol's Value Symbol's Name + Addend
-// RELOC-NEXT: 0000000000000000 000000010000e100 R_AARCH64_AUTH_ABS64 0000000000000000 .helper + 0
-// RELOC-NEXT: 0000000000000010 000000080000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g1 + 0
-// RELOC-NEXT: 0000000000000020 000000090000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g2 + 0
-// RELOC-NEXT: 0000000000000030 0000000a0000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g3 + 0
-// RELOC-NEXT: 0000000000000040 0000000b0000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g4 + 7
-// RELOC-NEXT: 0000000000000050 0000000c0000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g5 - 3
-// RELOC-NEXT: 0000000000000060 000000020000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g 6 + 0
-// RELOC-NEXT: 0000000000000070 0000000d0000e100 R_AARCH64_AUTH_ABS64 0000000000000000 _g 7 + 7
+// RELOC-NEXT: 0000000000000000 0000000100000244 R_AARCH64_AUTH_ABS64 0000000000000000 .helper + 0
+// RELOC-NEXT: 0000000000000010 0000000800000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g1 + 0
+// RELOC-NEXT: 0000000000000020 0000000900000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g2 + 0
+// RELOC-NEXT: 0000000000000030 0000000a00000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g3 + 0
+// RELOC-NEXT: 0000000000000040 0000000b00000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g4 + 7
+// RELOC-NEXT: 0000000000000050 0000000c00000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g5 - 3
+// RELOC-NEXT: 0000000000000060 0000000200000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g 6 + 0
+// RELOC-NEXT: 0000000000000070 0000000d00000244 R_AARCH64_AUTH_ABS64 0000000000000000 _g 7 + 7
 
 // RELOC: Hex dump of section '.test':
 //                VVVVVVVV addend, not needed for rela
diff --git a/llvm/test/tools/llvm-objdump/ELF/dynamic-section-machine-specific.test b/llvm/test/tools/llvm-objdump/ELF/dynamic-section-machine-specific.test
index 20219dd4893b77c..203c210eb46a3ee 100644
--- a/llvm/test/tools/llvm-objdump/ELF/dynamic-section-machine-specific.test
+++ b/llvm/test/tools/llvm-objdump/ELF/dynamic-section-machine-specific.test
@@ -268,6 +268,9 @@ ProgramHeaders:
 # AARCH64:      Dynamic Section:
 # AARCH64-NEXT:  AARCH64_BTI_PLT      0x0000000000000001
 # AARCH64-NEXT:  AARCH64_PAC_PLT      0x0000000000000002
+# AARCH64-NEXT:  AARCH64_AUTH_RELR    0x0000000000000003
+# AARCH64-NEXT:  AARCH64_AUTH_RELRSZ  0x0000000000000004
+# AARCH64-NEXT:  AARCH64_AUTH_RELRENT 0x0000000000000005
 
 --- !ELF
 FileHeader:
@@ -283,6 +286,12 @@ Sections:
         Value: 1
       - Tag:   DT_AARCH64_PAC_PLT
         Value: 2
+      - Tag:   DT_AARCH64_AUTH_RELR
+        Value: 3
+      - Tag:   DT_AARCH64_AUTH_RELRSZ
+        Value: 4
+      - Tag:   DT_AARCH64_AUTH_RELRENT
+        Value: 5
       - Tag:   DT_NULL
         Value: 0
 ProgramHeaders:
diff --git a/llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s b/llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s
new file mode 100644
index 000000000000000..1bbc6729bc702e6
--- /dev/null
+++ b/llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s
@@ -0,0 +1,98 @@
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag.s       -o tag.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag-short.s -o tag-short.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag-long.s  -o tag-long.o
+
+# RUN: llvm-readelf --notes tag.o       | FileCheck --check-prefix NORMAL %s
+# RUN: llvm-readelf --notes tag-short.o | FileCheck --check-prefix SHORT  %s
+# RUN: llvm-readelf --notes tag-long.o  | FileCheck --check-prefix LONG   %s
+
+# NORMAL: AArch64 PAuth ABI tag: platform 0x2a, version 0x1
+# SHORT:  AArch64 PAuth ABI tag: <corrupted size: expected at least 16, got 12>
+# LONG:   AArch64 PAuth ABI tag: platform 0x2a, version 0x1, additional info 0xEFCDAB8967452301
+
+# RUN: llvm-readobj --notes tag.o       | FileCheck --check-prefix LLVM-NORMAL %s
+# RUN: llvm-readobj --notes tag-short.o | FileCheck --check-prefix LLVM-SHORT %s
+# RUN: llvm-readobj --notes tag-long.o  | FileCheck --check-prefix LLVM-LONG %s
+
+// LLVM-SHORT:      Notes [
+// LLVM-SHORT-NEXT:   NoteSection {
+// LLVM-SHORT-NEXT:     Name: .note.AARCH64-PAUTH-ABI-tag
+// LLVM-SHORT-NEXT:     Offset: 0x40
+// LLVM-SHORT-NEXT:     Size: 0x1C
+// LLVM-SHORT-NEXT:     Note {
+// LLVM-SHORT-NEXT:       Owner: ARM
+// LLVM-SHORT-NEXT:       Data size: 0xC
+// LLVM-SHORT-NEXT:       Type: NT_ARM_TYPE_PAUTH_ABI_TAG
+// LLVM-SHORT-NEXT:       Description data (
+// LLVM-SHORT-NEXT:         0000: 2A000000 00000000 01000000
+// LLVM-SHORT-NEXT:       )
+// LLVM-SHORT-NEXT:     }
+// LLVM-SHORT-NEXT:   }
+// LLVM-SHORT-NEXT: ]
+
+// LLVM-NORMAL:      Notes [
+// LLVM-NORMAL-NEXT:   NoteSection {
+// LLVM-NORMAL-NEXT:     Name: .note.AARCH64-PAUTH-ABI-tag
+// LLVM-NORMAL-NEXT:     Offset: 0x40
+// LLVM-NORMAL-NEXT:     Size: 0x20
+// LLVM-NORMAL-NEXT:     Note {
+// LLVM-NORMAL-NEXT:       Owner: ARM
+// LLVM-NORMAL-NEXT:       Data size: 0x10
+// LLVM-NORMAL-NEXT:       Type: NT_ARM_TYPE_PAUTH_ABI_TAG
+// LLVM-NORMAL-NEXT:       Platform: 42
+// LLVM-NORMAL-NEXT:       Version: 1
+// LLVM-NORMAL-NEXT:     }
+// LLVM-NORMAL-NEXT:   }
+// LLVM-NORMAL-NEXT: ]
+
+// LLVM-LONG:      Notes [
+// LLVM-LONG-NEXT:   NoteSection {
+// LLVM-LONG-NEXT:     Name: .note.AARCH64-PAUTH-ABI-tag
+// LLVM-LONG-NEXT:     Offset: 0x40
+// LLVM-LONG-NEXT:     Size: 0x28
+// LLVM-LONG-NEXT:     Note {
+// LLVM-LONG-NEXT:       Owner: ARM
+// LLVM-LONG-NEXT:       Data size: 0x18
+// LLVM-LONG-NEXT:       Type: NT_ARM_TYPE_PAUTH_ABI_TAG
+// LLVM-LONG-NEXT:       Platform: 42
+// LLVM-LONG-NEXT:       Version: 1
+// LLVM-LONG-NEXT:       Additional info: EFCDAB8967452301
+// LLVM-LONG-NEXT:     }
+// LLVM-LONG-NEXT:   }
+// LLVM-LONG-NEXT: ]
+
+#--- abi-tag.s
+
+.section ".note.AARCH64-PAUTH-ABI-tag", "a"
+.long 4
+.long 16
+.long 1
+.asciz "ARM"
+
+.quad 42         // platform
+.quad 1          // version
+
+#--- abi-tag-short.s
+
+.section ".note.AARCH64-PAUTH-ABI-tag", "a"
+.long 4
+.long 12
+.long 1
+.asciz "ARM"
+
+.quad 42
+.word 1
+
+#--- abi-tag-long.s
+
+.section ".note.AARCH64-PAUTH-ABI-tag", "a"
+.long 4
+.long 24
+.long 1
+.asciz "ARM"
+
+.quad 42         // platform
+.quad 1          // version
+.quad 0x0123456789ABCDEF // extra data
diff --git a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
index 1a1c6dd4d0d1c22..06accfbf88236f8 100644
--- a/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
+++ b/llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test
@@ -138,19 +138,19 @@ ProgramHeaders:
 # RUN: llvm-readobj --dyn-relocations %t7 2>&1 | FileCheck %s -DFILE=%t7 --check-prefix=INVALID-DT-ANDROID-RELRSZ
 # RUN: llvm-readelf --dyn-relocations %t7 2>&1 | FileCheck %s -DFILE=%t7 --check-prefix=INVALID-DT-ANDROID-RELRSZ
 
-## INVALID-DT-RELRSZ:         warning: '[[FILE]]': invalid DT_RELRSZ value (0xff) or DT_RELRENT value (0x18)
-## INVALID-DT-ANDROID-RELRSZ: warning: '[[FILE]]': invalid DT_ANDROID_RELRSZ value (0xff) or DT_ANDROID_RELRENT value (0x18)
+## INVALID-DT-RELRSZ:              warning: '[[FILE]]': invalid DT_RELRSZ value (0xff) or DT_RELRENT value (0x18)
+## INVALID-DT-ANDROID-RELRSZ:      warning: '[[FILE]]': invalid DT_ANDROID_RELRSZ value (0xff) or DT_ANDROID_RELRENT value (0x18)
 
 ## Show we print a warning for an invalid relocation table entry size stored in a DT_RELRENT/DT_ANDROID_RELRENT entry.
-# RUN: yaml2obj --docnum=2 -DRELTYPE=RELR -DTAG1=DT_RELRSZ -DTAG2=DT_RELRENT -DTAG2VAL=0xFF %s -o %t8
-# RUN: llvm-readobj --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-RELRENT
-# RUN: llvm-readelf --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-RELRENT
-# RUN: yaml2obj --docnum=2 -DRELTYPE=RELR -DTAG1=DT_ANDROID_RELRSZ -DTAG2=DT_ANDROID_RELRENT -DTAG2VAL=0xFF %s -o %t9
-# RUN: llvm-readobj --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-ANDROID-RELRENT
-# RUN: llvm-readelf --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-ANDROID-RELRENT
+# RUN: yaml2obj --docnum=2 -DRELTYPE=RELR -DTAG1=DT_RELRSZ -DTAG2=DT_RELRENT -DTAG2VAL=0xFF %s -o %t9
+# RUN: llvm-readobj --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-RELRENT
+# RUN: llvm-readelf --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-RELRENT
+# RUN: yaml2obj --docnum=2 -DRELTYPE=RELR -DTAG1=DT_ANDROID_RELRSZ -DTAG2=DT_ANDROID_RELRENT -DTAG2VAL=0xFF %s -o %t10
+# RUN: llvm-readobj --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-ANDROID-RELRENT
+# RUN: llvm-readelf --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-ANDROID-RELRENT
 
-## INVALID-DT-RELRENT:         invalid DT_RELRSZ value (0x18) or DT_RELRENT value (0xff)
-## INVALID-DT-ANDROID-RELRENT: invalid DT_ANDROID_RELRSZ value (0x18) or DT_ANDROID_RELRENT value (0xff)
+## INVALID-DT-RELRENT:              invalid DT_RELRSZ value (0x18) or DT_RELRENT value (0xff)
+## INVALID-DT-ANDROID-RELRENT:      invalid DT_ANDROID_RELRSZ value (0x18) or DT_ANDROID_RELRENT value (0xff)
 
 ## Show we print a warning for an invalid value of DT_PLTRELSZ, which describes the total size
 ## of the relocation entries associated with the procedure linkage table.
@@ -459,3 +459,43 @@ ProgramHeaders:
   - Type:     PT_DYNAMIC
     FirstSec: .dynamic
     LastSec:  .dynamic
+
+## Show we print a warning for an invalid relocation table size stored in a DT_AARCH64_AUTH_RELRSZ entry.
+# RUN: yaml2obj --docnum=8 -DRELTYPE=RELR -DTAG1=DT_AARCH64_AUTH_RELRSZ -DTAG1VAL=0xFF -DTAG2=DT_AARCH64_AUTH_RELRENT %s -o %t8
+# RUN: llvm-readobj --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-AARCH64-AUTH-RELRSZ
+# RUN: llvm-readelf --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-AARCH64-AUTH-RELRSZ
+
+## INVALID-DT-AARCH64-AUTH-RELRSZ: warning: '[[FILE]]': invalid DT_AARCH64_AUTH_RELRSZ value (0xff) or DT_AARCH64_AUTH_RELRENT value (0x18)
+
+## Show we print a warning for an invalid relocation table entry size stored in a DT_AARCH64_AUTH_RELRENT entry.
+# RUN: yaml2obj --docnum=8 -DRELTYPE=RELR -DTAG1=DT_AARCH64_AUTH_RELRSZ -DTAG2=DT_AARCH64_AUTH_RELRENT -DTAG2VAL=0xFF %s -o %t11
+# RUN: llvm-readobj --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=INVALID-DT-AARCH64-AUTH-RELRENT
+# RUN: llvm-readelf --dyn-relocations %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=INVALID-DT-AARCH64-AUTH-RELRENT
+
+## INVALID-DT-AARCH64-AUTH-RELRENT: invalid DT_AARCH64_AUTH_RELRSZ value (0x18) or DT_AARCH64_AUTH_RELRENT value (0xff)
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_DYN
+  Machine: EM_AARCH64
+Sections:
+  - Name:  .relx.dyn
+    Type:  SHT_[[RELTYPE]]
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_[[RELTYPE]]
+        Value: 0x0
+      - Tag:   [[TAG1]]
+        Value: [[TAG1VAL=0x18]]
+      - Tag:   [[TAG2]]
+        Value: [[TAG2VAL=0x18]]
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:     PT_LOAD
+    FirstSec: .relx.dyn
+    LastSec:  .dynamic
diff --git a/llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test b/llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test
index c32ea33b9b3cbf6..e7bd9cf9b48b233 100644
--- a/llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test
+++ b/llvm/test/tools/llvm-readobj/ELF/dynamic-tags-machine-specific.test
@@ -355,20 +355,26 @@ ProgramHeaders:
 # RUN: llvm-readobj --dynamic-table %t.aarch64 | FileCheck %s --check-prefix=LLVM-AARCH64
 # RUN: llvm-readelf --dynamic-table %t.aarch64 | FileCheck %s --check-prefix=GNU-AARCH64
 
-# LLVM-AARCH64:     DynamicSection [ (4 entries)
+# LLVM-AARCH64:     DynamicSection [ (7 entries)
 # LLVM-AARCH64-NEXT:  Tag                Type                Name/Value
-# LLVM-AARCH64-NEXT:  0x0000000070000001 AARCH64_BTI_PLT     1
-# LLVM-AARCH64-NEXT:  0x0000000070000003 AARCH64_PAC_PLT     2
-# LLVM-AARCH64-NEXT:  0x0000000070000005 AARCH64_VARIANT_PCS 3
-# LLVM-AARCH64-NEXT:  0x0000000000000000 NULL                0x0
+# LLVM-AARCH64-NEXT:  0x0000000070000001 AARCH64_BTI_PLT      1
+# LLVM-AARCH64-NEXT:  0x0000000070000003 AARCH64_PAC_PLT      2
+# LLVM-AARCH64-NEXT:  0x0000000070000005 AARCH64_VARIANT_PCS  3
+# LLVM-AARCH64-NEXT:  0x0000000070000012 AARCH64_AUTH_RELR    0x4
+# LLVM-AARCH64-NEXT:  0x0000000070000011 AARCH64_AUTH_RELRSZ  5
+# LLVM-AARCH64-NEXT:  0x0000000070000013 AARCH64_AUTH_RELRENT 6
+# LLVM-AARCH64-NEXT:  0x0000000000000000 NULL                 0x0
 # LLVM-AARCH64-NEXT:]
 
-# GNU-AARCH64:      Dynamic section at offset {{.*}} contains 4 entries:
+# GNU-AARCH64:      Dynamic section at offset {{.*}} contains 7 entries:
 # GNU-AARCH64-NEXT:  Tag                Type                  Name/Value
-# GNU-AARCH64-NEXT:  0x0000000070000001 (AARCH64_BTI_PLT)     1
-# GNU-AARCH64-NEXT:  0x0000000070000003 (AARCH64_PAC_PLT)     2
-# GNU-AARCH64-NEXT:  0x0000000070000005 (AARCH64_VARIANT_PCS) 3
-# GNU-AARCH64-NEXT:  0x0000000000000000 (NULL)                0x0
+# GNU-AARCH64-NEXT:  0x0000000070000001 (AARCH64_BTI_PLT)      1
+# GNU-AARCH64-NEXT:  0x0000000070000003 (AARCH64_PAC_PLT)      2
+# GNU-AARCH64-NEXT:  0x0000000070000005 (AARCH64_VARIANT_PCS)  3
+# GNU-AARCH64-NEXT:  0x0000000070000012 (AARCH64_AUTH_RELR)    0x4
+# GNU-AARCH64-NEXT:  0x0000000070000011 (AARCH64_AUTH_RELRSZ)  5
+# GNU-AARCH64-NEXT:  0x0000000070000013 (AARCH64_AUTH_RELRENT) 6
+# GNU-AARCH64-NEXT:  0x0000000000000000 (NULL)                 0x0
 
 --- !ELF
 FileHeader:
@@ -386,6 +392,12 @@ Sections:
         Value: 2
       - Tag:   DT_AARCH64_VARIANT_PCS
         Value: 3
+      - Tag:   DT_AARCH64_AUTH_RELR
+        Value: 4
+      - Tag:   DT_AARCH64_AUTH_RELRSZ
+        Value: 5
+      - Tag:   DT_AARCH64_AUTH_RELRENT
+        Value: 6
       - Tag:   DT_NULL
         Value: 0
 ProgramHeaders:
diff --git a/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test b/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
index 3bb54b1adc1f4fe..704676b60f01215 100644
--- a/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
+++ b/llvm/test/tools/llvm-readobj/ELF/relr-relocs.test
@@ -158,7 +158,7 @@ Sections:
     Link:    [[LINK=<none>]]
 
 ## Check we report a warning when we are unable to dump relocations
-## for a SHT_RELR/SHT_ANDROID_RELR section.
+## for a SHT_RELR/SHT_ANDROID_RELR/SHT_AARCH64_AUTH_RELR section.
 
 ## Case A: check the case when relocations can't be read from an SHT_RELR section.
 # RUN: yaml2obj --docnum=2 -DENTSIZE=1 %s -o %t2.broken
@@ -186,7 +186,25 @@ Sections:
 # RUN: llvm-readelf --relocations %t2.broken.android 2>&1 | \
 # RUN:   FileCheck -DFILE=%t2.broken.android --check-prefix=BROKEN-GNU %s -DSECNAME=SHT_ANDROID_RELR
 
-## Check the behavior when the sh_link field of the SHT_RELR/SHT_ANDROID_RELR section
+## Case C: check the case when relocations can't be read from an SHT_AARCH64_AUTH_RELR section.
+##         SHT_AARCH64_AUTH_RELR = 0x70000004.
+# RUN: yaml2obj --docnum=2 -DENTSIZE=1 -DSHTYPE=0x70000004 %s -o %t2.broken.aarch64auth
+# RUN: llvm-readobj --relocations %t2.broken.aarch64auth 2>&1 | \
+# RUN:   FileCheck -DFILE=%t2.broken.aarch64auth --check-prefix=BROKEN-LLVM-AARCH64-AUTH %s -DSECNAME=SHT_AARCH64_AUTH_RELR
+# RUN: llvm-readelf --relocations %t2.broken.aarch64auth 2>&1 | \
+# RUN:   FileCheck -DFILE=%t2.broken.aarch64auth --check-prefix=BROKEN-GNU-AARCH64-AUTH %s -DSECNAME=SHT_AARCH64_AUTH_RELR
+
+# BROKEN-LLVM-AARCH64-AUTH:      Relocations [
+# BROKEN-LLVM-AA...
[truncated]

Copy link

github-actions bot commented Nov 17, 2023

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

llvm/include/llvm/BinaryFormat/ELF.h Outdated Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test Outdated Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test Outdated Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test Outdated Show resolved Hide resolved
llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test Outdated Show resolved Hide resolved
DynRelrRegion.SizePrintName = Dyn.d_tag == ELF::DT_RELRSZ
? "DT_RELRSZ value"
: "DT_ANDROID_RELRSZ value";
if (Dyn.d_tag == ELF::DT_RELRSZ)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably this should be another switch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, I agree that switch is better for choosing between enum values. However, we'll only have a single line in each case, and for each of them we'll also have a break. To avoid that, we might use immediately invoked function expression, but if else if was previously considered a better alternative in a discussion with @MaskRay here https://reviews.llvm.org/D156882?id=554796#inline-1548995.

If switch with additional break's still looks better than if else if - please re-open the thread and let me know, I'll change that.

Copy link
Member

Choose a reason for hiding this comment

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

For this case, I think it is better to do

if (emachine == EM_AARCH64 && xxx == ELF::DT_AARCH64_AUTH_RELRSZ) {
  DynRelrRegion.Size = Dyn.getVal();
  DynRelrRegion.SizePrintName = "DT_AARCH64_AUTH_RELRSZ value";
  continue;
}
switch (Dyn.d_tag) {
  ...
}

If emachine is not EM_AARCH64, DT_AARCH64_AUTH_RELRSZ is not defined.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed with @MaskRay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved checks for DT_AARCH64_AUTH_RELRSZ and DT_AARCH64_AUTH_RELRENT to an if before the main switch as you suggested. These two values are distinguished inside the if via inner switch with continue at the end of each case.

llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
@@ -1643,6 +1646,11 @@ enum {
NT_ANDROID_TYPE_MEMTAG = 4,
};

// ARM note types
enum {
NT_ARM_TYPE_PAUTH_ABI_TAG = 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I couldn't see this value defined in the linked spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I have a few (small) concerns with this:

  1. It's potentially possible (admittedly unlikely) that the name could be specified for something different in the future, so you'd have to modify existing sites to use a different name.
  2. Giving it a name means people will search for the string in the docs and not find it, potentially causing confusion in the future.

I don't know who is in charge of those docs, but is it possible to update them to specifically name the NT_* value? That would resolve my concern.

Copy link
Contributor Author

@kovdan01 kovdan01 Dec 6, 2023

Choose a reason for hiding this comment

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

@smithp35 Could we provide this constant name NT_ARM_TYPE_PAUTH_ABI_TAG (or some other name) and its value 1 in the section 14.1 of the spec https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#appendix-alternative-elf-marking-using-gnu-program-properties?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smithp35 A kind reminder regarding my question above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to make sure I've got this right:
The link above is for GNU program properties:
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#19appendix-alternative-elf-marking-using-gnu-program-properties

It defines an additional program property on top of the existing .note.GNU.property defined in the Linux ABI extensions https://raw.githubusercontent.com/wiki/hjl-tools/linux-abi/linux-abi-draft.pdf that already has its own NT_GNU_PROPERTY_TYPE_0

There is a part here which defines .note.AARCH64-PAUTH-ABI-tag which I think is where an additional NT_ARM_TYPE_PAUTH_ABI_TAG would make sense.
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#141default-marking-schema

Have I got that right?

In any case please can you raise an issue on the AArch64 ABI for that https://github.com/ARM-software/abi-aa so it can be addressed there. I'll also need to update the document to make it compatible with some forthcoming changes in the ABI to do with ELF marking. It is unfortunate that this will land too late for your timescale.

Essentially these pull-requests:

None have been merged yet, but pending more review comments there is a consensus within Arm that we'll be using build attributes (like 32-bit Arm) for ELF metadata in relocatable objects and for SysvAbi platforms such as Linux we'll be using .note.GNU.properties as our reference ELF marking scheme for executables and shared-libraries.

I expect that when the PAuthABI specification comes out of Alpha it will need to be updated, most likely to make the current Alternative ELF marking will be the reference marking, and the .note.AARCH64-PAUTH-ABI-tag will become the alternative. I've already defined build attibutes for PAuthABI that match the existing implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a part here which defines .note.AARCH64-PAUTH-ABI-tag which I think is where an additional NT_ARM_TYPE_PAUTH_ABI_TAG would make sense. https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#141default-marking-schema

Have I got that right?

Yes. It looks like I placed link a wrong link but a correct section number above :) I meant 14.1 https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#141default-marking-schema.

Submitted ARM-software/abi-aa#234. Thanks!

llvm/include/llvm/BinaryFormat/ELFRelocs/AArch64.def Outdated Show resolved Hide resolved
This patch adds llvm-readobj support for:

- Dynamic R_AARCH64_AUTH_* relocations (including RELR compressed AUTH
  relocations) as described here:
  https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#auth-variant-dynamic-relocations

- .note.AARCH64-PAUTH-ABI-tag section as defined here
  https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#elf-marking
@kovdan01
Copy link
Contributor Author

kovdan01 commented Dec 4, 2023

@jh7370 Thanks for your review, fixed the issues you've mentioned. Feel free to re-open resolved threads if something looks wrong for you.

@kovdan01
Copy link
Contributor Author

kovdan01 commented Dec 4, 2023

@MaskRay Would be glad to see you review on the changes

kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Dec 4, 2023
Comments use `##`. Lit and FileCheck directives should use single `#`. See
llvm#72713 (comment).
Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

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

Hi @kovdan01, two general process points:

  1. Please avoid force pushes unless it's absolutely necessary to rebase your PR: it makes it impossible to view the changes since my previous PR, and potentially has other issues too (it's also covered by the existing LLVM review policy).
  2. Please don't resolve coversations that I've initiated. I use this functionality as a checklist to ensure my comments have actually been addressed to my satisfaction, and if they're resolved, it makes it harder to track things. Please see my LLVM Discourse thread on the resolve conversation button for more details.

@@ -1643,6 +1646,11 @@ enum {
NT_ANDROID_TYPE_MEMTAG = 4,
};

// ARM note types
enum {
NT_ARM_TYPE_PAUTH_ABI_TAG = 1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I have a few (small) concerns with this:

  1. It's potentially possible (admittedly unlikely) that the name could be specified for something different in the future, so you'd have to modify existing sites to use a different name.
  2. Giving it a name means people will search for the string in the docs and not find it, potentially causing confusion in the future.

I don't know who is in charge of those docs, but is it possible to update them to specifically name the NT_* value? That would resolve my concern.

llvm/tools/llvm-readobj/ELFDumper.cpp Outdated Show resolved Hide resolved
DynRelrRegion.SizePrintName = Dyn.d_tag == ELF::DT_RELRSZ
? "DT_RELRSZ value"
: "DT_ANDROID_RELRSZ value";
if (Dyn.d_tag == ELF::DT_RELRSZ)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed with @MaskRay.

@kovdan01
Copy link
Contributor Author

kovdan01 commented Dec 6, 2023

Thanks @jh7370 and @MaskRay for your feedback and comments. Addressed the issues in 8d7ee46 - please consider reviewing the changes

Copy link
Collaborator

@jh7370 jh7370 left a comment

Choose a reason for hiding this comment

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

Looks good from my point of view. Please give @MaskRay a chance to comment.

# RUN: llvm-readobj --notes tag-short.o | FileCheck --check-prefix LLVM-SHORT %s
# RUN: llvm-readobj --notes tag-long.o | FileCheck --check-prefix LLVM-LONG %s

// LLVM-SHORT: Notes [
Copy link
Member

Choose a reason for hiding this comment

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

We should not mix # and //. Just use # everywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in b0c9a28, thanks

@@ -6208,6 +6263,13 @@ void ELFDumper<ELFT>::forEachRelocationDo(
Warn(RangeOrErr.takeError());
}
break;
case ELF::SHT_AARCH64_AUTH_RELR:
if (Obj.getHeader().e_machine != EM_AARCH64) {
this->reportUniqueWarning(
Copy link
Member

@MaskRay MaskRay Dec 6, 2023

Choose a reason for hiding this comment

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

we should drop this for non-AArch64. They may define the value for other purposes. The definition can be entirely unrelated.

Just use the default code path to handle an unknown section header type.

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 suppose a slightly different approach would be better if we have concerns regarding other targets defining the same section type for unrelated purposes. ELFDumper<ELFT>::forEachRelocationDo should only be called on sections which pass the check isRelocationSec<LEFT>(Sec). So, we can change this check and make it take the target into account - as for now, it returns true for SHT_AARCH64_AUTH_RESULT unconditionally. After such change, we can just use an assert(Obj.getHeader().e_machine == EM_AARCH64) under case ELF::SHT_AARCH64_AUTH_RESULT in ELFDumper<ELFT>::forEachRelocationDo to ensure the contract is satisfied.

@MaskRay Please let me know your thoughts on this.

Copy link
Member

Choose a reason for hiding this comment

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

I haven't checked, but if we can guarantee assert(Obj.getHeader().e_machine == EM_AARCH64). It's better than if (EM_AARCH64 && ELF::SHT_AARCH64_AUTH_RELR) do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It turns out that we can't assume that only relocation sections are passed to forEachRelocationDo - there are code paths when other section types also get there, for example, printSectionHeaders -> printRelocationsHelper -> forEachRelocationDo. So, we must break here on non-AArch64 emachine (the switch does not contain a default branch, so break looks OK for me).

I also added checks against emachine in other places where appropriate and moved code/tests to target-specific parts where needed. @MaskRay Please see f9103d5 and let me know if it is OK.

Copy link
Member

Choose a reason for hiding this comment

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

Testing EHeader.e_machine == EM_AARCH64 && SType == ELF::SHT_AARCH64_AUTH_RELR is reasonable.

@kovdan01 kovdan01 merged commit c8616c7 into llvm:main Dec 8, 2023
4 checks passed
kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Dec 8, 2023
kovdan01 added a commit that referenced this pull request Dec 8, 2023
kovdan01 added a commit that referenced this pull request Dec 8, 2023
kovdan01 added a commit that referenced this pull request Dec 8, 2023
Reapply #72713 after fixing formatted printing of
`uint64_t` values as hex (see failing build here
https://lab.llvm.org/buildbot/#/builders/186/builds/13604).

This patch adds llvm-readobj support for:

- Dynamic `R_AARCH64_AUTH_*` relocations (including RELR compressed AUTH
relocations) as described here:
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#auth-variant-dynamic-relocations

- `.note.AARCH64-PAUTH-ABI-tag` section as defined here
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#elf-marking
kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Dec 10, 2023
Comments use `##`. Lit and FileCheck directives should use single `#`. See
llvm#72713 (comment).
kovdan01 added a commit that referenced this pull request Dec 10, 2023
…4985)

Comments use `##`. Lit and FileCheck directives should use single `#`.
See
#72713 (comment).
kovdan01 added a commit that referenced this pull request Apr 4, 2024
#72714)

This patch adds lld support for:

- Dynamic R_AARCH64_AUTH_* relocations (without including RELR compressed AUTH
relocations) as described here:
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#auth-variant-dynamic-relocations

- .note.AARCH64-PAUTH-ABI-tag section as defined here
https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#elf-marking

Depends on #72713 and #85231

---------

Co-authored-by: Peter Collingbourne <[email protected]>
Co-authored-by: Fangrui Song <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants