Skip to content

LangRef: getelementptr: inbounds is about the object the pointer is 'based on' #95650

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

Merged
merged 1 commit into from
Jun 17, 2024

Conversation

RalfJung
Copy link
Contributor

As discussed here, we need the pointer to be inbounds of the allocated object the pointer is based on, not just any allocated object.

@llvmbot
Copy link
Member

llvmbot commented Jun 15, 2024

@llvm/pr-subscribers-llvm-ir

Author: Ralf Jung (RalfJung)

Changes

As discussed here, we need the pointer to be inbounds of the allocated object the pointer is based on, not just any allocated object.


Full diff: https://github.com/llvm/llvm-project/pull/95650.diff

1 Files Affected:

  • (modified) llvm/docs/LangRef.rst (+4-4)
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index 6935ccdfc9196..fcf6d310d03de 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -11390,10 +11390,10 @@ For ``nuw`` (no unsigned wrap):
 For ``inbounds`` all rules of the ``nusw`` attribute apply. Additionally,
 if the ``getelementptr`` has any non-zero indices, the following rules apply:
 
- * The base pointer has an *in bounds* address of an allocated object, which
-   means that it points into an 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.
+ * The base pointer has an *in bounds* address of the allocated object that it
+   is :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.
  * During the successive addition of offsets to the address, the resulting
    pointer must remain *in bounds* of the allocated object at each step.
 

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable to me. I think this requirement is necessary to ensure that gep inbounds (gep inbounds p, a), b refines to gep inbounds (p, a+b). Otherwise, p+a could point both to the end of one and start of another allocated object, such that a+b overflows. (And even independently of overflow, the start and end would be inbounds of different objects.)

One thing to note here is that this says "the allocated object", while the current inttoptr phrasing implies that a pointer may be based on multiple allocated objects. But I think that's a possibility we should exclude from the pointer aliasing semantics.

@nunoplopes How does alive2 currently model inbounds?

@RalfJung
Copy link
Contributor Author

The current ptrtoint/inttoptr semantics are completely incoherent anyway, as shown by issues such as #33896 and #34577.

Copy link
Member

@nunoplopes nunoplopes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.
If we ignore int2ptr casts, then Alive2 implements the proposed semantics. For int2ptr, I agree with Ralf that we are not in a consistent state right now, so it doesn't matter.

@nikic nikic merged commit 9b933e9 into llvm:main Jun 17, 2024
7 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants