[PATCH] D66664: [FIX] Nonnull is not always implied by dereferenceable

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 25 18:54:23 PDT 2019


jdoerfert added a comment.

In D66664#1643716 <https://reviews.llvm.org/D66664#1643716>, @efriedma wrote:

> If you want to really expand out the meaning of "CanBeNull", it means "was the number of dereferenceable bytes computed using a dereferenceable_or_null attribute/metadata".  It has nothing to do with whether a null pointer is generally valid in the given address space.  The logic has always worked this way, since before it was extracted into a separate function in D17572 <https://reviews.llvm.org/D17572>.


The logic always worked this way is hardly an argument, given that `dereferencebale` alone is already misused (=broken). While initially constructed for something, "CanBeNull" seems like a freestanding definition of the return value to me.

> getPointerDereferenceableBytes is not a good API, sure; if you want to refactor it, fine.

That is what I proposed to do in my RFC to the list and prototyped already (D66618 <https://reviews.llvm.org/D66618>). Please take a look.

> But this patch just breaks it.

I doubt it breaks the API, arguably it makes `CanBeNull` return false only if the pointer "cannot be null", which seems logical to me. When we additionally return "IsKnownDeref" (see D66618 <https://reviews.llvm.org/D66618>) we do not regress anything (probably what you mean by break here) and we could start to use the logic in the `isKnownNonZero` instead of duplicating stuff.

In D66664#1644161 <https://reviews.llvm.org/D66664#1644161>, @joerg wrote:

> It would be nice to also default to this for -ffreestanding.


What default do you want exactly? Did you see D66618 <https://reviews.llvm.org/D66618> and the RFC email I send to the list?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66664





More information about the llvm-commits mailing list