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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D60047#1495528 <https://reviews.llvm.org/D60047#1495528>, @efriedma wrote:

> I don't think I'd look at it in terms of a nondeterministic choice.  I'm not sure that really solves the issue, anyway, because the program flow could potentially constrain the non-determinism.
>
> Really, I think this discussion is going in circles because we don't have a precise definition of what it means to capture a pointer.  "Does not make any copies of the pointer" is unclear.
>
> I spent a few minutes trying to formulate a more precise rule, but the result I came up with is a mess, so I won't post it.  That said, I think the key here is that we have to make sure whatever definition we use allows GVN-like transforms to work: replacing a use of an integer value with a use of an equivalent integer value is legal, and replacing a use of a pointer value with a use of another pointer value with the same raw bits and basis is legal.  So an icmp doesn't count as a "copy" if we can compute the result some other way.


For all purposes so far we interpreted "Does not make any copies of the pointer" as "you cannot reproduce the bit pattern" and we use it to argue that the memory cannot be altered by outside side effects. Allowing arbitrary `icmp` sequences obviously allow to copy the bits (if have to one by one). Thus, `icmp` captures in general. `icmp` against `null` with some conditions on the pointer (see https://reviews.llvm.org/D60047#1495488) cannot really copy a single bit without causing UB. That is what we discuss here (the conditions) and what we need to make generalize the proposed functionality.

@aykevl If you address the inline comments you can probably commit and close this review if you want. The special cases you encoded seem to be sound. If you want you can also generalize the patch and we review that version again. Please let me know what you intend to do.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047