[PATCH] D123626: [InstCombine] Fold memrchr calls with constant size of zero or one.

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:873
+  Value *Size = CI->getArgOperand(2);
+  annotateNonNullAndDereferenceable(CI, 0, Size, DL);
+  Value *CharVal = CI->getArgOperand(1);
----------------
xbolva00 wrote:
> nikic wrote:
> > nikic wrote:
> > > I'm not entirely sure this is correct. This is definitely incorrect for memchr() and I submitted D123665 to fix that. memrchr() is a GNU extension, so I'm not sure whether the same rules apply there. I guess one could argue that memrchr() should require a fully dereferenceable object, as otherwise the base pointer will already point outside the object.
> > As a data point, sanitizer interception for these functions is implemented as follows: https://github.com/llvm/llvm-project/blob/ee44896cf42d3360f7e0229c360258c92262a47b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc#L984-L1021 So while the interceptor does not assume full dereferenceability for memchr(), it does assume so for memrchr(). So your current implementation would be consistent with sanitizers.
> Any sanitizer developer here?
> 
> It would be great to know why there is a such differenr behaviour.
“ I guess one could argue that memrchr() should require a fully dereferenceable object, as otherwise the base pointer will already point outside the object”

But yeah this makes sense


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

https://reviews.llvm.org/D123626