[PATCH] D70762: scudo: Add initial memory tagging support.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:260
+        //     data started on a page boundary).
+        // (2) Header was not reclaimed, data was partially reclaimed.
+        //
----------------
Is it possible that header was reclaimed, but data was only partially reclaimed? I think the code is correct in this case, too, but consider updating the comment.


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:35
+inline void disableMemoryTagChecks() {
+  __asm__ __volatile__(".arch_extension mte; msr tco, #1");
+}
----------------
I don't think it is the job of a memory allocator to mess with PSTATE.TCO.
Let the caller deal with it?



================
Comment at: compiler-rt/lib/scudo/standalone/wrappers_c.inc:180
+
+INTERFACE WEAK void SCUDO_PREFIX(malloc_disable_memory_tagging)() {
+  SCUDO_ALLOCATOR.disableMemoryTagging();
----------------
This api is not thread safe. TCO is per-thread, I think, so this would not work in a multi-threaded program at all. Either document this fact, or, preferably, remove the TCO code and use relaxed atomic for UseMemoryTagging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762