[PATCH] D118948: [MTE] Add -fsanitize=memtag* and friends.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I haven't investigated the use case yet, just commented a few things. Please split the patch into 3:

- BinaryFormat/ELF.h (see an inline comment)
- lld/ELF
- clang



================
Comment at: lld/ELF/SyntheticSections.cpp:3842
 
+static constexpr char kMemtagAndroidNoteName[] = "Android";
+void MemtagAndroidNote::writeTo(uint8_t *buf) {
----------------



================
Comment at: lld/ELF/SyntheticSections.cpp:3846
+  llvm::ELF::Elf64_Nhdr header;
+  header.n_namesz = sizeof(kMemtagAndroidNoteName);
+  header.n_descsz = sizeof(uint32_t);
----------------
See `GnuPropertySection::writeTo`


================
Comment at: lld/ELF/SyntheticSections.h:1235
   std::unique_ptr<VersionTableSection> verSym;
+  std::unique_ptr<MemtagAndroidNote> memtagAndroidNote;
 
----------------
keep variable names alphabetically


================
Comment at: lld/test/ELF/memtag-android-abi.s:1
+// Old versions of Android (Android 11 & 12) have very strict parsing logic on
+// the layout of the ELF note. This test serves as a "hey, you're going to break
----------------
REQUIRES: aarch64

Use `#` for comments. Rename this to `aarch64-*`


================
Comment at: lld/test/ELF/memtag-android-abi.s:19
+// RUN: llvm-readelf -S %t | FileCheck %s
+// RUN: llvm-objdump -Dz -j .note.android.memtag %t | \
+// RUN:    FileCheck %s --check-prefixes=NOTE,SYNC-NOTE
----------------
use `llvm-readelf -x .note.android.memtag` to dump the bytes of a section.

If this is a note section, consider `llvm-readelf -n`


================
Comment at: llvm/include/llvm/BinaryFormat/ELF.h:1536
+enum {
+  NT_TYPE_IDENT = 1,
+  NT_TYPE_KUSER = 3,
----------------
If Android wants to define notes, can it use a namespace `NT_ANDROID_*` like GNU?

Please see `https://reviews.llvm.org/D107949` how I add ELF attributes and related tests when new additions are added to BinaryFormat/ELF.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118948/new/

https://reviews.llvm.org/D118948