[PATCH] D66651: Annotate return values of allocation functions with dereferenceable_or_null

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp:4200
+                          Call.getContext(), Op1C->getZExtValue()));
+  } else if (isCallocLikeFn(&Call, TLI) && Op0C && Op1C) {
+    bool Overflow;
----------------
xbolva00 wrote:
> jdoerfert wrote:
> > xbolva00 wrote:
> > > xbolva00 wrote:
> > > > jdoerfert wrote:
> > > > > xbolva00 wrote:
> > > > > > xbolva00 wrote:
> > > > > > > xbolva00 wrote:
> > > > > > > > xbolva00 wrote:
> > > > > > > > > jdoerfert wrote:
> > > > > > > > > > What if one is NULL but the other not? Will `getWithDereferenceableOrNullBytes(0)` do the right thing?
> > > > > > > > > Maybe isNullValue confused you. isNullValue means just CstInt->getValue() == 0.
> > > > > > > > > 
> > > > > > > > > if Op0C is NULL -> no calloc annotation
> > > > > > > > > if Op1C is NULL -> no calloc annotation
> > > > > > > > > 
> > > > > > > > > if Op0C is '0' -> no annotation; see
> > > > > > > > > if ((Op0C && Op0C->isNullValue()) || (Op1C && Op1C->isNullValue()))
> > > > > > > > >     return;
> > > > > > > > > 
> > > > > > > > > Same for Op1C.
> > > > > > > > I will add a comment like
> > > > > > > > 
> > > > > > > >   // Bail out if the allocation size is zero.
> > > > > > > >   if ((Op0C && Op0C->isNullValue()) || (Op1C && Op1C->isNullValue()))
> > > > > > > >     return;
> > > > > > > Aha, I know what do you mean.
> > > > > > > 
> > > > > > > Hmm, it could work for calloc case - I will look on it.
> > > > > > eh, no :D
> > > > > > 
> > > > > > calloc(1, unknown_var) - We can do nothing in this case ..
> > > > > `calloc(1, 0)` or `calloc(0, 1)` is the question
> > > > No annotation for this cases - why should we? Calloc internally multiplies it
> > > > 
> > > > So calloc(0,1) is logically same as
> > > > 
> > > > p = malloc(0*1);
> > > > memset(p, 0, 0*1)
> > > man calloc
> > > "If nmemb or size is 0, then calloc() returns either NULL, or a unique pointer value that can later  be  successfully passed to free()."
> > > 
> > > (so same as malloc(0))
> > > 
> > > We could fold these cases to NULL, but not worth since this rather programmer's mistake than optimization opportunity.
> > > 
> > I am not asking for an annotation but if we don't accidentally put one as we should not.
> > > What if one is NULL but the other not? Will getWithDereferenceableOrNullBytes(0) do the right thing?
> getWithDereferenceableOrNullBytes(Call.getContext(), 0) never happens here.
> 
> If Op1 is zero int or Op0 is zero int (or both are zeros), we bail out soon.
> 
> We multiplies non zero numbers here.
My bad. sry.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66651