-
Notifications
You must be signed in to change notification settings - Fork 14k
LangRef: allocated objects can grow #141338
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-llvm-ir Author: Ralf Jung (RalfJung) ChangesBased on discussion with @nikic. Also Cc @nunoplopes. This enables the (reasonably common) pattern of using Having an allocated object grow seems entirely compatible with what LLVM optimizations assume, except that when LLVM sees an The other main point to be concerned about is having a It would be nice to also allow shrinking allocations (e.g. by
Right now, LLVM could argue that since Full diff: https://github.com/llvm/llvm-project/pull/141338.diff 1 Files Affected:
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 343ca743c74f8..adc38154e7161 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -3327,6 +3327,14 @@ behavior is undefined:
- the size of all allocated objects must be non-negative and not exceed the
largest signed integer that fits into the index type.
+Allocated objects that are created with operations recognized by LLVM (such as
+:ref:`alloca <i_alloca>`, heap allocation functions marked as such, and global
+variables) may *not* change their size. However, allocated objects can also be
+created by means not recognized by LLVM, e.g. by directly calling ``mmap``.
+Those allocated objects are allowed to grow, as long as they always satisfy the
+properties described above. Currently, allocated objects are not permitted to
+ever shrink, nor can they have holes.
+
.. _objectlifetime:
Object Lifetime
@@ -11870,6 +11878,8 @@ if the ``getelementptr`` has any non-zero indices, the following rules apply:
:ref:`based <pointeraliasing>` on. This means that it points into that
allocated object, or to its end. Note that the object does not have to be
live anymore; being in-bounds of a deallocated object is sufficient.
+ If the allocated object can grow, then the relevant size for being *in
+ bounds* is the maximal size the object will ever have, not its current size.
* During the successive addition of offsets to the address, the resulting
pointer must remain *in bounds* of the allocated object at each step.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see an issue with allowing this, but some more opinions would be appreciated.
7caad3a
to
18f4a90
Compare
@@ -11870,6 +11879,8 @@ if the ``getelementptr`` has any non-zero indices, the following rules apply: | |||
:ref:`based <pointeraliasing>` on. This means that it points into that | |||
allocated object, or to its end. Note that the object does not have to be | |||
live anymore; being in-bounds of a deallocated object is sufficient. | |||
If the allocated object can grow, then the relevant size for being *in | |||
bounds* is the maximal size the object will ever have, not its current size. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this semantics is problematic as you need to guess the future.
We need getelementptr to produce poison if it goes OOB, and with this wording, you need to delay the decision until the program exits, and then propagate it backwards.
This has implications in alias analysis. We would need to disable all rules that use reasoning such as p + offset > p's size
to conclude no-alias, because the size may be increased later. We have a few of these rules in BasicAA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this semantics is problematic as you need to guess the future. We need getelementptr to produce poison if it goes OOB, and with this wording, you need to delay the decision until the program exits, and then propagate it backwards.
See the comments above. I think we can avoid this issue by rephrasing this to something like:
If the allocated object can grow, then the relevant size for being in bounds is the maximal possible size the object could have (while satisfying the allocated object rules), not its current size.
I believe this still gives us all the properties we need from inbounds (in particular the ability to cross more than half the address space, even with a sequence of multiple inbounds operations).
This has implications in alias analysis. We would need to disable all rules that use reasoning such as
p + offset > p's size
to conclude no-alias, because the size may be increased later. We have a few of these rules in BasicAA.
This alias analysis only applies to fixed size objects with known size. I do not believe it will be affected by this change (which is only relevant to allocations which for LLVM does not know the size).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has implications in alias analysis. We would need to disable all rules that use reasoning such as
p + offset > p's size
to conclude no-alias, because the size may be increased later. We have a few of these rules in BasicAA.This alias analysis only applies to fixed size objects with known size. I do not believe it will be affected by this change (which is only relevant to allocations which for LLVM does not know the size).
Alias analysis works over heap-allocated objects. Anything that LLVM (MemoryBuiltins.h) can infer the size is fair game.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would need to disable all rules that use reasoning such as p + offset > p's size to conclude no-alias, because the size may be increased later.
No, we don't -- the PR explicitly discusses this: all allocated objects created by operations that are built-in to LLVM must never change their size.
Alias analysis works over heap-allocated objects. Anything that LLVM (MemoryBuiltins.h) can infer the size is fair game.
Indeed, and that's fine. All we need is some way to allocate memory such that LLVM cannot infer the size (and promises to never infer it) -- e.g. by calling mmap
, which I assume LLVM does not have a native understanding of.
Longer-term it may also be useful to offer a flag for malloc-like functions so that frontends can communicate to LLVM whether this allocation is allowed to change size or not, but that's left to future work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this semantics is problematic as you need to guess the future. We need getelementptr to produce poison if it goes OOB, and with this wording, you need to delay the decision until the program exits, and then propagate it backwards.
My idea was that conceptually the size would be given in the code, e.g. if we had an actual formal model. LLVM IR would just omit it since y'all don't like spec-only parameters. ;)
But as long as we keep the start of the allocation fixed, then as Nikita says we can also just say that the relevant size is the theoretical maximum, not the actual maximum -- and that is a simple pure function of the start address of the allocation.
18f4a90
to
90224fd
Compare
@@ -3327,6 +3327,15 @@ behavior is undefined: | |||
- the size of all allocated objects must be non-negative and not exceed the | |||
largest signed integer that fits into the index type. | |||
|
|||
Allocated objects that are created with operations recognized by LLVM (such as | |||
:ref:`alloca <i_alloca>`, heap allocation functions marked as such, and global |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "heap allocation functions marked as such" part here is meant to capture everything LLVM recognizes as a heap allocation function. Is there a better way to say this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "heap allocation functions with the allocsize
attribute"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't LLVM also recognize malloc
as a special magic name?
Those allocated objects are allowed to grow to the right (i.e., keeping the same | ||
base address, but increasing their size), as long as they always satisfy the | ||
properties described above. Currently, allocated objects are not permitted to | ||
grow to the left or to shrink, nor can they have holes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It occurred to me that it may be helpful to point out here that realloc
-style operations do not grow the allocation the sense described here. They always create a new allocation with fresh provenance, even if it may have the same base address as the previous allocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I'll add that.
90224fd
to
3fbf788
Compare
keeping the same base address, but increasing their size) while maintaining the | ||
validity of existing pointers, as long as they always satisfy the properties | ||
described above. Currently, allocated objects are not permitted to grow to the | ||
left or to shrink, nor can they have holes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the restrictions, the compiler can't tell where the "beginning" of the object is, so I'm not sure forbidding growth to the left has any meaningful effect.
I'm not sure what a "hole" is, in this context. I don't think we require that all bytes of an object have to be dereferenceable. It might make sense to forbid overlapping live objects, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the restrictions, the compiler can't tell where the "beginning" of the object is, so I'm not sure forbidding growth to the left has any meaningful effect.
It does have the one effect that it simplifies specifying when getelementptr inbounds
is allowed for this allocated object. In particular if we combine this with changing the "beginning" of an allocated object by shrinking it, we can have a situation like:
- let's say an allocation starts at
addr
and ends atmid
and covers 1/3 of the address space - at moment A, it is okay to
inbounds
offset a pointer by 1/3 of the address space to the right fromaddr
- then we shrink the allocation from the left, keeping only its last page
- then we grow the allocation to the right
- at moment B, it is okay to
inbounds
offset a pointer by 1/3 of the address space to the right frommid
But pointer offset is meant to be freely reorderable, so we could move the two offsets next to each other. We also can combine adjacent inbounds
offset, preserving inbounds
. But now we have an inbounds
offset covering 2/3 of the address space which returns poison
, so one of these steps was invalid.
I'm not sure what a "hole" is, in this context. I don't think we require that all bytes of an object have to be dereferenceable. It might make sense to forbid overlapping live objects, though.
By "hole" I mean e.g. munmap
ing a page from the middle of an otherwise contiguous range of allocated pages, while still considering the entire thing to be a single allocation. That seems like it would open a can of worms, e.g. optimizations could no longer assume things like:
- load from
%ptr
- let
%ptr2
be%ptr
offset by 8k bytes (withoutinbounds
) - load from
%ptr2
- Now we know the range between the two pointers is dereferenceable
Also, what exactly does getelementptr inbounds
mean in the presence of holes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular if we combine this with changing the "beginning" of an allocated object by shrinking it
But you currently forbid shrinking?
I mean e.g. munmaping a page from the middle of an otherwise contiguous range of allocated pages
I agree munmapping is problematic; overlapping objects would be hard to reason about. There are potentially useful "holes", though: mprotecting a page from a continuous range for a JIT would be a hole in the sense that reads aren't legal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But you currently forbid shrinking?
Yeah but I'd like to allow it in the future -- it seems more important to me than growing to the left.
I agree munmapping is problematic; overlapping objects would be hard to reason about. There are potentially useful "holes", though: mprotecting a page from a continuous range for a JIT would be a hole in the sense that reads aren't legal.
I don't think an mprotect causing reads and writes to trap is fundamentally different from munmap for this purpose, or is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah but I'd like to allow it in the future -- it seems more important to me than growing to the left.
Hmm, okay.
I don't think an mprotect causing reads and writes to trap is fundamentally different from munmap for this purpose, or is it?
Alias analysis doesn't really care if an object is actually accessible at the moment; it just cares we don't overlap with some other object. Dereferenceability is only relevant if you want to do speculative loads. So you can separate dereferenceability from the rest of the properties of an allocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alias analysis doesn't really care if an object is actually accessible at the moment; it just cares we don't overlap with some other object. Dereferenceability is only relevant if you want to do speculative loads. So you can separate dereferenceability from the rest of the properties of an allocation.
Right, but my example above used dereferenceability. So it is equally an example for why mprotect on the middle of an allocation could be problematic.
Based on discussion with @nikic. Also Cc @nunoplopes.
This enables the (reasonably common) pattern of using
mmap
to reserve but not actually map a wide range of pages, and then only adding in more pages as memory is actually needed. Effectively, that region of memory is one big allocated object for LLVM, but crucially, that allocated object changes its size.Having an allocated object grow seems entirely compatible with what LLVM optimizations assume, except that when LLVM sees an
alloca
or similar instruction, it will assume that a pointer that has beengetelementptr inbounds
by more than the size of the allocated object cannot alias thatalloca
. But for allocated objects that are created e.g. bymmap
, where LLVM does not know their size, this cannot happen anyway.The other main point to be concerned about is having a
getelementptr inbounds
that is moved up across an operation that grows an allocated object: this should be legal asgetelementptr
is freely reorderable. We achieve that by saying that for allocated objects that change their size, "inbounds" means "inbounds of their maximal size", not "inbounds of their current size".It would be nice to also allow shrinking allocations (e.g. by
munmap
ing pages at the end), but that is more tricky. Consider an example like this:ptr
ptr
Right now, LLVM could argue that since
ptr
clearly has not been deallocated, there must be at least 4 bytes of dereferenceable memory behindptr
after the call. If allocations can shrink, this kind of reasoning is no longer valid. I don't know if LLVM actually does reasoning like that -- I think it should not, since I think it should be possible to have allocations that shrink -- but to remain conservative I am not proposing that as part of this patch.