llvm-project
llvm-project copied to clipboard
llvm
[lld][AArch64][ELF][PAC] Support AUTH relocations and AUTH ELF marking
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
Depends on #72713 and #85231
@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:
-
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
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]
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.
- Addressed the review comments except https://github.com/llvm/llvm-project/pull/72714/#discussion_r1423345089 (I'll submit a subsequent commit addressing that shortly).
- Fixed merge conflict appeared after #77300, see a021f15540300e032446825de805143f0f6214c4.
@MaskRay Published updates on all issues you've mentioned - would be glad to see your comments on new changes.
@MaskRay Would be glad to see your comments on changes addressing your comments
Apologies for the delay. With the switch to GitHub it's kinda difficult to find review requests that need my attention...
Just my 2 cents: have you tried graphite.dev? Seems to have nice filtering for PRs that require attention
@MaskRay
Is there a dynamic loader implementation? Without it it's difficult to verify the correctness.
Yes, there is: see https://github.com/access-softek/musl/tree/dkovalev/pauth-code-drop. I've used that to verify the correctness, and it seems that there are no issues (at least no ones were found). Feel free to use this musl implementation and verify the lld patch correctness by yourself independently.
It's possibly worth waiting until I address the review comments and rebase the PR in order to use the latest version of code when testing lld output with musl.
lld/test/ELF/aarch64-reloc-pauth.s line 65 at r9 (raw file):
# RELR-NEXT: } # RUN: llvm-readobj -x .test %t2 | FileCheck --check-prefix=HEX %s
You can run -r -x .test in one invocation to save one llvm-readobj command.
I simplified the code a bit.
I think -no-pie must use a dynamic relocation as well. See https://github.com/MaskRay/llvm-project/tree/pauth-lld-nopie
@MaskRay Thanks for your feedback, see answers to your comments and new commits addressing the issues.
I want to bring attention to discussion_r1495128934 - I've discovered a case with large addends (fitting 32-bits, but large, like 0x7fffffff) which was not handled properly. See e9b53374522fca221414aac3d5a3b225e7fc73d4 for the fix. You can find all the details in the commit message, comments in code and tests.
There are also several old comments which are still unresolved but IMO fixed. I would be glad if you look through them and let me know if I've missed something and some issues are still there. For the fixed issues, I kindly ask you to resolve threads - it would be much easier to look through the PR if only unfixed comments are unresolved.
- discussion_r1423344812 - fixed in 589c6455a929f41ed3a79fc7d91119586eb1ee7b
- discussion_r1423345089 - fixed in 594f8a0e8331b5d11f3efc58fcaa7eae4b9fd7b4
- discussion_r1423345225 - it looks like we've achieved some consensus in test naming and current test names (aarch64-bti-pac-cli-error.s, aarch64-feature-pauth.s, aarch64-reloc-pauth-ro.s, aarch64-reloc-pauth.s) seem OK
- discussion_r1423345733 - test directives count looks reasonable now: not too little, not too many of them
- discussion_r1472312886 - fixed in de73eae53481bb805fbabe97a38055ebaad934de
- discussion_r1472313816 - fixed in de73eae53481bb805fbabe97a38055ebaad934de
- discussion_r1472314138 - fixed in de73eae53481bb805fbabe97a38055ebaad934de
- discussion_r1485223221 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163
- discussion_r1485223337 - fixed in ec1c6322341f3f25f3ad0070810c4a90d188c3b5
- discussion_r1485224296 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
- discussion_r1485224665 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163
- discussion_r1485225472 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163
- discussion_r1485226064 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163
- discussion_r1485231933 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
- discussion_r1485233154 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163 and not even relevant after 1448a4a4e0e3cb70358558579e97fce3f0990356
- discussion_r1485234537 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163 and 1448a4a4e0e3cb70358558579e97fce3f0990356
- discussion_r1485235734 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
- discussion_r1485237672 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
- discussion_r1485239935 - fixed in aff3a96d4383b6aa930d4d21db949b3b459f2163
- discussion_r1485240613 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
- discussion_r1485243679 - fixed in 4e53afaa50e63466263d0697e0e2a335d281dbf6
@MaskRay Would be glad to see your feedback on the changes
Apologies for the delay. I think the latest update is really close. I've made several comments. The primary ones are:
- processAux should be simplified
- how
finalizeAddressDependentContentshould do the fixup. &isec.relocs().back()will become dangling if the relocations variable gets resized.- an unneeded
template
@MaskRay Thanks for feedback! I've addressed the comments. Regarding
processAux should be simplified
Please see my comment in discussion_r1517190622. I might miss something, but I don't feel that processAux can be significantly simpler. Even if we move all checks that we can to finalizeAddressDependentContent, the check of part.relrAuthDyn against nullptr would still remain in processAux, and both else if and else branches would still be present. I agree that the check against 32-bit addend is redundant here though - it's anyway re-checked later.
:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Do we really need to support .note.AARCH64-PAUTH-ABI-tag NT_ARM_TYPE_PAUTH_ABI_TAG?
GNU program properties are well-integrated in glibc now.
If they choose to implement the pauth ABI, and toolchains support program properties, it's likely they'll leverage this established system, avoiding .note.AARCH64-PAUTH-ABI-tag.
Since there is no counterpart for GNU_PROPERTY_AARCH64_FEATURE_1_BTI and PAC in .note.AARCH64-PAUTH-ABI-tag, it seems even less motivation for .note.AARCH64-PAUTH-ABI-tag, since an extra note section would just increase code size.
FreeBSD, with its existing return address pauth support, also seems content with GNU program properties.
https://github.com/access-softek/musl/tree/dkovalev/pauth-code-drop may need an update
Do we really need to support .note.AARCH64-PAUTH-ABI-tag
NT_ARM_TYPE_PAUTH_ABI_TAG?
@MaskRay The spec has it defined, and I suppose there is a reason for this. We do not control the input object files we get, and a we might have to process ones with .note.AARCH64-PAUTH-ABI-tag - someone can possibly generate such files since its explicitly allowed by the spec. If we do not process this section properly, we'll just silently concatenate corresponding input sections and emit this in the output - this is definitely not what we want. As for me, support is not that complicated and does not add too much complexity to lld - so, is there a strong reason not to have it? We benefit a lot from supporting the spec fully w/o any gaps and pay for this with a bit of extra code.
P.S. I'm going to push a couple of small changes to this PR and leave comments regarding the latest feedback before re-requesting the review.
Do we really need to support .note.AARCH64-PAUTH-ABI-tag
NT_ARM_TYPE_PAUTH_ABI_TAG?@MaskRay The spec has it defined, and I suppose there is a reason for this. We do not control the input object files we get, and a we might have to process ones with .note.AARCH64-PAUTH-ABI-tag - someone can possibly generate such files since its explicitly allowed by the spec. If we do not process this section properly, we'll just silently concatenate corresponding input sections and emit this in the output - this is definitely not what we want. As for me, support is not that complicated and does not add too much complexity to lld - so, is there a strong reason not to have it? We benefit a lot from supporting the spec fully w/o any gaps and pay for this with a bit of extra code.
P.S. I'm going to push a couple of small changes to this PR and leave comments regarding the latest feedback before re-requesting the review.
I can give some background on the specification.
At the time the specification was first written (probably 2020) GNU properties were unpopular in Arm for relocatable objects after experiences with BTI. I used a PT_NOTE as a first pass to avoid GNU properties. In the intervening years Arm's thinking of ELF marking has coalesced towards GNU properties in ELF Executables/Shared-libraries (ELF loaders already have to deal with BTI) and Build Attributes for relocatable objects.
I think the only value in the alternative PT_NOTE is if there are legacy objects/executables that can't be recompiled using it. To the best of my knowledge I'm not aware of any of these as this is the only serious implementation of the ELF spec. All other implementations that I know of have been incomplete prototypes.
I would be very happy to take out the PT_NOTE from the specification if no-one is using it.
To the best of my knowledge I'm not aware of any of these as this is the only serious implementation of the ELF spec.
I'm not aware of such implementations as well.
I suppose we can do the following:
- Update the spec and remove PT_NOTE from it. If someone needs it for some reason, they can ask for it explicitly.
- Keep only GNU property support in lld.
@MaskRay @smithp35 Please let me know what do you think on this. I'll update the PR depending on your answers.
To the best of my knowledge I'm not aware of any of these as this is the only serious implementation of the ELF spec.
I'm not aware of such implementations as well.
I suppose we can do the following:
- Update the spec and remove PT_NOTE from it. If someone needs it for some reason, they can ask for it explicitly.
- Keep only GNU property support in lld.
@MaskRay @smithp35 Please let me know what do you think on this. I'll update the PR depending on your answers.
Thanks to @smithp35 for the background. I want to avoid .note.AARCH64-PAUTH-ABI-tag complexity.
Think: If the compiler emits both note sections, there will be small but extra overhead for each relocatable file. Some users may want an option to turn off .note.AARCH64-PAUTH-ABI-tag, then we end up with additional compiler driver complexity.
To the best of my knowledge I'm not aware of any of these as this is the only serious implementation of the ELF spec.
I'm not aware of such implementations as well.
I suppose we can do the following:
- Update the spec and remove PT_NOTE from it. If someone needs it for some reason, they can ask for it explicitly.
- Keep only GNU property support in lld.
@MaskRay @smithp35 Please let me know what do you think on this. I'll update the PR depending on your answers.
I've created https://github.com/ARM-software/abi-aa/pull/250 to remove the alternative format from the specification.
@MaskRay Thanks for your feedback, please see new version of the changes. Overall comment:
- The PR is now dependent on https://github.com/llvm/llvm-project/pull/85231, so buildkite job fails expectedly.
- Only keep support for GNU property, drop support for the alternative.
- Avoid mixing different terms in comments: applied your suggestions with minor typo fixes d32b8e32fc1c938e5a005997b832d215a308037d + inspected other comments and fixed them as well 4409838992a87a325843393be1ff30c67cdabc7b
- With new version of spec, the size of the core info becomes exactly 16 bytes (previously >=16 bytes allowed). Do we want to use a fixed-size container like
std::optional<std::array<uint8_t, 16>>(sizeof 17, alignof 1, optional for a case of no core info) instead ofArrayRef<uint8_t>(sizeof 16, alignof 8)? - Renamed "AArch64 PAuth ABI tag" to and "AArch64 PAuth ABI compatibility info" to "AArch64 PAuth ABI core info" (728c93355150ef5ad712aca3910def600d68ee86, 128831c02cb54ed7b0b9afa7c17b3f8d3ffb6f7c) as defined in new version of the spec.
- Do we want to use
errorOrWarninstead offatalinreadGnuPropertylike we did when implementing reading of the alternative section before? I've usedfatalsince existing code uses it, but maybe we want to change it (probably in a subsequent PR). Twine::utohexstrinstead oftoHexwith/*LowerCase=*/true- I use the latter since I want to print anArrayRefof 16 bytes, not a singleuint64_t. We can probably print it as two bytes, but I'm not sure if it would be nice and if it would not cause issues with different endianness. Do I miss something?
https://github.com/access-softek/musl/tree/dkovalev/pauth-code-drop may need an update
@MaskRay This musl implementation is just a proof-of-concept to ensure that AUTH relocs work properly and to have ability to actually execute pauth-enabled code. In terms of this PoC, reading (platform,version) values from GNU property and faulting on their mismatch in different DSOs is considered out of scope - it does not give that much value in terms of testing the toolchain itself. So, we only have crucial features implemented - particularly, handling AUTH relocs.
access-softek/musl@
dkovalev/pauth-code-drop may need an update@MaskRay This musl implementation is just a proof-of-concept to ensure that AUTH relocs work properly and to have ability to actually execute pauth-enabled code. In terms of this PoC, reading (platform,version) values from GNU property and faulting on their mismatch in different DSOs is considered out of scope - it does not give that much value in terms of testing the toolchain itself. So, we only have crucial features implemented - particularly, handling AUTH relocs.
Thanks.
The subtle .relr.auth.dyn support makes we wonder how much benefit we should expect from introducing a new relocation format.
I have filed https://github.com/ARM-software/abi-aa/issues/252 for quantifying the benefits of DT_AARCH64_AUTH_RELR.
:white_check_mark: With the latest revision this PR passed the Python code formatter.
@MaskRay Would be glad to see your feedback - previously mentioned issues are addressed in several latest commits. See also an overall comment and a couple of questions https://github.com/llvm/llvm-project/pull/72714#issuecomment-2007448859