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

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:293
 
+    Ptr = maybeUntagPointer(Ptr);
+
----------------
eugenis wrote:
> Do we want to touch memory with the tagged pointer first to catch double-free & invalid-free bugs?
Should be handled below in the chunk header check, no?


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:495
+  void disableMemoryTagging() {
+    if (useMemoryTagging()) disableMemoryTagChecks();
+    Primary.disableMemoryTagging();
----------------
nit: newline after `if`?


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:26
+
+#if defined(ANDROID_EXPERIMENTAL_MTE)
+
----------------
Can we move this ifdef inside of `systemSupportsMemoryTagging`?


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:50
+  void *TaggedPtr, *Cur, *End;
+  __asm__ __volatile__(
+      R"(
----------------
These asm stubs seem mostly abstractable - which would allow us to extend to future platforms easier, and make the intermediate [read - non-mte instructions] code easier to maintain.

Looks like we could abstract away to `storeZeroTag` abd `randomTagMemory` (or similar).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762