[PATCH] D93820: [InstSimplify] Don't fold gep p, -p to null

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 26 09:55:28 PST 2020


nikic added a comment.

In D93820#2471788 <https://reviews.llvm.org/D93820#2471788>, @nlopes wrote:

> In D93820#2471778 <https://reviews.llvm.org/D93820#2471778>, @nikic wrote:
>
>> In D93820#2471737 <https://reviews.llvm.org/D93820#2471737>, @nlopes wrote:
>>
>>> I only looked at the tests and they were correct before, see here: https://alive2.llvm.org/ce/z/UzW3pv
>>> The tests are weird because they have 'gep inbounds'. The reason they are correct (and weird) is that the only way `p - (int)p/sizeof(*p)` is inbounds is p being null. Anything else will overflow.
>>
>> This doesn't look right to me, at least not given current LangRef wording. Lets say we have `gep inbounds p, -p`, where `p = ptr(base_addr = 1, offset = -1)`. This means that the address value of `p` is `0`, but it has provenance of the object at `base_addr = 1`. As such, the `inbounds` is not violated (both `p` and the gep results are inbounds of the zero address), but we still change provenance.
>
> There's an extra catch: `gep inbounds` requires both the input and output pointers to be in bounds. This part is explicit in LangRef, at least.
> Some examples:
>
>   p = malloc()
>   q = gep inbounds p, -1  // poison
>   r = gep p, -1           // ok
>   s = gep inbounds r, 1   // poison: r is not inbounds
>   t = gep r, 1            // ok, offset = 0
>   u = gep inbounds t, 1   // ok, offset = 1 (assuming malloc size > 0)

Right, but inbounds and provenance are, as far as I can tell, orthogonal concepts. Alive claims that this code has UB due to use of gep inbounds: https://alive2.llvm.org/ce/z/zTctIR At the same time, the gep inbounds itself is not poison: https://alive2.llvm.org/ce/z/wxGGyu That makes it looks like Alive also constrains provenance based on gep inbounds, not just the value of the pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93820



More information about the llvm-commits mailing list