[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/memtag.h:30
+  return (getauxval(AT_HWCAP2) & HWCAP2_MTE) &&
+         (prctl(PR_GET_TAGGED_ADDR_CTRL, 0, 0, 0, 0) & PR_MTE_TCF_MASK) !=
+             PR_MTE_TCF_NONE;
----------------
pcc wrote:
> kevin.brodsky wrote:
> > I wonder if this is really a good thing. If libc fails to enable tag checking before the allocator is initialised (which is quite possible, given that until recently `malloc()` was called very early in Bionic's libc_init), then Scudo will not tag anything. Wouldn't it be possible instead to explicitly ask Scudo to use tagging when it is initialised? This would also be more consistent with the `malloc_disable_memory_tagging()` interface: Scudo does not take care of enabling / disabling tag checking, so arguably it shouldn't check if it is enabled either.
> So the motivation behind adding the check here was along the lines of "if the application doesn't enable memory tag checks then there's no point in enabling memory tagging". But I can see the value in decoupling these two things especially since the libc might not have a chance to turn on memory tagging before the first allocation. I'll change this to be purely based on the hwcap and let the application call `malloc_disable_memory_tagging()` early if it doesn't want to use memory tagging.
> 
> I'd prefer not to add an explicit initialization step to the allocator since the allocation functions can in principle be called at any time, so we'd need additional complexity to handle the case where the allocation functions were called before the allocator was formally initialized.
I forgot about the other reason why I added this check, which was to avoid breaking the tests if the libc did not issue the prctl to enable memory tag checks. This seems like something that can be properly checked for in the tests themselves, which I've now done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70762