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

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

In D60047#1491414 <https://reviews.llvm.org/D60047#1491414>, @jdoerfert wrote:

> Should we interpret your comment as "the reasoning seems sound", or did you only want to point out the special case which I discuss below?


You should interpret it as a drive by comment. :)  I'm fine if you've thought about the case I suggest and are convinced that this change is sound nevertheless.

>> @sanjoy wrote:
>>  However, this example is kind of iffy because in general the `deref_or_null` attribute is not valid since `p` in `main` could have been something other than `-4`.  But *if* the execution has `p` == `-4` then the attribute is well defined.
> 
> I thought about this case, though I didn't spell it out explicitly in my earlier comment (the part: //as the addition "with infinitely precise signed arithmetic" would not overflow//).
> 
> Let me start by stating that I interpret pointers as unsigned values which, under certain conditions, are allowed to overflow. (Maybe that is in itself a problem in my thinking.)
>  So for me `p` is not `-4` but always some positive value, probably `2^N - 1 - 4` where `N` is the pointer bit-width. If that is acceptable, then the inbounds wording in the lang ref
>  prevents the problem you describe. Any addition with a positive value, e.g., the one you choose `(2^N - 1 - 4) + 4`, cannot be `null` without an overflow and inbounds prohibits this overflow.

I'm not sure if inbounds play into this, and I also noticed my example has typos and also is needlessly confusing; corrected example below:

  int** global_ptr;
  
  void unknown_func();
  
  void foo(int32* /*deref_or_null(4)*/ ptr) {
    if (ptr == null) {
      int32* ptr_leaked = (int32*)(intptr_t)-4;
      *global_ptr = ptr_leaked;
    }
  }
  
  void bar() {
    int32* p = new int32;
    // p happens to numerically be -4 == 2^64-4
    int32* p_with_offset = p + 1; // non-inbounds GEP, evaluates to null
    foo(p_with_offset);
  
    // p has been escaped and we can't DCE the first store:
    *p = 10;
    unknown_func();
    *p = 20;
  }

Now lets say we're only looking at `foo`.  I think the current code will conclude that `foo` does not capture `ptr` (we're comparing a `deref_or_null` pointer against `null`) and we will (lets assume) mark `ptr` as `nocapture` which I believe is incorrect, modulo the iffy bit I mentioned before.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047