[lld] [MTE] Drop MTE globals for fully static executables, not ban #68217
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-lld-elf @llvm/pr-subscribers-lld ChangesIntegrating MTE globals on Android revealed a lot of cases where MTE globals doesn't work for fully static executables. They need a It seems like a much better idea to simply have LLD not do the special Full diff: https://github.com/llvm/llvm-project/pull/68217.diff 5 Files Affected:
diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
index 6272276e94b2d35..46ca8c94a362d2a 100644
--- a/lld/ELF/Driver.cpp
+++ b/lld/ELF/Driver.cpp
@@ -3029,10 +3029,20 @@ void LinkerDriver::link(opt::InputArgList &args) {
// partition.
copySectionsIntoPartitions();
- if (config->emachine == EM_AARCH64 &&
- config->androidMemtagMode != ELF::NT_MEMTAG_LEVEL_NONE) {
+ if (canHaveMemtagGlobals()) {
llvm::TimeTraceScope timeScope("Process memory tagged symbols");
createTaggedSymbols(ctx.objectFiles);
+ } else if (config->emachine == EM_AARCH64) {
+ // For fully static executables, make sure we prune any potential
+ // SHT_AARCH64_MEMTAG_GLOBALS_STATIC sections.
+ for (InputFile* file : ctx.objectFiles) {
+ if (file->kind() != InputFile::ObjKind)
+ continue;
+ for (InputSectionBase *section : file->getSections()) {
+ if (section && section->type == SHT_AARCH64_MEMTAG_GLOBALS_STATIC)
+ section->markDead();
+ }
+ }
}
// Create synthesized sections such as .got and .plt. This is called before
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index d6adc4ff3d644db..9075a2a545b556b 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -251,6 +251,15 @@ void elf::addReservedSymbols() {
ElfSym::edata2 = add("_edata", -1);
}
+// Fully static executables can't have MTE globals, because we need:
+// - A dynamic loader to process relocations, and
+// - Dynamic entries.
+bool elf::canHaveMemtagGlobals() {
+ return config->emachine == EM_AARCH64 &&
+ config->androidMemtagMode != ELF::NT_MEMTAG_LEVEL_NONE &&
+ (config->relocatable || config->shared || needsInterpSection());
+}
+
static OutputSection *findSection(StringRef name, unsigned partition = 1) {
for (SectionCommand *cmd : script->sectionCommands)
if (auto *osd = dyn_cast<OutputDesc>(cmd))
@@ -345,11 +354,7 @@ template <class ELFT> void elf::createSyntheticSections() {
std::make_unique<SymbolTableSection<ELFT>>(*part.dynStrTab);
part.dynamic = std::make_unique<DynamicSection<ELFT>>();
- if (config->emachine == EM_AARCH64 &&
- config->androidMemtagMode != ELF::NT_MEMTAG_LEVEL_NONE) {
- if (!config->relocatable && !config->shared && !needsInterpSection())
- error("--android-memtag-mode is incompatible with fully-static "
- "executables (-static)");
+ if (canHaveMemtagGlobals()) {
part.memtagAndroidNote = std::make_unique<MemtagAndroidNote>();
add(*part.memtagAndroidNote);
part.memtagDescriptors = std::make_unique<MemtagDescriptors>();
diff --git a/lld/ELF/Writer.h b/lld/ELF/Writer.h
index c69de54f76e9b4b..eaf021aac42ef56 100644
--- a/lld/ELF/Writer.h
+++ b/lld/ELF/Writer.h
@@ -56,6 +56,8 @@ uint8_t getMipsFpAbiFlag(uint8_t oldFlag, uint8_t newFlag,
bool isMipsN32Abi(const InputFile *f);
bool isMicroMips();
bool isMipsR6();
+
+bool canHaveMemtagGlobals();
} // namespace lld::elf
#endif
diff --git a/lld/test/ELF/Inputs/aarch64-memtag-globals.s b/lld/test/ELF/Inputs/aarch64-memtag-globals.s
index c48083f5550f871..cc7ca6e3d13dc51 100644
--- a/lld/test/ELF/Inputs/aarch64-memtag-globals.s
+++ b/lld/test/ELF/Inputs/aarch64-memtag-globals.s
@@ -380,3 +380,38 @@ global_extern_const_definition_but_nonconst_import:
global_extern_untagged_definition_but_tagged_import:
.word 0
.size global_extern_untagged_definition_but_tagged_import, 4
+
+#--- input_3.s
+## Generated with:
+##
+## - clang <input_file.c> -fsanitize=memtag-globals -O2 -S -o - \
+## --target=aarch64-linux-android31 -fno-asynchronous-unwind-tables
+##
+## <input_file.c> contents:
+##
+## int global_extern_outside_this_dso;
+##
+## int main() {
+## return 0;
+## }
+
+ .text
+ .file "main.c"
+ .globl main // -- Begin function main
+ .p2align 2
+ .type main,@function
+main: // @main
+// %bb.0: // %entry
+ mov w0, wzr
+ ret
+.Lfunc_end0:
+ .size main, .Lfunc_end0-main
+ // -- End function
+ .memtag global_extern_outside_this_dso // @global_extern_outside_this_dso
+ .type global_extern_outside_this_dso,@object
+ .bss
+ .globl global_extern_outside_this_dso
+ .p2align 4, 0x0
+global_extern_outside_this_dso:
+ .zero 16
+ .size global_extern_outside_this_dso, 16
diff --git a/lld/test/ELF/aarch64-memtag-globals.s b/lld/test/ELF/aarch64-memtag-globals.s
index fab6a032f0ce5c3..90eae0ad646dbf2 100644
--- a/lld/test/ELF/aarch64-memtag-globals.s
+++ b/lld/test/ELF/aarch64-memtag-globals.s
@@ -73,10 +73,33 @@ Symbols:
# RUN: %t1.o %t2.o -o %t1.so 2>&1 | FileCheck %s --check-prefix=CHECK-DYNRELOC
# CHECK-DYNRELOC: --apply-dynamic-relocs cannot be used with MTE globals
-## And ensure that fully-static executables are banned.
-# RUN: not ld.lld --static --android-memtag-mode=sync \
-# RUN: %t1.o %t2.o -o %t1.so 2>&1 | FileCheck %s --check-prefix=CHECK-NOSTATIC
-# CHECK-NOSTATIC: --android-memtag-mode is incompatible with fully-static executables (-static)
+## Ensure that fully statically linked executables just simply drop the MTE
+## globals stuff: special relocations, data in the place to be relocated,
+## dynamic entries, etc.
+# RUN: llvm-mc --filetype=obj -triple=aarch64-none-linux-android \
+# RUN: %t/input_3.s -o %t3.o
+# RUN: ld.lld -static -Bstatic --android-memtag-mode=sync %t1.o %t2.o %t3.o -o %t.static.so
+# RUN: llvm-readelf -s --section-headers --relocs --memtag %t.static.so | \
+# RUN: FileCheck %s --check-prefix=CHECK-STATIC
+# CHECK-STATIC-NOT: .memtag.globals.static
+# CHECK-STATIC-NOT: DT_AARCH64_MEMTAG_
+
+# CHECK-STATIC: There are no relocations in this file
+# CHECK-STATIC: Memtag Dynamic Entries:
+# CHECK-STATIC-NEXT: < none found >
+
+# RUN: llvm-objdump -tDz %t.static.so | FileCheck %s --check-prefix=CHECK-STATIC-SPECIAL-RELOCS
+# CHECK-STATIC-SPECIAL-RELOCS: [[#%x,HIDDEN_GLOBAL_ADDR:]] {{.*}} .bss {{0*}}10 hidden_global
+# CHECK-STATIC-SPECIAL-RELOCS: <pointer_to_hidden_global_end>:
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x{{0*}}[[#HIDDEN_GLOBAL_ADDR + 12]]
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
+# CHECK-STATIC-SPECIAL-RELOCS: <pointer_past_hidden_global_end>:
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x{{0*}}[[#HIDDEN_GLOBAL_ADDR + 16]]
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
+# CHECK-STATIC-SPECIAL-RELOCS-NEXT: .word 0x00000000
# CHECK: Symbol table '.dynsym' contains
# CHECK-DAG: [[#%x,GLOBAL:]] 32 OBJECT GLOBAL DEFAULT [[#]] global{{$}}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I forget whether LLD supports static PIE, but wouldn’t that allow MTE globals? And why can’t even static PDEs support them? We preserve and process relocations for IFUNCs today. |
Looking at the ifunc resolver glue in bionic (Android's loader/libc), it maybe possible in future support MTE globals as part of fully static executables. MTE globals requires two parts:
Either way, I'd consider MTE globals for static binaries as an extension for future work. For now, we just don't support it. |
smithp35
reviewed
Oct 6, 2023
smithp35
approved these changes
Oct 6, 2023
MaskRay
reviewed
Oct 10, 2023
MaskRay
approved these changes
Oct 10, 2023
Integrating MTE globals on Android revealed a lot of cases where libraries are built as both archives and DSOs, and they're linked into fully static and dynamic executables respectively. MTE globals doesn't work for fully static executables. They need a dynamic loader to process the special R_AARCH64_RELATIVE relocation semantics with the encoded offset. Fully static executables that had out-of-bounds derived symbols (like 'int* foo_end = foo[16]') crash under MTE globals w/ static executables. So, LLD in its current form simply errors out when you try and compile a fully static executable that has a single MTE global variable in it. It seems like a much better idea to simply have LLD not do the special work for MTE globals in fully static contexts, and to drop any unnecessary metadata. This means that you can build archives with MTE globals and link them into both fully-static and dynamic executables.
hctim
force-pushed
the
mte/globals-static
branch
from
ea5d562
to
adbdbd4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Integrating MTE globals on Android revealed a lot of cases where
libraries are built as both archives and DSOs, and they're linked into
fully static and dynamic executables respectively.
MTE globals doesn't work for fully static executables. They need a
dynamic loader to process the special R_AARCH64_RELATIVE relocation
semantics with the encoded offset. Fully static executables that had
out-of-bounds derived symbols (like 'int* foo_end = foo[16]') crash
under MTE globals w/ static executables. So, LLD in its current form
simply errors out when you try and compile a fully static executable
that has a single MTE global variable in it.
It seems like a much better idea to simply have LLD not do the special
work for MTE globals in fully static contexts, and to drop any
unnecessary metadata. This means that you can build archives with MTE
globals and link them into both fully-static and dynamic executables.