rust-lang
[ptr] Document maximum allocation size
Partially addresses https://github.com/rust-lang/unsafe-code-guidelines/issues/465
r? @joshtriplett
(rustbot has picked a reviewer for you, use r? to override)
FCP passed in https://github.com/rust-lang/rust/pull/116988 so you can now assume that null is 0 here.
These are fairly fundamental requirements of rust. Shouldn't they also go into the reference?
Yeah it's the usual issue where the stdlib docs is more likely to be actually visible but conceptually the reference is where this belongs. But given that this already defines "allocated object", this seems like a sensible place.
I think what is left here is uncontroversial:
It is guaranteed that an allocated object never spans more than
isize::MAXbytes. An allocated object cannot contain [null()] (i.e., the address with the numerical value 0) and cannot contain the last (usize::MAX) byte of the address space.
But let's ping some people and nominate to see if anyone disagrees. Cc @rust-lang/opsem @rust-lang/lang
r? @RalfJung
Yeah it's the usual issue where the stdlib docs is more likely to be actually visible but conceptually the reference is where this belongs.
There are those newfangled thingies called hyperlinks. 😉
Imo the reference should at least contain the requirement while std spells out the consequences of that. How it interacts with Layout, Allocator, pointer methods etc.
The type layout page already contains a section on size and alignment. The maximum allocation size could be specified there. Or maybe under the currently quite anemic memory model page.
The type layout page already contains a section on size and alignment. The maximum allocation size could be specified there.
No that's definitely the wrong page, this guarantee has nothing to do with types nor layouts.
Or maybe under the currently quite anemic memory model page.
Yeah that makes more sense. It's just hard to specify a memory model in a piecemeal way so it's somewhat unclear what one should say there.
Team member @tmandry has proposed to merge this. The next step is review by the rest of the tagged team members:
- [ ] @joshtriplett
- [ ] @nikomatsakis
- [x] @pnkfelix
- [x] @scottmcm
- [x] @tmandry
No concerns currently listed.
Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!
cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. See this document for info about what commands tagged team members can give me.
@rustbot labels -I-lang-nominated
As tmandry noted, we discussed this today in triage and were positive on doing it. There was some discussion about the wording:
It is guaranteed that an allocated object never spans more than
isize::MAXbytes. An allocated object cannot contain [null()] (i.e., the address with the numerical value 0) and cannot contain the last (usize::MAX) byte of the address space.
The concern was about potential ambiguity on whether this was talking about the contents of the allocated object or about the addresses across which it is allocated. @nikomatsakis proposed wording to the effect of, "the range of addresses used by an allocated object cannot overlap with null."
This is now proposed for FCP merge, so we'll unnominate.
The concern was about potential ambiguity on whether this was talking about the contents of the allocated object or about the addresses across which it is allocated. @nikomatsakis proposed wording to the effect of, "the range of addresses used by an allocated object cannot overlap with null."
Ah, good point. I would suggest something like, "the set of addresses used by an allocated object cannot contain null". We're carefully not committing to contiguous objects.
How do folks feel about this proposed text, possibly modified according to this feedback? I believe it addresses this concern, but also generally makes the text clearer and more precise. My plan (albeit not written down anywhere, so no reason anyone should have known this was my plan) was to follow up on those two comments when I had time and update the PR accordingly.
Oh there was still unresolved text in that long thread, sorry I missed that. I think it'd help to get some text in the PR, to make it more clear what is being discussed.
The last commit resolves all my concerns.
@joshtriplett @nikomatsakis @pnkfelix there's an FCP checkbox pending here. :)
Also Cc @rust-lang/opsem -- it can't hurt to have more eyes on this.
@joshtriplett @nikomatsakis @pnkfelix the FCP has been sitting here for quite a while -- is there still something that should be discussed?
The final comment period, with a disposition to merge, as per the review above, is now complete.
As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.
This will be merged soon.
:pushpin: Commit be075d33b6585182463327f575a7e4fd4d6faee9 has been approved by scottmcm
It is now in the queue for this repository.
Done. You can see on the diff that the squash happened successfully without modifying the PR contents.