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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

We need to address the comments before we commit this.

__Sorry that I only look at this so late in the game!__



================
Comment at: lib/Analysis/CaptureTracking.cpp:342
             break;
+        auto *O = I->getOperand(0)->stripPointerCasts();
+        // A getelementptr inbounds is either null or an in-bounds GEP, which
----------------
Strip pointer casts is actually/unfortunately too strong because it strips address space casts:
See also: http://lists.llvm.org/pipermail/llvm-dev/2018-December/128423.html


================
Comment at: lib/Analysis/CaptureTracking.cpp:345
+        // means that there is no way to let a pointer escape using clever GEP
+        // hacking.
+        if (GetElementPtrInst *GEP =
----------------
I do not understand the comment above. A GEP inbounds can have any value, including 0, or am I missing a precondition here? Could you also describe why non-inbounds GEPs are not eligible?


================
Comment at: lib/Analysis/CaptureTracking.cpp:353
+        // (in-bounds) pointer.
+        if (Argument *A = dyn_cast<Argument>(O))
+          if (A->getDereferenceableOrNullBytes())
----------------
Why is this specific to arguments? We have other pointers that can be `dereferenceable_or_null`, see `Value::getPointerDereferenceableBytes`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047