[PATCH] D63360: hwasan: Use bits [3..11) of the ring buffer entry address as the base stack tag.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: compiler-rt/test/hwasan/TestCases/random-align-right.c:30
 // CHECK1: accessing p[20]
+// CHECK1: accessing p[20]
 // CHECK1-NEXT: HWAddressSanitizer: tag-mismatch
----------------
eugenis wrote:
> pcc wrote:
> > eugenis wrote:
> > > why did you copy this line?
> > Because the additional call to GenerateRandomTag in InitRandomState causes the values that we assign to tail_magic in HwasanAllocatorInit to change in such a way that we catch the second bad access and not the first one. Without copying this line the `CHECK?-NEXT` lines fail to match because the failure doesn't happen on the line after the first message.
> Wait, so the comment above the loop lies? We are testing for the bug on the first iteration, not an any iteration. Maybe remove -NEXT?
> 
Yeah, looks like a mistake in the test. The intent here was apparently that we'd let any access fail, but the test actually only lets the first access fail (and because of determinism that is always what happens).

We can't just remove -NEXT because it would prevent the test from determining which access found the bug. So I've rewritten this test a bit so that we can do that.


================
Comment at: compiler-rt/test/hwasan/TestCases/stack-history-length.c:20
+  // Bump the tag number so that OOB() doesn't end up with tag 0.
+  FUNC();
+
----------------
eugenis wrote:
> pcc wrote:
> > eugenis wrote:
> > > I don't understand this. How does a single additional call to FUNC help guarantee that property?
> > In the case where there are 2046 calls to `FUNC` we have:
> > - tag 1 for `FUNC0`
> > - tag 2..2047 (mod 256) for `FUNC1`
> > - tag 2048 (mod 256) for `OOB` (i.e. 0)
> > Adding the call to `FUNC` shifts all of the tags by 1 so that `OOB` gets tag 1.
> Could we call OOB twice instead? And weaken the test a bit so that it is ok with result being off by 1 or 2 in any direction?
Works for me, done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63360