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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: clang/include/clang/Driver/SanitizerArgs.h:67
 
+  std::string MemtagMode = "";
+
----------------



================
Comment at: clang/lib/Driver/SanitizerArgs.cpp:658
+        Args.getLastArgValue(options::OPT_fsanitize_memtag_mode_EQ, "sync");
+    if (S.equals("async") || S.equals("sync")) {
+      MemtagMode = S.str();
----------------
Prefer the more common `==` to `equals`


================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:996
+    if (SanArgs.hasMemtagHeap())
+      CmdArgs.push_back("-memtag-heap");
+    if (SanArgs.hasMemtagStack())
----------------



================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:998
+    if (SanArgs.hasMemtagStack())
+      CmdArgs.push_back("-memtag-stack");
+  }
----------------



================
Comment at: clang/test/Driver/memtag-ld.c:1
+// REQUIRES: aarch64-registered-target
+
----------------
Pure driver tests (`-###`) generally don't need `REQUIRES: aarch64-registered-target`. You can test that the test should still pass by removing AArch64 from LLVM_TARGETS_TO_BUILD


================
Comment at: clang/test/Lexer/has_feature_memtag_heap.cpp:1
+// RUN: %clang_cc1 -E -fsanitize=memtag-heap %s -o - | FileCheck --check-prefix=CHECK-MEMTAG %s
+// RUN: %clang     -E -fsanitize=memtag --target=aarch64-unknown-linux -march=armv8a+memtag %s -o - \
----------------
The file can be merged into `has_feature_memtag_stack.cpp`. Just rename one `MemTagSanitizerEnabled` to avoid duplicate.


================
Comment at: clang/test/Lexer/has_feature_memtag_stack.cpp:2
+// RUN: %clang_cc1 -E -fsanitize=memtag-stack %s -o - | FileCheck --check-prefix=CHECK-MEMTAG %s
+// RUN: %clang     -E -fsanitize=memtag --target=aarch64-unknown-linux -march=armv8a+memtag %s -o - \
+// RUN:     | FileCheck --check-prefix=CHECK-MEMTAG %s
----------------
`%clang` is generally only used in test/Driver tests. (There are violations but the patch should not add more).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948