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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked 3 inline comments as done.
pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/combined.h:199
 
+  void *untagPointerMaybe(void *Ptr) {
+    if (Primary.SupportsMemoryTagging)
----------------
cryptoad wrote:
> I am assuming the compiler will inline this most of the time, but could we put that `inline` or `FORCE_INLINE` to double down?
`inline` is implicit on inline definitions (and somewhat confusingly in C++ `inline` doesn't really control inlining decisions, it's more of a linkage specification), so adding it here would have no effect. I'm personally not a fan of micro-optimizing inlining decisions like this, but I suppose it wouldn't hurt to add an `ALWAYS_INLINE` here.


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:14
+
+#include <sys/auxv.h>
+#include <sys/prctl.h>
----------------
cryptoad wrote:
> If I followed properly `memtag.h` is included on all platforms, but I am not sure `sys/*.h` is available everywhere (Fuchsia doesn't have one).
> So this probably requires some `#if SCUDO_LINUX` or something to that extent?
Yes, this will need `SCUDO_LINUX`, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762