[clang] c9f5b5c - [MTE] Disable all MTE protection of globals in sections (#78443)

[via cfe-commits]

Author: Mitch Phillips
Date: 2024-01-22T11:55:39+01:00
New Revision: c9f5b5c935bd12d76d4bafff61d8116cb3229972

URL: https://github.com/llvm/llvm-project/commit/c9f5b5c935bd12d76d4bafff61d8116cb3229972
DIFF: https://github.com/llvm/llvm-project/commit/c9f5b5c935bd12d76d4bafff61d8116cb3229972.diff

LOG: [MTE] Disable all MTE protection of globals in sections (#78443)

Previous work in this area (#70186) disabled MTE in constructor
sections. Looks like I missed one, ".preinit_array".

Also, in the meantime, I found an exciting feature in the linker where
globals placed into an explicit section, where the section name is a
valid C identifer, gets an implicit '__start_<sectionname>' and
'__stop_<sectionname>' symbol as well. This is convenient for iterating
over some globals, but of course iteration over differently-tagged
globals in MTE explodes.

Thus, disable MTE globals for anything that has a section.

Added: 
    

Modified: 
    clang/test/CodeGen/memtag-globals-asm.cpp
    llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/CodeGen/memtag-globals-asm.cpp b/clang/test/CodeGen/memtag-globals-asm.cpp
index 4b76b394e0c1dc..186045f8f2fb5b 100644
--- a/clang/test/CodeGen/memtag-globals-asm.cpp
+++ b/clang/test/CodeGen/memtag-globals-asm.cpp
@@ -271,6 +271,10 @@ CONSTRUCTOR(".ctors") func_t func_ctors = func_constructor;
 CONSTRUCTOR(".dtors") func_t func_dtors = func_constructor;
 CONSTRUCTOR(".init_array") func_t func_init_array = func_constructor;
 CONSTRUCTOR(".fini_array") func_t func_fini_array = func_constructor;
+CONSTRUCTOR(".preinit_array") func_t preinit_array = func_constructor;
+CONSTRUCTOR("array_of_globals") int global1;
+CONSTRUCTOR("array_of_globals") int global2;
+CONSTRUCTOR("array_of_globals") int global_string;
 
 // CHECK-NOT: .memtag func_constructor
 // CHECK-NOT: .memtag func_init
@@ -279,3 +283,7 @@ CONSTRUCTOR(".fini_array") func_t func_fini_array = func_constructor;
 // CHECK-NOT: .memtag func_dtors
 // CHECK-NOT: .memtag func_init_array
 // CHECK-NOT: .memtag func_fini_array
+// CHECK-NOT: .memtag preinit_array
+// CHECK-NOT: .memtag global1
+// CHECK-NOT: .memtag global2
+// CHECK-NOT: .memtag global_string

diff  --git a/llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp b/llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
index 8ce6f94e7341de..27959489e7dfa4 100644
--- a/llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
+++ b/llvm/lib/Target/AArch64/AArch64GlobalsTagging.cpp
@@ -43,13 +43,25 @@ static bool shouldTagGlobal(GlobalVariable &G) {
     return false;
   }
 
-  // Don't instrument function pointers that are going into various init arrays
-  // via `__attribute__((section(<foo>)))`:
-  // https://github.com/llvm/llvm-project/issues/69939
-  if (G.hasSection() &&
-      (G.getSection() == ".init" || G.getSection() == ".fini" ||
-       G.getSection() == ".init_array" || G.getSection() == ".fini_array" ||
-       G.getSection() == ".ctors" || G.getSection() == ".dtors")) {
+  // Globals can be placed implicitly or explicitly in sections. There's two
+  // 
diff erent types of globals that meet this criteria that cause problems:
+  //  1. Function pointers that are going into various init arrays (either
+  //     explicitly through `__attribute__((section(<foo>)))` or implicitly
+  //     through `__attribute__((constructor)))`, such as ".(pre)init(_array)",
+  //     ".fini(_array)", ".ctors", and ".dtors". These function pointers end up
+  //     overaligned and overpadded, making iterating over them problematic, and
+  //     each function pointer is individually tagged (so the iteration over
+  //     them causes SIGSEGV/MTE[AS]ERR).
+  //  2. Global variables put into an explicit section, where the section's name
+  //     is a valid C-style identifier. The linker emits a `__start_<name>` and
+  //     `__stop_<na,e>` symbol for the section, so that you can iterate over
+  //     globals within this section. Unfortunately, again, these globals would
+  //     be tagged and so iteration causes SIGSEGV/MTE[AS]ERR.
+  //
+  // To mitigate both these cases, and because specifying a section is rare
+  // outside of these two cases, disable MTE protection for globals in any
+  // section.
+  if (G.hasSection()) {
     Meta.Memtag = false;
     G.setSanitizerMetadata(Meta);
     return false;