False negative when inferring gep nuw on alloca

[nikic]

This verifies:

define ptr @src_alloca(i64 %x) {
  %p = alloca [128 x i8]
  %gep = getelementptr i8, ptr %p, i64 %x
  ret ptr %gep
}

define ptr @tgt_alloca(i64 %x) {
  %p = alloca [128 x i8]
  %gep = getelementptr nuw i8, ptr %p, i64 %x
  ret ptr %gep
}

The equivalent code with a global does correctly require inbounds on the original GEP.

https://alive2.llvm.org/ce/z/C6amHn

[nunoplopes]

Ah, I think this example is too simplified.
See here for a more complex example: https://alive2.llvm.org/ce/z/fgEo3X

The issue is that returning a pointer to alloca is equivalent to returning poison, hence the original function was already returning poison.

[nikic]

Hm, why is returning a pointer to an alloca poison? Shouldn't it be equivalent to (basically) freeze poison?

[nunoplopes]

It's up to us, I guess, since I don't think it's defined in langref.

But what can a caller do with a pointer to the callee's stack? It cannot dereference it. It could compare it, but why leak the address space?
Returning a local pointer is UB in C, so returning poison is justified. We could change to freeze poison, but since poison is justified, I would go with it. It's just easier to reason about IMHO.
(poison is also more efficient for Alive than freeze).

[regehr]

I struggle to think of a use case for this situation where freeze poison would be more useful than poison

[nikic]

Returning poison is incompatible with Rust, or any other language that does not have pointer zapping semantics.

It's up to us, I guess, since I don't think it's defined in langref.

It's defined by omission. If we don't explicitly specify it's poison, then it can't be poison because alloca does not return a poison value.

[nunoplopes]

But Rust does not allow one to return a pointer to a local frame, right? It either returns a pointer to a referenced-counted object, one of the arguments, or a moved object (like C++).
But I'm not an expert in Rust. But I don't understand how this decision would impact Rust.

[nikic]

But Rust does not allow one to return a pointer to a local frame, right? It either returns a pointer to a referenced-counted object, one of the arguments, or a moved object (like C++). But I'm not an expert in Rust. But I don't understand how this decision would impact Rust.

You can take a pointer to a local and return it. It does not even require unsafe code: https://rust.godbolt.org/z/r8aEGjcqd

Of course it will be UB to perform an access through the pointer after the return, but you can still use it in other well-defined ways. (E.g. by printing the address or comparing it to null.)

[nikic]

Probably also worth clarifying that it's not literally freeze poison -- the address still has to satisfy the usual requirements, like being aligned, not null, etc. TBH I don't really get why return of alloca requires/gets any special treatment at all. It seems like it should be just like any other use of a pointer? Or maybe a use with stripped provenance, if we want to be a bit more precise?

[nunoplopes]

Ok, that example from Rust scares me. cc @RalfJung

We started with changing local stack pointers to poison on return because that was the easiest way to model this. But for Rust we would need to change the memory model to have dead stack objects (which we already kinda have because of the lifetime intrinsics) and just return a dead stack object. Not sure what the refinement condition looks like? If we need to support gep inbounds on these pointers, then they should have at least the same amount of offsetting to the left and to the right as the source pointer. Or do we want to say it has to have the exact same size, offset, alignment?

LLVM does seems to be pretty nice and not touching the alloca or geps at all: https://gcc.godbolt.org/z/KhPWvzaTr

[nikic]

Right, I'd expect the semantics here to be the same as using the alloca pointer after a lifetime.end -- including the semantics that has for inbounds (which I believe is not affected by lifetime.start/end, to preserve free sinking/hoisting of GEPs, so it's inbounds if it were inbounds of the now dead allocation).

[nunoplopes]

Ok, that makes sense. I will try this out and report back if there some test cases fail.