-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[BOLT] Add GNUPropertyRewriter and warn on AArch64 BTI note #161206
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
Conversation
This commit adds the GNUPropertyRewriter, which parses features from the .gnu.property.note section. Currently we only read the bit indicating BTI support (GNU_PROPERTY_AARCH64_FEATURE_1_BTI). As BOLT does not add BTI landing pads to targets of indirect branches/calls, we have to emit a warning that the output binary may be corrupted.
@llvm/pr-subscribers-bolt Author: Gergely Bálint (bgergely0) ChangesThis commit adds the GNUPropertyRewriter, which parses features from the .gnu.property.note section. Currently we only read the bit indicating BTI support (GNU_PROPERTY_AARCH64_FEATURE_1_BTI). As BOLT does not add BTI landing pads to targets of indirect branches/calls, we have to emit a warning that the output binary may be corrupted. Full diff: https://github.com/llvm/llvm-project/pull/161206.diff 9 Files Affected:
diff --git a/bolt/include/bolt/Core/BinaryContext.h b/bolt/include/bolt/Core/BinaryContext.h
index 082f1cec34d52..8960b1984745f 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -190,6 +190,9 @@ class BinaryContext {
/// Unique build ID if available for the binary.
std::optional<std::string> FileBuildID;
+ /// GNU property note indicating AArch64 BTI.
+ bool UsesBTI{false};
+
/// Set of all sections.
struct CompareSections {
bool operator()(const BinarySection *A, const BinarySection *B) const {
@@ -384,6 +387,9 @@ class BinaryContext {
}
void setFileBuildID(StringRef ID) { FileBuildID = std::string(ID); }
+ bool usesBTI() const { return UsesBTI; }
+ void setUsesBTI(bool Value) { UsesBTI = Value; }
+
bool hasSymbolsWithFileName() const { return HasSymbolsWithFileName; }
void setHasSymbolsWithFileName(bool Value) { HasSymbolsWithFileName = Value; }
diff --git a/bolt/include/bolt/Rewrite/MetadataRewriters.h b/bolt/include/bolt/Rewrite/MetadataRewriters.h
index b71bd6cad2505..2c09c879b9128 100644
--- a/bolt/include/bolt/Rewrite/MetadataRewriters.h
+++ b/bolt/include/bolt/Rewrite/MetadataRewriters.h
@@ -27,6 +27,8 @@ std::unique_ptr<MetadataRewriter> createPseudoProbeRewriter(BinaryContext &);
std::unique_ptr<MetadataRewriter> createSDTRewriter(BinaryContext &);
+std::unique_ptr<MetadataRewriter> createGNUPropertyRewriter(BinaryContext &);
+
} // namespace bolt
} // namespace llvm
diff --git a/bolt/lib/Rewrite/CMakeLists.txt b/bolt/lib/Rewrite/CMakeLists.txt
index 775036063dd5a..5b15edcacb482 100644
--- a/bolt/lib/Rewrite/CMakeLists.txt
+++ b/bolt/lib/Rewrite/CMakeLists.txt
@@ -25,6 +25,7 @@ add_llvm_library(LLVMBOLTRewrite
PseudoProbeRewriter.cpp
RewriteInstance.cpp
SDTRewriter.cpp
+ GNUPropertyRewriter.cpp
NO_EXPORT
DISABLE_LLVM_LINK_LLVM_DYLIB
diff --git a/bolt/lib/Rewrite/GNUPropertyRewriter.cpp b/bolt/lib/Rewrite/GNUPropertyRewriter.cpp
new file mode 100644
index 0000000000000..ecef9c61e3e0d
--- /dev/null
+++ b/bolt/lib/Rewrite/GNUPropertyRewriter.cpp
@@ -0,0 +1,147 @@
+//===- bolt/Rewrite/GNUPropertyRewriter.cpp -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Read the .gnu.property.note section.
+//
+//===----------------------------------------------------------------------===//
+
+#include "bolt/Rewrite/MetadataRewriter.h"
+#include "bolt/Rewrite/MetadataRewriters.h"
+#include "llvm/Support/Errc.h"
+
+using namespace llvm;
+using namespace bolt;
+
+namespace {
+
+class GNUPropertyRewriter final : public MetadataRewriter {
+
+ Expected<uint32_t> decodeGNUPropertyNote(StringRef Desc);
+
+public:
+ GNUPropertyRewriter(StringRef Name, BinaryContext &BC)
+ : MetadataRewriter(Name, BC) {}
+
+ Error sectionInitializer() override;
+};
+
+Error GNUPropertyRewriter::sectionInitializer() {
+
+ ErrorOr<BinarySection &> Sec =
+ BC.getUniqueSectionByName(".note.gnu.property");
+ if (!Sec)
+ return Error::success();
+
+ // Accumulate feature bits
+ uint32_t FeaturesAcc = 0;
+
+ StringRef Buf = Sec->getContents();
+ DataExtractor DE(Buf, BC.AsmInfo->isLittleEndian(),
+ BC.AsmInfo->getCodePointerSize());
+ DataExtractor::Cursor Cursor(0);
+ while (Cursor && !DE.eof(Cursor)) {
+ const uint32_t NameSz = DE.getU32(Cursor);
+ const uint32_t DescSz = DE.getU32(Cursor);
+ const uint32_t Type = DE.getU32(Cursor);
+
+ StringRef Name =
+ NameSz ? Buf.slice(Cursor.tell(), Cursor.tell() + NameSz) : "<empty>";
+ Cursor.seek(alignTo(Cursor.tell() + NameSz, 4));
+
+ const uint64_t DescOffset = Cursor.tell();
+ StringRef Desc =
+ DescSz ? Buf.slice(DescOffset, DescOffset + DescSz) : "<empty>";
+ Cursor.seek(alignTo(DescOffset + DescSz, 4));
+ if (!Cursor)
+ return createStringError(
+ errc::executable_format_error,
+ "out of bounds while reading .gnu.property.note section: %s",
+ toString(Cursor.takeError()).c_str());
+
+ if (Type == ELF::NT_GNU_PROPERTY_TYPE_0 && Name.starts_with("GNU") &&
+ DescSz) {
+ auto Features = decodeGNUPropertyNote(Desc);
+ if (!Features)
+ return Features.takeError();
+ FeaturesAcc |= *Features;
+ }
+ }
+
+ if (BC.isAArch64()) {
+ BC.setUsesBTI(FeaturesAcc & llvm::ELF::GNU_PROPERTY_AARCH64_FEATURE_1_BTI);
+ if (BC.usesBTI())
+ BC.outs() << "BOLT-WARNING: binary is using BTI. Optimized binary may be "
+ "corrupted\n";
+ }
+
+ return Error::success();
+}
+
+/// Desc contains an array of property descriptors. Each member has the
+/// following structure:
+/// typedef struct {
+/// Elf_Word pr_type;
+/// Elf_Word pr_datasz;
+/// unsigned char pr_data[PR_DATASZ];
+/// unsigned char pr_padding[PR_PADDING];
+/// } Elf_Prop;
+///
+/// As there is no guarantee that the features are encoded in which element of
+/// the array, we have to read all, and OR together the result.
+Expected<uint32_t> GNUPropertyRewriter::decodeGNUPropertyNote(StringRef Desc) {
+ DataExtractor DE(Desc, BC.AsmInfo->isLittleEndian(),
+ BC.AsmInfo->getCodePointerSize());
+ DataExtractor::Cursor Cursor(0);
+ const uint32_t Align = DE.getAddressSize();
+
+ std::optional<uint32_t> Features = 0;
+ while (Cursor && !DE.eof(Cursor)) {
+ const uint32_t PrType = DE.getU32(Cursor);
+ const uint32_t PrDataSz = DE.getU32(Cursor);
+
+ const uint64_t PrDataStart = Cursor.tell();
+ const uint64_t PrDataEnd = PrDataStart + PrDataSz;
+ Cursor.seek(PrDataEnd);
+ if (!Cursor)
+ return createStringError(
+ errc::executable_format_error,
+ "out of bounds while reading .gnu.property.note section: %s",
+ toString(Cursor.takeError()).c_str());
+
+ if (PrType == llvm::ELF::GNU_PROPERTY_AARCH64_FEATURE_1_AND) {
+ if (PrDataSz != 4) {
+ return createStringError(
+ errc::executable_format_error,
+ "Property descriptor size has to be 4 bytes on AArch64\n");
+ }
+ DataExtractor::Cursor Tmp(PrDataStart);
+ // PrDataSz = 4 -> PrData is uint32_t
+ const uint32_t FeaturesItem = DE.getU32(Tmp);
+ if (!Tmp)
+ return createStringError(
+ errc::executable_format_error,
+ "out of bounds while reading .gnu.property.note section: %s",
+ toString(Tmp.takeError()).c_str());
+ Features = Features ? (*Features | FeaturesItem) : FeaturesItem;
+ }
+
+ Cursor.seek(alignTo(PrDataEnd, Align));
+ if (!Cursor)
+ return createStringError(
+ errc::executable_format_error,
+ "out of bounds while reading .gnu.property.note section: %s",
+ toString(Cursor.takeError()).c_str());
+ }
+ return Features.value_or(0u);
+}
+} // namespace
+
+std::unique_ptr<MetadataRewriter>
+llvm::bolt::createGNUPropertyRewriter(BinaryContext &BC) {
+ return std::make_unique<GNUPropertyRewriter>("gnu-property-rewriter", BC);
+}
diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 8b78c53aa99b3..908dfd4fe626a 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -3341,6 +3341,8 @@ void RewriteInstance::initializeMetadataManager() {
MetadataManager.registerRewriter(createPseudoProbeRewriter(*BC));
MetadataManager.registerRewriter(createSDTRewriter(*BC));
+
+ MetadataManager.registerRewriter(createGNUPropertyRewriter(*BC));
}
void RewriteInstance::processSectionMetadata() {
diff --git a/bolt/test/AArch64/Inputs/property-note-bti.yaml b/bolt/test/AArch64/Inputs/property-note-bti.yaml
new file mode 100644
index 0000000000000..541ae92c92f6f
--- /dev/null
+++ b/bolt/test/AArch64/Inputs/property-note-bti.yaml
@@ -0,0 +1,50 @@
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+ Entry: 0x400510
+ProgramHeaders:
+ - Type: PT_NOTE
+ Flags: [ PF_R ]
+ FirstSec: .note.gnu.property
+ LastSec: .note.gnu.property
+ VAddr: 0x400338
+ Align: 0x8
+ - Type: PT_LOAD
+ Flags: [ PF_R ]
+ VAddr: 0x0
+ Align: 0x10000
+ FileSize: 0xf8
+ MemSize: 0xf8
+ Offset: 0x0
+Sections:
+ - Name: .text
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+ Address: 0x2a0000
+ AddressAlign: 0x4
+ Content: 400580d2c0035fd6
+ - Name: .note.gnu.property
+ Type: SHT_NOTE
+ Flags: [ SHF_ALLOC ]
+ Address: 0x400338
+ AddressAlign: 0x8
+ Notes:
+ - Name: GNU
+ Desc: 000000C0040000000300000000000000
+ Type: NT_GNU_PROPERTY_TYPE_0
+ - Type: SectionHeaderTable
+ Sections:
+ - Name: .note.gnu.property
+ - Name: .symtab
+ - Name: .strtab
+ - Name: .shstrtab
+ - Name: .text
+Symbols:
+ - Name: .note.gnu.property
+ Type: STT_SECTION
+ Section: .note.gnu.property
+ Value: 0x400338
+...
diff --git a/bolt/test/AArch64/Inputs/property-note-nobti.yaml b/bolt/test/AArch64/Inputs/property-note-nobti.yaml
new file mode 100644
index 0000000000000..a041a586d7804
--- /dev/null
+++ b/bolt/test/AArch64/Inputs/property-note-nobti.yaml
@@ -0,0 +1,50 @@
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+ Entry: 0x400510
+ProgramHeaders:
+ - Type: PT_NOTE
+ Flags: [ PF_R ]
+ FirstSec: .note.gnu.property
+ LastSec: .note.gnu.property
+ VAddr: 0x400338
+ Align: 0x8
+ - Type: PT_LOAD
+ Flags: [ PF_R ]
+ VAddr: 0x0
+ Align: 0x10000
+ FileSize: 0xf8
+ MemSize: 0xf8
+ Offset: 0x0
+Sections:
+ - Name: .text
+ Type: SHT_PROGBITS
+ Flags: [ SHF_ALLOC, SHF_EXECINSTR ]
+ Address: 0x2a0000
+ AddressAlign: 0x4
+ Content: 400580d2c0035fd6
+ - Name: .note.gnu.property
+ Type: SHT_NOTE
+ Flags: [ SHF_ALLOC ]
+ Address: 0x400338
+ AddressAlign: 0x8
+ Notes:
+ - Name: GNU
+ Desc: 000000C0040000000200000000000000
+ Type: NT_GNU_PROPERTY_TYPE_0
+ - Type: SectionHeaderTable
+ Sections:
+ - Name: .note.gnu.property
+ - Name: .symtab
+ - Name: .strtab
+ - Name: .shstrtab
+ - Name: .text
+Symbols:
+ - Name: .note.gnu.property
+ Type: STT_SECTION
+ Section: .note.gnu.property
+ Value: 0x400338
+...
diff --git a/bolt/test/AArch64/bti-note.test b/bolt/test/AArch64/bti-note.test
new file mode 100644
index 0000000000000..825456268a247
--- /dev/null
+++ b/bolt/test/AArch64/bti-note.test
@@ -0,0 +1,10 @@
+// This test checks that the GNUPropertyRewriter can decode the BTI feature flag.
+// It decodes an executable with BTI, and checks for the warning.
+
+RUN: yaml2obj %p/Inputs/property-note-bti.yaml &> %t.exe
+
+RUN: llvm-readelf -n %t.exe | FileCheck %s
+CHECK: BTI
+
+RUN: llvm-bolt %t.exe -o %t.exe.bolt | FileCheck %s -check-prefix=CHECK-BOLT
+CHECK-BOLT: BOLT-WARNING: binary is using BTI.
diff --git a/bolt/test/AArch64/no-bti-note.test b/bolt/test/AArch64/no-bti-note.test
new file mode 100644
index 0000000000000..92ed747ef7ac1
--- /dev/null
+++ b/bolt/test/AArch64/no-bti-note.test
@@ -0,0 +1,10 @@
+// This test checks that the GNUPropertyRewriter can decode the BTI feature flag.
+// It decodes an executable without BTI, and checks for the warning.
+
+RUN: yaml2obj %p/Inputs/property-note-nobti.yaml &> %t.exe
+
+RUN: llvm-readelf -n %t.exe | FileCheck %s
+CHECK-NOT: BTI
+
+RUN: llvm-bolt %t.exe -o %t.exe.bolt | FileCheck %s -check-prefix=CHECK-BOLT
+CHECK-BOLT-NOT: BOLT-WARNING: binary is using BTI.
|
Note to reviewers: the newly added GNUPropertyRewriter and the BuildIDRewriter both decode the contents of a note section, so a part of the code is common. I can add a follow-up NFC(I) patch later, to merge the two into a new NoteSectionRewriter, that can decode both. Should I do that? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Gergely,
Thanks for adding support for reading features from .note.gnu.property
.
I added a few nits.
- full warning message in lit tests - \p in comment - more specific error messages
Thanks for the review and approval @paschalis-mpeis! I addressed the nits. I will wait a few days before merging to give some time for objections. |
.gnu.property.note -> .note.gnu.property
) This commit adds the GNUPropertyRewriter, which parses features from the .note.gnu.property section. Currently we only read the bit indicating BTI support (GNU_PROPERTY_AARCH64_FEATURE_1_BTI). As BOLT does not add BTI landing pads to targets of indirect branches/calls, we have to emit a warning that the output binary may be corrupted.
This commit adds the GNUPropertyRewriter, which parses features from the .note.gnu.property section.
Currently we only read the bit indicating BTI support (GNU_PROPERTY_AARCH64_FEATURE_1_BTI).
As BOLT does not add BTI landing pads to targets of indirect branches/calls, we have to emit a warning that the output binary may be corrupted.