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

[lld][AArch64][ELF][PAC] Support AUTH relocations and AUTH ELF marking #72714

Merged
merged 45 commits into from Apr 4, 2024

Conversation

kovdan01
Copy link
Contributor

@kovdan01 kovdan01 commented Nov 17, 2023

This patch adds lld support for:

Depends on #72713 and #87545

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 17, 2023

@llvm/pr-subscribers-lld-elf
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-lld

Author: Daniil Kovalev (kovdan01)

Changes

This patch adds lld support for:

Depends on #72713


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

11 Files Affected:

  • (modified) lld/ELF/Arch/AArch64.cpp (+5)
  • (modified) lld/ELF/Config.h (+4)
  • (modified) lld/ELF/Driver.cpp (+55-2)
  • (modified) lld/ELF/InputFiles.cpp (+44)
  • (modified) lld/ELF/InputFiles.h (+1)
  • (modified) lld/ELF/Relocations.cpp (+26)
  • (modified) lld/ELF/SyntheticSections.cpp (+38-6)
  • (modified) lld/ELF/SyntheticSections.h (+16-3)
  • (modified) lld/ELF/Writer.cpp (+17)
  • (added) lld/test/ELF/aarch64-feature-pauth.s (+83)
  • (added) lld/test/ELF/aarch64-ptrauth.s (+156)
diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index 048f0ec30ebd283..6828d3f57c10e84 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -112,6 +112,7 @@ RelExpr AArch64::getRelExpr(RelType type, const Symbol &s,
   case R_AARCH64_MOVW_UABS_G2:
   case R_AARCH64_MOVW_UABS_G2_NC:
   case R_AARCH64_MOVW_UABS_G3:
+  case R_AARCH64_AUTH_ABS64:
     return R_ABS;
   case R_AARCH64_TLSDESC_ADR_PAGE21:
     return R_AARCH64_TLSDESC_PAGE;
@@ -395,6 +396,10 @@ void AArch64::relocate(uint8_t *loc, const Relocation &rel,
   case R_AARCH64_PREL64:
     write64(loc, val);
     break;
+  case R_AARCH64_AUTH_ABS64:
+    checkIntUInt(loc, val, 32, rel);
+    write32(loc, val);
+    break;
   case R_AARCH64_ADD_ABS_LO12_NC:
     or32AArch64Imm(loc, val);
     break;
diff --git a/lld/ELF/Config.h b/lld/ELF/Config.h
index 56229334f9a44ae..1b633a79842769d 100644
--- a/lld/ELF/Config.h
+++ b/lld/ELF/Config.h
@@ -187,6 +187,7 @@ struct Config {
   llvm::StringRef cmseOutputLib;
   StringRef zBtiReport = "none";
   StringRef zCetReport = "none";
+  StringRef zPauthReport = "none";
   llvm::StringRef ltoBasicBlockSections;
   std::pair<llvm::StringRef, llvm::StringRef> thinLTOObjectSuffixReplace;
   llvm::StringRef thinLTOPrefixReplaceOld;
@@ -275,6 +276,7 @@ struct Config {
   bool relocatable;
   bool relrGlibc = false;
   bool relrPackDynRelocs = false;
+  bool relrPackAuthDynRelocs = false;
   llvm::DenseSet<llvm::StringRef> saveTempsArgs;
   llvm::SmallVector<std::pair<llvm::GlobPattern, uint32_t>, 0> shuffleSections;
   bool singleRoRx;
@@ -492,6 +494,8 @@ struct Ctx {
   void reset();
 
   llvm::raw_fd_ostream openAuxiliaryFile(llvm::StringRef, std::error_code &);
+
+  SmallVector<uint8_t, 0> aarch64PauthAbiTag;
 };
 
 LLVM_LIBRARY_VISIBILITY extern Ctx ctx;
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6290880c43d3b95..374b4143e9d02e3 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -65,6 +65,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/raw_ostream.h"
+#include <algorithm>
 #include <cstdlib>
 #include <tuple>
 #include <utility>
@@ -459,6 +460,8 @@ static void checkOptions() {
       error("-z force-bti only supported on AArch64");
     if (config->zBtiReport != "none")
       error("-z bti-report only supported on AArch64");
+    if (config->zPauthReport != "none")
+      error("-z pauth-report only supported on AArch64");
   }
 
   if (config->emachine != EM_386 && config->emachine != EM_X86_64 &&
@@ -558,6 +561,7 @@ constexpr const char *knownZFlags[] = {
     "nognustack",
     "nokeep-text-section-prefix",
     "nopack-relative-relocs",
+    "nopack-relative-auth-relocs",
     "norelro",
     "noseparate-code",
     "nostart-stop-gc",
@@ -566,6 +570,7 @@ constexpr const char *knownZFlags[] = {
     "origin",
     "pac-plt",
     "pack-relative-relocs",
+    "pack-relative-auth-relocs",
     "rel",
     "rela",
     "relro",
@@ -583,7 +588,7 @@ constexpr const char *knownZFlags[] = {
 static bool isKnownZFlag(StringRef s) {
   return llvm::is_contained(knownZFlags, s) ||
          s.starts_with("common-page-size=") || s.starts_with("bti-report=") ||
-         s.starts_with("cet-report=") ||
+         s.starts_with("cet-report=") || s.starts_with("pauth-report=") ||
          s.starts_with("dead-reloc-in-nonalloc=") ||
          s.starts_with("max-page-size=") || s.starts_with("stack-size=") ||
          s.starts_with("start-stop-visibility=");
@@ -1514,7 +1519,8 @@ static void readConfigs(opt::InputArgList &args) {
   }
 
   auto reports = {std::make_pair("bti-report", &config->zBtiReport),
-                  std::make_pair("cet-report", &config->zCetReport)};
+                  std::make_pair("cet-report", &config->zCetReport),
+                  std::make_pair("pauth-report", &config->zPauthReport)};
   for (opt::Arg *arg : args.filtered(OPT_z)) {
     std::pair<StringRef, StringRef> option =
         StringRef(arg->getValue()).split('=');
@@ -1671,6 +1677,9 @@ static void readConfigs(opt::InputArgList &args) {
         getPackDynRelocs(args);
   }
 
+  config->relrPackAuthDynRelocs = getZFlag(
+      args, "pack-relative-auth-relocs", "nopack-relative-auth-relocs", false);
+
   if (auto *arg = args.getLastArg(OPT_symbol_ordering_file)){
     if (args.hasArg(OPT_call_graph_ordering_file))
       error("--symbol-ordering-file and --call-graph-order-file "
@@ -2639,6 +2648,47 @@ static uint32_t getAndFeatures() {
   return ret;
 }
 
+static void getAarch64PauthInfo() {
+  if (ctx.objectFiles.empty())
+    return;
+
+  auto NonEmptyIt = std::find_if(
+      ctx.objectFiles.begin(), ctx.objectFiles.end(),
+      [](const ELFFileBase *f) { return !f->aarch64PauthAbiTag.empty(); });
+  if (NonEmptyIt == ctx.objectFiles.end())
+    return;
+
+  ctx.aarch64PauthAbiTag = (*NonEmptyIt)->aarch64PauthAbiTag;
+  StringRef F1 = (*NonEmptyIt)->getName();
+  for (ELFFileBase *F : ArrayRef(ctx.objectFiles)) {
+    StringRef F2 = F->getName();
+    const SmallVector<uint8_t, 0> &D1 = ctx.aarch64PauthAbiTag;
+    const SmallVector<uint8_t, 0> &D2 = F->aarch64PauthAbiTag;
+    if (D1.empty() != D2.empty()) {
+      auto Helper = [](StringRef Report, const Twine &Msg) {
+        if (Report == "warning")
+          warn(Msg);
+        else if (Report == "error")
+          error(Msg);
+      };
+
+      Helper(config->zPauthReport,
+             (D1.empty() ? F1.str() : F2.str()) +
+                 " has no AArch64 PAuth compatibility info while " +
+                 (D1.empty() ? F2.str() : F1.str()) +
+                 " has one; either all or no input files must have it");
+    }
+
+    if (!D1.empty() && !D2.empty() &&
+        !std::equal(D1.begin(), D1.end(), D2.begin(), D2.end()))
+      errorOrWarn(
+          "incompatible values of AArch64 PAuth compatibility info found"
+          "\n" +
+          F1 + ": 0x" + toHex(ArrayRef(D1.data(), D1.size())) + "\n" + F2 +
+          ": 0x" + toHex(ArrayRef(D2.data(), D2.size())));
+  }
+}
+
 static void initSectionsAndLocalSyms(ELFFileBase *file, bool ignoreComdats) {
   switch (file->ekind) {
   case ELF32LEKind:
@@ -2976,6 +3026,9 @@ void LinkerDriver::link(opt::InputArgList &args) {
   // contain a hint to tweak linker's and loader's behaviors.
   config->andFeatures = getAndFeatures();
 
+  if (config->emachine == EM_AARCH64)
+    getAarch64PauthInfo();
+
   // The Target instance handles target-specific stuff, such as applying
   // relocations or writing a PLT section. It also contains target-dependent
   // values such as a default image base address.
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index e83cca31105d489..cf7a9de330181f1 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -961,6 +961,44 @@ template <class ELFT> static uint32_t readAndFeatures(const InputSection &sec) {
   return featuresSet;
 }
 
+// Extract compatibility info for aarch64 pointer authentication from the
+// .note.AARCH64-PAUTH-ABI-tag section and write it to the corresponding ObjFile
+// field. See the following ABI documentation:
+// https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#elf-marking
+template <class ELFT>
+static void readAArch64PauthAbiTag(const InputSection &sec, ObjFile<ELFT> &f) {
+  using Elf_Nhdr = typename ELFT::Nhdr;
+  using Elf_Note = typename ELFT::Note;
+  ArrayRef<uint8_t> data = sec.content();
+  auto reportError = [&](const Twine &msg) {
+    errorOrWarn(toString(sec.file) + ":(" + sec.name + "): " + msg);
+  };
+
+  auto *nhdr = reinterpret_cast<const Elf_Nhdr *>(data.data());
+  if (data.size() < sizeof(Elf_Nhdr) ||
+      data.size() < nhdr->getSize(sec.addralign)) {
+    reportError("section is too short");
+    return;
+  }
+
+  Elf_Note note(*nhdr);
+  if (nhdr->n_type != NT_ARM_TYPE_PAUTH_ABI_TAG)
+    reportError("invalid type field value " + Twine(nhdr->n_type) + " (" +
+                Twine(NT_ARM_TYPE_PAUTH_ABI_TAG) + " expected)");
+  if (note.getName() != "ARM")
+    reportError("invalid name field value " + note.getName() +
+                " (ARM expected)");
+
+  ArrayRef<uint8_t> desc = note.getDesc(sec.addralign);
+  if (desc.size() < 16) {
+    reportError("too short AArch64 PAuth compatibility info "
+                "(at least 16 bytes expected)");
+    return;
+  }
+
+  f.aarch64PauthAbiTag = SmallVector<uint8_t, 0>(iterator_range(desc));
+}
+
 template <class ELFT>
 InputSectionBase *ObjFile<ELFT>::getRelocTarget(uint32_t idx,
                                                 const Elf_Shdr &sec,
@@ -1019,6 +1057,12 @@ InputSectionBase *ObjFile<ELFT>::createInputSection(uint32_t idx,
       return &InputSection::discarded;
     }
 
+    if (config->emachine == EM_AARCH64 &&
+        name == ".note.AARCH64-PAUTH-ABI-tag") {
+      readAArch64PauthAbiTag<ELFT>(InputSection(*this, sec, name), *this);
+      return &InputSection::discarded;
+    }
+
     // Split stacks is a feature to support a discontiguous stack,
     // commonly used in the programming language Go. For the details,
     // see https://gcc.gnu.org/wiki/SplitStacks. An object file compiled
diff --git a/lld/ELF/InputFiles.h b/lld/ELF/InputFiles.h
index ab98d78fcf1455a..6a74ba7fb20998c 100644
--- a/lld/ELF/InputFiles.h
+++ b/lld/ELF/InputFiles.h
@@ -218,6 +218,7 @@ class ELFFileBase : public InputFile {
 public:
   uint32_t andFeatures = 0;
   bool hasCommonSyms = false;
+  SmallVector<uint8_t, 0> aarch64PauthAbiTag;
 };
 
 // .o file.
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index fe3d7f419e84aa6..5b5e6b154d52f42 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1444,6 +1444,32 @@ template <class ELFT, class RelTy> void RelocationScanner::scanOne(RelTy *&i) {
     }
   }
 
+  if (config->emachine == EM_AARCH64 && type == R_AARCH64_AUTH_ABS64) {
+    // Assume relocations from relocatable objects are RELA.
+    assert(RelTy::IsRela);
+    std::lock_guard<std::mutex> lock(relocMutex);
+    // For a preemptible symbol, we can't use a relative relocation. For an
+    // undefined symbol, we can't compute offset at link-time and use a relative
+    // relocation. Use a symbolic relocation instead.
+    Partition &part = sec->getPartition();
+    if (sym.isPreemptible || sym.isUndefined()) {
+      part.relaDyn->addSymbolReloc(type, *sec, offset, sym, addend, type);
+    } else if (part.relrAuthDyn && sec->addralign >= 2 && offset % 2 == 0 &&
+               isInt<32>(sym.getVA(addend))) {
+      // Implicit addend is below 32-bits so we can use the compressed
+      // relative relocation section. The R_AARCH64_AUTH_RELATIVE
+      // has a smaller addend fielf as bits [63:32] encode the signing-schema.
+      sec->addReloc({expr, type, offset, addend, &sym});
+      part.relrAuthDyn->relocsVec[parallel::getThreadIndex()].push_back(
+          {sec, offset});
+    } else {
+      part.relaDyn->addReloc({R_AARCH64_AUTH_RELATIVE, sec, offset,
+                              DynamicReloc::AddendOnlyWithTargetVA, sym, addend,
+                              R_ABS});
+    }
+    return;
+  }
+
   // If the relocation does not emit a GOT or GOTPLT entry but its computation
   // uses their addresses, we need GOT or GOTPLT to be created.
   //
diff --git a/lld/ELF/SyntheticSections.cpp b/lld/ELF/SyntheticSections.cpp
index 2b32eb3a0fe3558..fa7589806a7b5b0 100644
--- a/lld/ELF/SyntheticSections.cpp
+++ b/lld/ELF/SyntheticSections.cpp
@@ -331,6 +331,29 @@ void GnuPropertySection::writeTo(uint8_t *buf) {
 
 size_t GnuPropertySection::getSize() const { return config->is64 ? 32 : 28; }
 
+AArch64PauthAbiTag::AArch64PauthAbiTag()
+    : SyntheticSection(llvm::ELF::SHF_ALLOC, llvm::ELF::SHT_NOTE,
+                       config->wordsize, ".note.AARCH64-PAUTH-ABI-tag") {}
+
+bool AArch64PauthAbiTag::isNeeded() const {
+  return !ctx.aarch64PauthAbiTag.empty();
+}
+
+void AArch64PauthAbiTag::writeTo(uint8_t *buf) {
+  const SmallVector<uint8_t, 0> &data = ctx.aarch64PauthAbiTag;
+  write32(buf, 4);                             // Name size
+  write32(buf + 4, data.size());               // Content size
+  write32(buf + 8, NT_ARM_TYPE_PAUTH_ABI_TAG); // Type
+  memcpy(buf + 12, "ARM", 4);                  // Name string
+  memcpy(buf + 16, data.data(), data.size());
+  memset(buf + 16 + data.size(), 0, getSize() - 16 - data.size()); // Padding
+}
+
+size_t AArch64PauthAbiTag::getSize() const {
+  return alignToPowerOf2(16 + ctx.aarch64PauthAbiTag.size(),
+                         config->is64 ? 8 : 4);
+}
+
 BuildIdSection::BuildIdSection()
     : SyntheticSection(SHF_ALLOC, SHT_NOTE, 4, ".note.gnu.build-id"),
       hashSize(getHashSize()) {}
@@ -1406,6 +1429,12 @@ DynamicSection<ELFT>::computeContents() {
     addInt(config->useAndroidRelrTags ? DT_ANDROID_RELRENT : DT_RELRENT,
            sizeof(Elf_Relr));
   }
+  if (part.relrAuthDyn && part.relrAuthDyn->getParent() &&
+      !part.relrAuthDyn->relocs.empty()) {
+    addInSec(DT_AARCH64_AUTH_RELR, *part.relrAuthDyn);
+    addInt(DT_AARCH64_AUTH_RELRSZ, part.relrAuthDyn->getParent()->size);
+    addInt(DT_AARCH64_AUTH_RELRENT, sizeof(Elf_Relr));
+  }
   // .rel[a].plt section usually consists of two parts, containing plt and
   // iplt relocations. It is possible to have only iplt relocations in the
   // output. In that case relaPlt is empty and have zero offset, the same offset
@@ -1717,10 +1746,13 @@ template <class ELFT> void RelocationSection<ELFT>::writeTo(uint8_t *buf) {
   }
 }
 
-RelrBaseSection::RelrBaseSection(unsigned concurrency)
-    : SyntheticSection(SHF_ALLOC,
-                       config->useAndroidRelrTags ? SHT_ANDROID_RELR : SHT_RELR,
-                       config->wordsize, ".relr.dyn"),
+RelrBaseSection::RelrBaseSection(unsigned concurrency, bool isAArch64Auth)
+    : SyntheticSection(
+          SHF_ALLOC,
+          isAArch64Auth
+              ? SHT_AARCH64_AUTH_RELR
+              : (config->useAndroidRelrTags ? SHT_ANDROID_RELR : SHT_RELR),
+          config->wordsize, isAArch64Auth ? ".relr.auth.dyn" : ".relr.dyn"),
       relocsVec(concurrency) {}
 
 void RelrBaseSection::mergeRels() {
@@ -1988,8 +2020,8 @@ bool AndroidPackedRelocationSection<ELFT>::updateAllocSize() {
 }
 
 template <class ELFT>
-RelrSection<ELFT>::RelrSection(unsigned concurrency)
-    : RelrBaseSection(concurrency) {
+RelrSection<ELFT>::RelrSection(unsigned concurrency, bool isAArch64Auth)
+    : RelrBaseSection(concurrency, isAArch64Auth) {
   this->entsize = config->wordsize;
 }
 
diff --git a/lld/ELF/SyntheticSections.h b/lld/ELF/SyntheticSections.h
index 3a9f4ba886f6bbb..d183a547c682051 100644
--- a/lld/ELF/SyntheticSections.h
+++ b/lld/ELF/SyntheticSections.h
@@ -144,6 +144,16 @@ class GnuPropertySection final : public SyntheticSection {
   size_t getSize() const override;
 };
 
+// .note.AARCH64-PAUTH-ABI-tag section. See
+// https://github.com/ARM-software/abi-aa/blob/main/pauthabielf64/pauthabielf64.rst#elf-marking
+class AArch64PauthAbiTag final : public SyntheticSection {
+public:
+  AArch64PauthAbiTag();
+  void writeTo(uint8_t *buf) override;
+  size_t getSize() const override;
+  bool isNeeded() const override;
+};
+
 // .note.gnu.build-id section.
 class BuildIdSection : public SyntheticSection {
   // First 16 bytes are a header.
@@ -543,7 +553,8 @@ class RelocationBaseSection : public SyntheticSection {
   static bool classof(const SectionBase *d) {
     return SyntheticSection::classof(d) &&
            (d->type == llvm::ELF::SHT_RELA || d->type == llvm::ELF::SHT_REL ||
-            d->type == llvm::ELF::SHT_RELR);
+            d->type == llvm::ELF::SHT_RELR ||
+            d->type == llvm::ELF::SHT_AARCH64_AUTH_RELR);
   }
   int32_t dynamicTag, sizeDynamicTag;
   SmallVector<DynamicReloc, 0> relocs;
@@ -599,7 +610,7 @@ struct RelativeReloc {
 
 class RelrBaseSection : public SyntheticSection {
 public:
-  RelrBaseSection(unsigned concurrency);
+  RelrBaseSection(unsigned concurrency, bool isAArch64Auth = false);
   void mergeRels();
   bool isNeeded() const override {
     return !relocs.empty() ||
@@ -617,7 +628,7 @@ template <class ELFT> class RelrSection final : public RelrBaseSection {
   using Elf_Relr = typename ELFT::Relr;
 
 public:
-  RelrSection(unsigned concurrency);
+  RelrSection(unsigned concurrency, bool isAArch64Auth = false);
 
   bool updateAllocSize() override;
   size_t getSize() const override { return relrRelocs.size() * this->entsize; }
@@ -1319,6 +1330,7 @@ struct Partition {
   std::unique_ptr<PackageMetadataNote> packageMetadataNote;
   std::unique_ptr<RelocationBaseSection> relaDyn;
   std::unique_ptr<RelrBaseSection> relrDyn;
+  std::unique_ptr<RelrBaseSection> relrAuthDyn;
   std::unique_ptr<VersionDefinitionSection> verDef;
   std::unique_ptr<SyntheticSection> verNeed;
   std::unique_ptr<VersionTableSection> verSym;
@@ -1363,6 +1375,7 @@ struct InStruct {
   std::unique_ptr<StringTableSection> strTab;
   std::unique_ptr<SymbolTableBaseSection> symTab;
   std::unique_ptr<SymtabShndxSection> symTabShndx;
+  std::unique_ptr<AArch64PauthAbiTag> aarch64PauthAbiTag;
 
   void reset();
 };
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a84e4864ab0e5a5..f1b569daada663e 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -445,6 +445,12 @@ template <class ELFT> void elf::createSyntheticSections() {
       add(*part.relrDyn);
     }
 
+    if (config->relrPackAuthDynRelocs) {
+      part.relrAuthDyn = std::make_unique<RelrSection<ELFT>>(
+          threadCount, /*isAArch64Auth=*/true);
+      add(*part.relrAuthDyn);
+    }
+
     if (!config->relocatable) {
       if (config->ehFrameHdr) {
         part.ehFrameHdr = std::make_unique<EhFrameHeader>();
@@ -566,6 +572,11 @@ template <class ELFT> void elf::createSyntheticSections() {
   if (config->andFeatures)
     add(*make<GnuPropertySection>());
 
+  if (!ctx.aarch64PauthAbiTag.empty()) {
+    in.aarch64PauthAbiTag = std::make_unique<AArch64PauthAbiTag>();
+    add(*in.aarch64PauthAbiTag);
+  }
+
   // .note.GNU-stack is always added when we are creating a re-linkable
   // object file. Other linkers are using the presence of this marker
   // section to control the executable-ness of the stack area, but that
@@ -1725,6 +1736,8 @@ template <class ELFT> void Writer<ELFT>::finalizeAddressDependentContent() {
       changed |= part.relaDyn->updateAllocSize();
       if (part.relrDyn)
         changed |= part.relrDyn->updateAllocSize();
+      if (part.relrAuthDyn)
+        changed |= part.relrAuthDyn->updateAllocSize();
       if (part.memtagDescriptors)
         changed |= part.memtagDescriptors->updateAllocSize();
     }
@@ -2179,6 +2192,10 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
         part.relrDyn->mergeRels();
         finalizeSynthetic(part.relrDyn.get());
       }
+      if (part.relrAuthDyn) {
+        part.relrAuthDyn->mergeRels();
+        finalizeSynthetic(part.relrAuthDyn.get());
+      }
 
       finalizeSynthetic(part.dynSymTab.get());
       finalizeSynthetic(part.gnuHashTab.get());
diff --git a/lld/test/ELF/aarch64-feature-pauth.s b/lld/test/ELF/aarch64-feature-pauth.s
new file mode 100644
index 000000000000000..0520b2f28631e10
--- /dev/null
+++ b/lld/test/ELF/aarch64-feature-pauth.s
@@ -0,0 +1,83 @@
+# REQUIRES: aarch64
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag1.s -o tag11.o
+# RUN: cp tag11.o tag12.o
+# RUN: ld.lld -shared tag11.o tag12.o -o tagok.so
+# RUN: llvm-readelf -n tagok.so | FileCheck --check-prefix OK %s
+
+# OK: AArch64 PAuth ABI tag: platform 0x2a, version 0x1
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag2.s -o tag2.o
+# RUN: not ld.lld tag11.o tag12.o tag2.o -o /dev/null 2>&1 | FileCheck --check-prefix ERR1 %s
+
+# ERR1: error: incompatible values of AArch64 PAuth compatibility info found
+# ERR1: {{.*}}: 0x2A000000000000000{{1|2}}00000000000000
+# ERR1: {{.*}}: 0x2A000000000000000{{1|2}}00000000000000
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu abi-tag-errs.s -o errs.o
+# RUN: not ld.lld errs.o -o /dev/null 2>&1 | FileCheck --check-prefix ERR2 %s
+
+# ERR2:      error: {{.*}}: invalid type field value 42 (1 expected)
+# ERR2-NEXT: error: {{.*}}: invalid name field value XXX (ARM expected)
+# ERR2-NEXT: error: {{.*}}: too short AArch64 PAuth compatibility info (at least 16 bytes expected)
+
+# RUN: llvm-mc -filetype=obj -triple=aarch6...
[truncated]

@kovdan01
Copy link
Contributor Author

The buildkite is failing as expected since the patch depends on change from #72713 which is not included in this PR. Locally, the build and tests pass. I'll manually re-run buildkite when #72713 is merged.

@kovdan01
Copy link
Contributor Author

kovdan01 commented Dec 4, 2023

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

This patch adds lld 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

Co-authored-by: Peter Collingbourne <peter@pcc.me.uk>
@kovdan01
Copy link
Contributor Author

Rebased on current main branch and force-pushed to trigger buildkite

lld/ELF/Driver.cpp Outdated Show resolved Hide resolved
lld/ELF/Driver.cpp Outdated Show resolved Hide resolved
lld/ELF/Driver.cpp Outdated Show resolved Hide resolved
lld/ELF/Driver.cpp Outdated Show resolved Hide resolved
lld/ELF/InputFiles.cpp Outdated Show resolved Hide resolved
lld/ELF/Relocations.cpp Outdated Show resolved Hide resolved
lld/test/ELF/aarch64-ptrauth.s Outdated Show resolved Hide resolved
lld/test/ELF/aarch64-ptrauth.s Outdated Show resolved Hide resolved
@kovdan01
Copy link
Contributor Author

kovdan01 commented Jan 9, 2024

@kovdan01
Copy link
Contributor Author

@MaskRay Published updates on all issues you've mentioned - would be glad to see your comments on new changes.

@kovdan01
Copy link
Contributor Author

@MaskRay A kind reminder regarding the PR

@kovdan01
Copy link
Contributor Author

kovdan01 commented Apr 1, 2024

@MaskRay Just in case you need any help with testing (e.g. actually running using patched musl) pauth-enabled executables linked with patched lld - feel free to ask. For example, I can provide a downstream toolchain capable of building pauth-enabled elfs and a sysroot. Alternatively, I can create some small test object files with AUTH relocs (for example, via yaml2obj or smth similar) to be linked into a pauth-enabled executable with patched lld (w/o need of a whole pauth-enabled toolchain). This executable can be used to check if lld works properly.

@MaskRay
Copy link
Member

MaskRay commented Apr 2, 2024

Is the targeted user a downstream fork of musl and possibly Android Open Source Project?
It seems that they may be willing to implement DT_AARCH64_AUTH_RELR (ARM-software/abi-aa#252).

I wonder whether a potential libc implementation wants to adopt PAuthABI without implementing DT_AARCH64_AUTH_RELR. Relative relocations, unlike other relocations, need to be handled by rtld and static-linked executables as well. Introducing a new variant has some complexity.
If such a libc implementation is present (possibly upstream musl), we would need another linker option to keep RELR but drop DT_AARCH64_AUTH_RELR.

Perhaps make DT_AARCH64_AUTH_RELR separate from this patch so that we can evaluate it independently. The rest of part of this patch mostly looks good.

@kovdan01
Copy link
Contributor Author

kovdan01 commented Apr 2, 2024

@MaskRay I'll split the patch and move RELR-related stuff to a separate one so we don't get stuck on this. Regarding the need for RELR itself - as @smithp35 said (ARM-software/abi-aa#252 (comment)), RELR will be present in the spec unless everyone is happy with CREL. So, it does not look like that RELR is going to be deleted from the spec soon (also keeping in mind that RELR is already a well-known format while CREL is a new one) - and, since RELR is present in the spec, it's definitely worth having support for RELR in lld.

Regarding

we would need another linker option to keep RELR but drop DT_AARCH64_AUTH_RELR.

Do you mean that we need smth like -z pack-relative-relocs-wo-auth, which will enable regular RELR but not auth RELR (in contrast to -z pack-relative-relocs which enables both regular and auth RELR)?

@MaskRay
Copy link
Member

MaskRay commented Apr 2, 2024

@MaskRay I'll split the patch and move RELR-related stuff to a separate one so we don't get stuck on this. Regarding the need for RELR itself - as @smithp35 said (ARM-software/abi-aa#252 (comment)), RELR will be present in the spec unless everyone is happy with CREL. So, it does not look like that RELR is going to be deleted from the spec soon (also keeping in mind that RELR is already a well-known format while CREL is a new one) - and, since RELR is present in the spec, it's definitely worth having support for RELR in lld.

Regarding

we would need another linker option to keep RELR but drop DT_AARCH64_AUTH_RELR.

Do you mean that we need smth like -z pack-relative-relocs-wo-auth, which will enable regular RELR but not auth RELR (in contrast to -z pack-relative-relocs which enables both regular and auth RELR)?

We likely need an extra option, depending on how libc maintainers perceive complexity and benefits.
Some workload may have already deployed -z pack-relative-relocs. If .relr.auth.dyn is piggybacked on top of -z pack-relative-relocs, it might make deployment more difficult if bootstrap/static-pie part of .relr.auth.dyn is not implemented in rtld/libc.

Furthermore, the .relr.auth.dyn part of the PR logically can be separated.

@kovdan01
Copy link
Contributor Author

kovdan01 commented Apr 3, 2024

@MaskRay I've deleted RELR-related changes in 05cf361. The changes are kept in the branch https://github.com/kovdan01/llvm-project/tree/pauth-lld-relr. I'll open the PR with RELR changes as soon as this gets merged (hopefully soon). The reason why I'm not opening it now is that it's dependent on the current PR, and, given that both branches are in my fork, the PR would also be there. I can re-open current PR from a user branch as described here https://llvm.org/docs/GitHub.html#branches, but I'm not sure if it's a good idea since review history would become splitted. Please let me know which option do you prefer.

if (config->is64)
write32(buf + 28, 0); // Padding
unsigned offset = 16;

Copy link
Member

Choose a reason for hiding this comment

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

unneeded blank line

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, thanks, see 2e8ec83


config->andFeatures = -1;

StringRef referenceFileName;
Copy link
Member

@MaskRay MaskRay Apr 3, 2024

Choose a reason for hiding this comment

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

This loop tries to find a reference filename, but the loop can be removed.

Consider cherry picking the simplification at https://github.com/maskray/llvm-project/tree/lld-pauth ? The branch assumes that #87434 is brought back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This actually can't be removed - try to change files order in corresponding lld run lines in test/ELF/aarch64-feature-pauth.s from tag1.o noinfo1.o noinfo2.o to noinfo1.o tag1.o noinfo2.o, and you'll get an error. I'll submitted this update - see 99df511.

Explanation: if there are some files w/o PAuth core info which are processed before file with it, we'll never get a corresponding warning/error which we expect when enable -z pauth-report=warning|error. That's why it's crucial to find a file with PAuth core info first (if any), and then check others against it.

Please let me know if I miss something and there is a room for simplification even given the case above.

Copy link
Member

Choose a reason for hiding this comment

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

You are right, and thanks for the test improvement.

Add single quotes around referenceFileName and this patch LGTM!

@MaskRay
Copy link
Member

MaskRay commented Apr 3, 2024

LGTM. Sorry for the long delay.

I've made some suggestions in https://github.com/MaskRay/llvm-project/tree/lld-pauth , which assume that #87434 is brought back.

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

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

@MaskRay
Copy link
Member

MaskRay commented Apr 3, 2024

@MaskRay I've deleted RELR-related changes in 05cf361. The changes are kept in the branch kovdan01/llvm-project@pauth-lld-relr. I'll open the PR with RELR changes as soon as this gets merged (hopefully soon). The reason why I'm not opening it now is that it's dependent on the current PR, and, given that both branches are in my fork, the PR would also be there. I can re-open current PR from a user branch as described here llvm.org/docs/GitHub.html#branches, but I'm not sure if it's a good idea since review history would become splitted. Please let me know which option do you prefer.

Perhaps

@kovdan01
Copy link
Contributor Author

kovdan01 commented Apr 3, 2024

@MaskRay Thanks for your feedback and suggestions. I'll stick with your plan in terms of merging opened PRs and submitting a new one for RELR changes.

I've pushed several changes to this based on your latest feedback. Overall comment:

  • As far as I can see, your suggestion for readSecutiryNotes simplification is not quite correct. See my comment in related thread above.
  • Added mention of R_AARCH64_AUTH_RELATIVE in release notes (see 0eb272a).
  • Added -z pauth-report description to man page (see 964e9fe)
  • Applied your suggestions from f459527, 2846e76 and 45fcbb9.

@kovdan01 kovdan01 requested a review from MaskRay April 3, 2024 08:06
toString(f) +
": -z pauth-report: file does not have AArch64 "
"PAuth core info while " +
referenceFileName + " has one");
Copy link
Member

Choose a reason for hiding this comment

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

single quotes around referenceFileName

(We place single quotes around an identifier when it is placed beside a non-identifier word.)


config->andFeatures = -1;

StringRef referenceFileName;
Copy link
Member

Choose a reason for hiding this comment

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

You are right, and thanks for the test improvement.

Add single quotes around referenceFileName and this patch LGTM!

@kovdan01 kovdan01 merged commit cca9115 into llvm:main Apr 4, 2024
5 checks passed
@kovdan01
Copy link
Contributor Author

kovdan01 commented Apr 4, 2024

Submitted #87635 adding .relr.auth.dyn support as requested in https://github.com/llvm/llvm-project/pull/72714/#issuecomment-2033613105

kovdan01 added a commit to kovdan01/llvm-project that referenced this pull request Apr 5, 2024
Use symbol `.data` (STT_SECTION) instead of `__ehdr_start` for
some relocation entries. See discussion in
llvm#72714
kovdan01 added a commit that referenced this pull request Apr 5, 2024
Use symbol `.data` (STT_SECTION) instead of `__ehdr_start` for some
relocation entries. See discussion in

[72714/#discussion_r1517127619](#72714)
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.

None yet

6 participants