[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#1491331 <https://reviews.llvm.org/D60047#1491331>, @jdoerfert wrote:

> 1. A scope (=function) in which `null` is not a valid pointer (see `NullPointerIsDefined(Function *, unsigned AddrSpace)`)
> 2. A `dereferenceable_or_null` pointer `Ptr` (see `Value::getPointerDereferenceableBytes`) Now comparing `Ptr` agains `null` should be `noescape`.
>
> My reasoning (which has to be checked!):
>
> The idea is that you cannot arbitrarily manipulate the pointer while keeping the two properties.
>  If `Ptr` is `null` to begin with, we are fine (I think).
>  If it is not, it points to an allocation with extend `[start:end)` where `end - start` equals the dereferenceable bytes.
>  Due to property 1) we know `null` is not contained in `[start:end)`.
>  That means we cannot manipulate a pointer such that 2) holds and a `null` comparison would leak information because it always has to be `false`.


One pedantic case is when `end` is `null`:

  void f(int32* /*deref_or_null(4)*/ ptr) {
    if (ptr == null) {
      int32* ptr_leaked = (int32*)-4;
      *ptr_leaked = 42;
    }
  }
  
  int main() {
    int* p = new int;
    // p happens to numerically be -4
    f(p);
    print(*p); // Should print 42
  }

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.



================
Comment at: lib/Analysis/CaptureTracking.cpp:334
     case Instruction::ICmp: {
-      // Don't count comparisons of a no-alias return value against null as
-      // captures. This allows us to ignore comparisons of malloc results
-      // with null, for example.
       if (ConstantPointerNull *CPN =
+          dyn_cast<ConstantPointerNull>(I->getOperand(1))) {
----------------
`auto *` is probably appropriate here.  Same for `GEP` and `A` below.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047