[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/memtag.h:35
+inline void disableMemoryTagChecks() {
+  __asm__ __volatile__(".arch_extension mte; msr tco, #1");
+}
----------------
pcc wrote:
> eugenis wrote:
> > I don't think it is the job of a memory allocator to mess with PSTATE.TCO.
> > Let the caller deal with it?
> > 
> I can see the argument either way.
> 
> On one hand, TCO is a thread-wide property which the caller could arguably be considered responsible for.
> 
> On the other, disabling tag checks is required in order to avoid tag check failures for future allocations which reuse previously tagged chunks, so to a certain extent it makes sense to perform an operation that is required in order to keep the allocator working, even if it involves setting what is technically a thread-wide property. The operation that we perform depends on the allocator's implementation, so arguably it belongs with the allocator. If for example we switched to mprotecting without PROT_MTE when tag checks are disabled, there would be no need to set TCO here.
> 
> If you still think that it belongs in the caller, maybe we could document that setting TCO is required, but this may change in the future.
OK, SGTM, let's keep it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762