[PATCH] D60047: [CaptureTracking] Don't let comparisons against null escape inbounds pointers

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 7 14:53:07 PDT 2019


jdoerfert added a comment.

In D60047#1493888 <https://reviews.llvm.org/D60047#1493888>, @sanjoy wrote:

> In D60047#1492884 <https://reviews.llvm.org/D60047#1492884>, @jdoerfert wrote:
>
> > I don't see it and your example misses the very important `dereferenceable_or_null` part.
> >
> > Even if they are colluding, the behavior is undefined if the "guessed" (= modified) pointer is outside the dereferenceable range and not null. 
> >  So, assuming the pointer is initially not known and not null. Furthermore, it is `B` bytes dereferenceable and `O` bytes "away" from `null`.
> >  All checks with a positive offset `<= B` are, as mentioned before, useless.
> >  Any check with an offset `> B` that is not equivalent to the `O` offset will cause UB.
> >  Only "guessing" the `O` offset will not cause UB. Now if you could do that, you could also not do the comparison at all and simply go with `ptr + O`.
>
>
> I don't think this is true.  `bar` could have allocated a buffer of size 1000, which happened to end up at address 42000 (numerically) and then (legally, without UB) passed in 42004 to `foo` (`42004` is dereferenceable(4)).  In such a case the null check would not pass and there would not be a valid object at `-4`.
>
> The "collusion" is that whatever pointer `foo` gets, that pointer minus 4 is a valid pointer.  If it got `null` (as is allowed by `deref_or_null`) then `-4` is a valid pointer, if it got `100` then `96` is a valid pointer etc.  This sort of invariant is not that weird; even in the LLVM codebase `llvm::User` pointers point into the "middle" of an object by construction.
>
> > So I don't see how there can be any guessing, colluding, or similar thing without triggering UB almost inevitable or knowing the pointer value beforehand.


Could you explain this again with example code? I don't think I understand what you are saying, sorry.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047





More information about the llvm-commits mailing list