[LangRef] Clarify that the pointer after an object must be valid. #127892

fhahn · 2025-02-19T20:36:58Z

In some places, we rely on the assumption that the pointer after the object must also be valid and not overflow, but it does not seem to be spelled out clearly in LangRef, unless I missed a reference.

The GetElementPtr section mentions that the maximum object size is half the pointer index type space, but then the pointer past the object may wrap. Clarify that the pointer after the object must also be valid.

This should match Alive2's semantics: https://alive2.llvm.org/ce/z/Dk8QFL (https://github.com/AliveToolkit/alive2/blob/master/tools/transform.cpp#L1288)

fhahn · 2025-02-19T20:37:28Z

Not sure if there is a better place to document the assumption than in the GEP semantics

llvmbot · 2025-02-19T20:37:32Z

@llvm/pr-subscribers-llvm-ir

Author: Florian Hahn (fhahn)

Changes

In some places, we rely on the assumption that the pointer after the object must also be valid and not overflow, but it does not seem to be spelled out clearly in LangRef, unless I missed a reference.

The GetElementPtr section mentions that the maximum object size is half the pointer index type space, but then the pointer past the object may wrap. Clarify that the pointer after the object must also be valid.

This should match Alive2's semantics: https://alive2.llvm.org/ce/z/Dk8QFL (https://github.com/AliveToolkit/alive2/blob/master/tools/transform.cpp#L1288)

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

1 Files Affected:

(modified) llvm/docs/LangRef.rst (+3-2)

diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index deb87365ae8d7..8a12e6b964a9f 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -11722,8 +11722,9 @@ As a corollary, the only pointer in bounds of the null pointer in the default
 address space is the null pointer itself.
 
 These rules are based on the assumption that no allocated object may cross
-the unsigned address space boundary, and no allocated object may be larger
-than half the pointer index type space.
+the unsigned address space boundary, the pointer after the object must be valid,
+and no allocated object may be larger than half the pointer index type space
+- 1.
 
 If ``inbounds`` is present on a ``getelementptr`` instruction, the ``nusw``
 attribute will be automatically set as well. For this reason, the ``nusw``

nikic · 2025-02-19T20:40:28Z

Not sure if there is a better place to document the assumption than in the GEP semantics

It would make sense to add a separate section on allocated objects.

Edit: See also https://doc.rust-lang.org/std/ptr/index.html#allocated-object for Rust's documentation of LLVM's allocated object rules.

nikic · 2025-02-19T20:51:11Z

llvm/docs/LangRef.rst

@@ -11722,8 +11722,9 @@ As a corollary, the only pointer in bounds of the null pointer in the default
 address space is the null pointer itself.

 These rules are based on the assumption that no allocated object may cross
-the unsigned address space boundary, and no allocated object may be larger
-than half the pointer index type space.
+the unsigned address space boundary, the pointer after the object must be valid,


"must be valid" is ambiguous. I'd explicitly say "must not be null".

I would express it in terms of overflow because null can have a different connotation in other address spaces.
I suggest a remark like:

based on the assumption that no allocated object may cross the unsigned address space boundary (including a pointer after the end of the object).

Updated the wording, thanks

nikic · 2025-02-19T20:55:48Z

llvm/docs/LangRef.rst

-than half the pointer index type space.
+the unsigned address space boundary, the pointer after the object must be valid,
+and no allocated object may be larger than half the pointer index type space
+- 1.


I'd phrase this as "than the largest signed integer that fits into the index type". It's not super clear what exactly "half" here means, but the intention is that the size is <= SignedMax.

Updated to be non negative and < SignedMax (otherwise there may be no room for the pointer after the object).

I don't follow. The size already points to the one-past-the-end location, so why do you need the extra increment?

I was still thinking about the previous wording, which I read as it takes up all signed positive addresses. Should be adjusted nowthanks

fhahn

Introduce a section for Allocated Objects, re-using parts of the Object Lifetime section.

I also updated various references to consistently refer to allocated objects. I could also split this off the a separate PR if preferred?

nunoplopes · 2025-02-20T17:30:24Z

looks good to me, thanks!

nikic

LGTM

nikic · 2025-02-23T19:42:49Z

llvm/docs/LangRef.rst

+that is not based on the object tries to read or write to the object, it is
+undefined behavior.
+
+The following properties hold for all allocated objects:


