[PATCH] D86000: Add an unsigned shift base sanitizer

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: compiler-rt/test/ubsan/TestCases/Integer/unsigned-shift.cpp:6
+  volatile unsigned _v = (val);    \
+  volatile unsigned _a = (amount); \
+  unsigned res = _v << _a;         \
----------------
jfb wrote:
> vsk wrote:
> > Does the test not work without the volatiles?
> It seems that LLVM sees too much without volatile, yes.
The optimizer isn't running. Perhaps this is necessary because clang's constant folder kicks in?


================
Comment at: llvm/docs/ReleaseNotes.rst:151
+
+One can silence the check with masking, for example:
+
----------------
This could use some more explanation, maybe s/with masking/by clearing bits that would be shifted out/?


================
Comment at: llvm/docs/ReleaseNotes.rst:153
+
+  `(lhs & ~(~1U << ((sizeof(lhs)*CHAR_BIT) - rhs))) << rhs`
+
----------------
jfb wrote:
> lebedev.ri wrote:
> > Surely not `~1U`.
> Indeed, fixed.
I don't think this pattern works when rhs = 0 (https://godbolt.org/z/rvEGqh).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86000