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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:475
 def err_stack_tagging_requires_hardware_feature : Error<
-  "'-fsanitize=memtag' requires hardware support (+memtag)">;
+  "'-fsanitize=memtag-stack' requires hardware support (+memtag). For Armv8, "
+  "try compiling with -march=armv8a+memtag.">;
----------------
Split out renaming of memtag to memtag-stack first? This will remove a lot of diff from this patch.


================
Comment at: clang/include/clang/Driver/Options.td:1619
+                                        Group<f_clang_Group>,
+                                        HelpText<"Set default MTE mode to 'async' (default) or 'sync'.">;
 def fsanitize_hwaddress_experimental_aliasing
----------------
Let's make "sync" the default mode.


================
Comment at: lld/ELF/Config.h:346
+  // Mode of MTE to write to the ELF note. Should be one of NT_MEMTAG_ASYNC (for
+  // async), NT_MEMTAG_SYNC (for sync), or NT_MEMTAG_DEFAULT (for none). If
+  // async or sync is enabled, write the ELF note specifying the default MTE
----------------
NT_MEMTAG_DEFAULT -> NT_MEMTAG_LEVEL_NONE


================
Comment at: lld/ELF/Driver.cpp:691
+      error("When using --memtag-stack or --memtag-heap, a --memtag-mode value "
+            "is required.");
+  } else {
----------------
Should it be required? We could assume default Sync here the same as in clang frontend, that's one less flag to pass around or list in manual linker invocations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118948