[PATCH] D70762: scudo: Add initial memory tagging support.
Evgenii Stepanov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 10 13:06:03 PST 2019
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
More information about the llvm-commits
mailing list