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

Mon May 20 21:43:06 PDT 2019

jdoerfert added a comment.

In D60047#1507601 <https://reviews.llvm.org/D60047#1507601>, @aykevl wrote:

> I've updated the code, but there are a few things I'm not so sure about:

Thx!

> - It seems to me that comparisons against null might escape a pointer if the function is marked `"null-pointer-is-valid"`?

"Correct", see below.

> But at the same time, that seems to be equivalent to comparing against a raw pointer (for example, `ptr == (uint32*)0x1000`) in a function that is not marked `"null-pointer-is-valid"` so I don't think it's a special case: if the comparison returns true you already had the pointer (in the form of a constant) so no pointer really escapes here.

The discussion we had earlier concluded in the fact that the combination of `deref_or_null` + `null` is not valid will give you `no-escape`. If you rempove any part, it doesn't work. So without `"null-pointer-is-valid"`, `null` might be valid. And any other address might always be valid. In neither case we can conclude `no-escape`.

> - Can pointers in non-0 namespaces be considered noescape with this change?

It depends on the namespace, see below.

>   It might be better to be conservative and just don't apply this optimization to non-0 AS pointers. On the other hand, the same reasoning as in the previous point might apply here.

There is the method to determine if `null` is valid in a namespace, use that one and it should be fine (I think part of `Function`).

> Also, there is the `stripPointerCasts` issue that has not yet been resolved, waiting for D61607 <https://reviews.llvm.org/D61607> to be merged.

Correct, though I hope we will come to a conclusion on that one soon.

================
Comment at: lib/Analysis/CaptureTracking.cpp:353
+        // (in-bounds) pointer.
+        if (Argument *A = dyn_cast<Argument>(O))
+          if (A->getDereferenceableOrNullBytes())
----------------
aykevl wrote:
> jdoerfert wrote:
> > Why is this specific to arguments? We have other pointers that can be `dereferenceable_or_null`, see `Value::getPointerDereferenceableBytes`
> Does `Value::getPointerDereferenceableBytes` have the same semantics (ignoring the CanBeNull) as `dereferenceable_or_null`? I see there are quite a few things that make a pointer dereferenceable, like `byval`. It seems to me that the same reasoning that holds true for `dereferenceable_or_null` also holds true for any dereferenceable (or null) pointer, but just want to make sure.
> 
> Also, where do I get a `DataLayout` from?
`Value::getPointerDereferenceableBytes` is what you want here. If it is not (in practice) then it is broken and we need to fix it. `byval` is a "stronger" version of `dereferenceable`.

> Also, where do I get a DataLayout from?

>From the Module: `Module::getDataLayout()`

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60047