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

Wed Jan 29 08:26:01 PST 2020

bjope added inline comments.

================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:14
+
+#include <sys/auxv.h>
+#include <sys/prctl.h>
----------------
bjope wrote:
> pcc wrote:
> > bjope wrote:
> > > pcc wrote:
> > > > 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.
> > > I also had to move the auxv include a few lines down (inside the {{#if defined(ANDROID_EXPERIMENTAL_MTE)}} guard) for things to build on my RedHat 6.10 server.
> > > Would it make sense to do that change upstream?
> > I don't think so, we will eventually need to include it unconditionally on Linux once ANDROID_EXPERIMENTAL_MTE goes away as mentioned in the commit message. I think it would be better to change the build system so that we don't build/test scudo on Linux machines without a sys/auxv.h.
> Oh yes, that would be better of course. Although, I'm not that familiar with adding such checks, so I'm not sure exactly how to do it properly.
> I see sys/auxv.h being used in other places in compiler-rt as well, but maybe I don't hit those due to some existing build system checks for those parts (and maybe those checks arent't directed at checking for the presence of sys/auxv.h, but rather something else).
Proposed fix, checking that sys/auxv.h can be found: https://reviews.llvm.org/D73631

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762