[PATCH] D124633: [SimplifyLibCalls] Pointers passed to libcalls must point to valid objects

[Martin Sebor via Phabricator via llvm-commits]

msebor accepted this revision.
msebor added a comment.
This revision is now accepted and ready to land.

This revision looks good to me.  I suggest reviewing the rest of the folders for missing annotations and adding those in a follow up patch.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp:155
 static void annotateNonNullNoUndefBasedOnAccess(CallInst *CI,
-                                         ArrayRef<unsigned> ArgNos) {
+                                         ArrayRef<unsigned> ArgNos, bool MinDereferenceableBytes = 1) {
   Function *F = CI->getCaller();
----------------
xbolva00 wrote:
> msebor wrote:
> > msebor wrote:
> > > It seems that for a `bool` the default ought  to `true` rather than 1 (ditto for the actual argument at the call sites); a better name for the argument would also reflect the fact it's a toggle rather than a count.
> > > 
> > > But I wonder if changing the last argument to something like `Value *MinBytes = nullptr` and computing the Boolean result from it here would be a way to simplify the code and avoid having to do that at the call site.
> > It occurs to me that since the function is also called when folding `wcslen` where the annotation refers to the number of wide characters,  using the word //byte// for the argument isn't entirely accurate.  It should probably be //element// instead (ditto in `annotateDereferenceableBytes`).
> >> It occurs to me that since the function is also called when folding wcslen
> how? I see no annotateNonNullNoUndefBasedOnAccess in optimizeWcslen.
You're right, I must have been looking at the narrow character version.  That seems like an oversight.  I'd expect `optimizeWcslen` to annotate the pointer argument the same way most of the narrow functions do, wouldn't you?

Some narrow character folders like `optimizeStrPBrk` and `optimizeStrSpn` among others are also missing a call to `annotateNonNullNoUndefBasedOnAccess`, so they might all need updating.  If so, that could be done in a followup change.


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

https://reviews.llvm.org/D124633