[PATCH] D92880: scudo: Split setRandomTag in two. NFCI.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:106
 
-  2:
-  )"
-      :
-      [TaggedPtr] "=&r"(*TaggedBegin), [Cur] "=&r"(*TaggedEnd), [End] "=&r"(End)
-      : [Ptr] "r"(Ptr), [Size] "r"(Size), [ExcludeMask] "r"(ExcludeMask)
-      : "memory");
+inline uptr storeTags(uptr Begin, uptr End) {
+  if (Begin != End) {
----------------
hctim wrote:
> Worth a `DCHECK` that `Begin` and `End` are granule-aligned (and in selectRandomTag)?
`End` doesn't need to be granule-aligned (we can pass an unaligned `End` from the primary allocator). This is the reason why we return the address of the end of the tagged region instead of just letting the caller use `End`. I suppose that we could check `Begin` here though.


================
Comment at: compiler-rt/lib/scudo/standalone/memtag.h:107
+inline uptr storeTags(uptr Begin, uptr End) {
+  if (Begin != End) {
+    __asm__ __volatile__(
----------------
hctim wrote:
> Probably better to be safe and make this `if (Begin < End)`?
I'm not sure about that. I think that if `Begin > End` we will want to crash here (i.e. the likely source of the bug) instead of at some indeterminate point later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92880