This should also have "otherwise, the behavior is undefined" wording.

Added, thanks

nikic · 2025-02-23T19:45:52Z

llvm/docs/LangRef.rst

-These rules are based on the assumption that no allocated object may cross
-the unsigned address space boundary, and no allocated object may be larger
-than half the pointer index type space.
+These rules are based on the assumption for


Suggested change

These rules are based on the assumption for

These rules are based on the assumptions for

Though I'm not sure we really need this sentence anymore, it may make more sense to link "allocated object" in the bullet point above.

Updated the bullet point to link to the allocated objects section and remove the sentence here.

nikic · 2025-02-23T19:46:46Z

llvm/docs/LangRef.rst

@@ -26458,7 +26469,7 @@ Overview:
 """""""""

 The '``llvm.invariant.end``' intrinsic specifies that the contents of a


Suggested change

The '``llvm.invariant.end``' intrinsic specifies that the contents of a

The '``llvm.invariant.end``' intrinsic specifies that the contents of an

Fixed thanks

In some places, we rely on the assumption that the pointer after the object must also be valid and not overflow, but it does not seem to be spelled out clearly in LangRef, unless I missed a reference. The GetElementPtr section mentions that the maximum object size is half the pointer index type space, but then the pointer past the object may wrap. Clarify that the pointer after the object must also be valid. This should match Alive2's semantics: https://alive2.llvm.org/ce/z/Dk8QFL (https://github.com/AliveToolkit/alive2/blob/master/tools/transform.cpp#L1288)

… valid. (#127892) In some places, we rely on the assumption that the pointer after the object must also be valid and not overflow, but it does not seem to be spelled out clearly in LangRef, unless I missed a reference. The GetElementPtr section mentions that the maximum object size is half the pointer index type space, but then the pointer past the object may wrap. Clarify that the pointer after the object must also be valid. This should match Alive2's semantics: https://alive2.llvm.org/ce/z/Dk8QFL (https://github.com/AliveToolkit/alive2/blob/master/tools/transform.cpp#L1288) PR: llvm/llvm-project#127892

fhahn requested review from nikic, preames, nunoplopes, jdoerfert and efriedma-quic February 19, 2025 20:36

llvmbot added the llvm:ir label Feb 19, 2025

nikic reviewed Feb 19, 2025

View reviewed changes

fhahn force-pushed the langref-gep-pointer-after-object-valid branch 2 times, most recently from 4221331 to 2ec31c0 Compare February 20, 2025 16:39

fhahn commented Feb 20, 2025

View reviewed changes

fhahn force-pushed the langref-gep-pointer-after-object-valid branch 2 times, most recently from eadd721 to 66b93da Compare February 23, 2025 18:09

fhahn mentioned this pull request Feb 23, 2025

[LangRef] Document accessing memory outside of object is UB. #128429

Open

nikic approved these changes Feb 23, 2025

View reviewed changes

fhahn changed the title ~~[LangRef] Clarify that the pointer after an objet must be valid.~~ [LangRef] Clarify that the pointer after an object must be valid. Feb 24, 2025

fhahn added 4 commits February 24, 2025 16:51

!Fixup add allocated object section.

9b0bae4

!fixup adjust size wording

371b527

!fixup address latest comments, thanks!

c0cc0af

fhahn force-pushed the langref-gep-pointer-after-object-valid branch from 66b93da to c0cc0af Compare February 24, 2025 17:02

fhahn merged commit 47822c8 into llvm:main Feb 24, 2025
10 of 12 checks passed

fhahn deleted the langref-gep-pointer-after-object-valid branch February 24, 2025 21:23

	These rules are based on the assumption for
	These rules are based on the assumptions for

		@@ -26458,7 +26469,7 @@ Overview:
		"""""""""

		The '``llvm.invariant.end``' intrinsic specifies that the contents of a

[LangRef] Clarify that the pointer after an object must be valid. #127892

[LangRef] Clarify that the pointer after an object must be valid. #127892

Uh oh!

Conversation

fhahn commented Feb 19, 2025

Uh oh!

fhahn commented Feb 19, 2025

Uh oh!

llvmbot commented Feb 19, 2025

Uh oh!

nikic commented Feb 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fhahn left a comment

Choose a reason for hiding this comment

Uh oh!

nunoplopes commented Feb 20, 2025

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nikic commented Feb 19, 2025 •

edited

Loading