[1.95.0] Document provenance and its use in const eval#707
[1.95.0] Document provenance and its use in const eval#707kirtchev-adacore wants to merge 3 commits into
Conversation
812a13f to
ecca0f0
Compare
ecca0f0 to
21bb6de
Compare
| well-formed indirection value | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| A :dt:`well-formed indirection value` is an :t:`indirection value` with :t:`provenance`, where all bytes that comprise the :t:`indirection value` are part of the same group. |
There was a problem hiding this comment.
the definition feels incomplete, but I don't know yet how to fill it... maybe inspiration can be taken from a review from my old pr
There was a problem hiding this comment.
Maybe
... bytes that comprise the :t:`indirection value` are initialized and are part
of the same group.Basically we want to say that an indirection value does not (somehow) contain the bytes of something else.
There was a problem hiding this comment.
Right.
I think what the Reference PRs and core::ptr are trying to say is that you can obtain a value by doing sketchy things, such as memory transmutation. If you plan on using that value as an indirection value with provenance, then the value better have "correct bytes", what 2091 calls "bytes must be fragments of the same original pointer value in the correct order" and 2138 calls "part of an adjacent group of correctly-ordered bytes that form an entire pointer".
There was a problem hiding this comment.
I updated the definition of "well-formed indirection value".
There was a problem hiding this comment.
I updated the PR with the terminology suggested by Ralph.
There was a problem hiding this comment.
The update to pointers makes sense along the lines of what Ralf suggested, cool.
I see two possible gaps remaining in the definition (details and rustc examples on the src/values.rst thread for fls_ffh8mAkebORJ):
- It still requires provenance, so provenance-free pointer constants that rustc accepts (
core::ptr::null(),5 as *const u8) are still ruled out. The fragment restriction only concerns bytes that carry provenance. - "Fragments of the same :t:
original pointer" over-constrains the source: fragments of a derived pointer (e.g. a cast) reassemble fine; the compiler only needs the bytes to rebuild a single pointer value with provenance.
For this entry:
| A :dt:`well-formed indirection value` is an :t:`indirection value` with :t:`provenance`, where all bytes that comprise the :t:`indirection value` are part of the same group. | |
| A :dt:`well-formed pointer` is a :t:`pointer` where either no byte of the | |
| :t:`pointer` carries :t:`provenance`, or every byte of the :t:`pointer` is the | |
| corresponding byte of a single :t:`pointer` with :t:`provenance`. |
("initialized" is fine to keep or drop — it's already implied by fls_6lg0oaaopc26.)
There was a problem hiding this comment.
Incorporated.
21bb6de to
e42d104
Compare
The information has been taken from - [core::ptr Provenance](https://doc.rust-lang.org/core/ptr/index.html#provenance) - [reference#2091](rust-lang/reference#2091) - [reference#2139](rust-lang/reference#2138)
e42d104 to
1844443
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
1844443 to
e23ca4b
Compare
e23ca4b to
a8393ef
Compare
There was a problem hiding this comment.
Hey @kirtchev-adacore, thanks for taking the time to write this up.
I spotted some things we could tune up here and there for precision and to match current understandings for provenance that seem to exist in the Project.
I know the release is tomorrow and we meet Friday, but hopefully this is not too much 😅
| A :t:`derived pointer` inherits the :t:`provenance` of the :t:`pointer` it was created from, if any. | ||
|
|
||
| :dp:`fls_ffh8mAkebORJ` | ||
| A :t:`well-formed pointer` is a :t:`pointer` with :t:`provenance`, where all bytes that comprise the :t:`pointer` are initialized, correctly ordered, and are fragments of the same :t:`original pointer`. |
There was a problem hiding this comment.
The pointer terminology reads cleaner than "indirection value" since it now matches "how the Project talks about Rust".
On the definition itself, three things seem to remain, one of which is the big one.
-
The definition still opens with "a pointer with provenance", and the const/static rules require well-formedness, so provenance-free pointer constants are still ruled illegal even though rustc accepts both and neither carries provenance. The fragment restriction from const-eval: full support for pointer fragments rust#144081 / const-eval: fix and re-enable pointer fragment support rust#148259 applies to bytes that carry provenance; pointers whose bytes carry none are fine.
const P: *const u8 = 5 as *const u8; const N: *const u8 = core::ptr::null();
-
"Fragments of the same :t:
original pointer" over-constrains the source. The fragment mechanism reassembles a pointer value, whatever its derivation — nothing requires it to be original in this PR's sense ("created via allocation"). Concretely,const P: *const i32 = &1 as *const i32;is accepted, and the value's bytes come from a derived (cast) pointer. "A single pointer with provenance" is the condition that matches the compiler. -
"Correctly ordered" doesn't quite pin position and completeness, since an ordered-but-offset subset of a larger pointer's bytes could be read as satisfying it. "Every byte is the corresponding byte" helps close that.
Putting those together, for the chapter paragraph:
| A :t:`well-formed pointer` is a :t:`pointer` with :t:`provenance`, where all bytes that comprise the :t:`pointer` are initialized, correctly ordered, and are fragments of the same :t:`original pointer`. | |
| A :t:`well-formed pointer` is a :t:`pointer` where either no byte of the | |
| :t:`pointer` carries :t:`provenance`, or every byte of the :t:`pointer` is the | |
| corresponding byte of a single :t:`pointer` with :t:`provenance`. |
(and the same wording with :dt: on the defined term for the glossary entry).
Regarding "initialized": to me this seems fine either way. It's already implied as creating any pointer value from uninitialized memory is UB via fls_6lg0oaaopc26, and its union exception can't apply to a raw pointer or reference. So we could keep "initialized" for emphasis or dropped for brevity without changing what's legal.
There was a problem hiding this comment.
I like it, incorporated!
| :t:`constant initializer`. | ||
|
|
||
| :dp:`fls_l1FOH8zt0XRZ` | ||
| If the :t:`value` produced by evaluating a :t:`constant initializer` is a :t:`pointer`, then the :t:`pointer` shall be a :t:`well-formed pointer`. |
There was a problem hiding this comment.
"If the value ... is a :t:pointer" only binds when the constant's value is a pointer at top level. The compiler's validation of final values recurses through the whole value — the fragment check lands there (see the interning/validation discussion in rust-lang/rust#144081 and rust-lang/rust#148259) — so pointers inside aggregates are checked too. These are in scope for the compiler but not covered by the current wording:
const C: (u32, &u32) = (0, &1);
const D: [*const u8; 2] = [&2u8, &3u8];Suggest quantifying over contained pointers. For the constant rule:
| If the :t:`value` produced by evaluating a :t:`constant initializer` is a :t:`pointer`, then the :t:`pointer` shall be a :t:`well-formed pointer`. | |
| If the :t:`value` produced by evaluating a :t:`constant initializer` is or | |
| contains a :t:`pointer`, then each such :t:`pointer` shall be a | |
| :t:`well-formed pointer`. |
There was a problem hiding this comment.
I wan't aware of the compiler behavior, incorporated!
| The :t:`static` is determined by evaluating its :t:`static initializer`. | ||
|
|
||
| :dp:`fls_37oocZVDne5Y` | ||
| If the :t:`value` produced by evaluating a :t:`static initializer` is a :t:`pointer`, then the :t:`pointer` shall be a :t:`well-formed pointer`. |
There was a problem hiding this comment.
The same aggregate-coverage issue applies to fls_37oocZVDne5Y: the compiler validates final values recursively, so pointers inside aggregates are checked too. Suggest quantifying over contained pointers for the static rule:
| If the :t:`value` produced by evaluating a :t:`static initializer` is a :t:`pointer`, then the :t:`pointer` shall be a :t:`well-formed pointer`. | |
| If the :t:`value` produced by evaluating a :t:`static initializer` is or | |
| contains a :t:`pointer`, then each such :t:`pointer` shall be a | |
| :t:`well-formed pointer`. |
There was a problem hiding this comment.
Incorporated.
| It is undefined behavior to access memory through a :t:`pointer` which does not have :t:`provenance` over that memory. | ||
|
|
||
| :dp:`fls_MwDoxVPRZCqm` | ||
| It is undefined behavior to offset a :t:`pointer` beyond the bounds of the memory block it has :t:`provenance` over. |
There was a problem hiding this comment.
Hrm, I can foresee two problems possibly. Right fix here might be deletion?
-
This does not hold for wrapping arithmetic.
core::ptris explicit thatwrapping_offset/wrapping_addmay leave the allocation with no UB; only the in-bounds family (offset,add,sub) carries the bounds precondition, and even there one-past-the-end is allowed. Neither "across" nor "beyond" fixes that underlying issue. -
Also, documenting this looks to me to be outside the FLS's intended purview, and the FLS has said so before regarding library specifics. The in-bounds requirement is a contract of those library functions (it remains UB to violate it — the question is only where that gets documented), and the changelog has twice recorded the decision not to document exactly this class of rule: offset_from: always allow pointers to point to the same address rust#124921 ("offset_from: always allow pointers to point to the same address") and offset: allow zero-byte offset on arbitrary pointers rust#117329 ("Allow zero-byte offsets...") are both annotated "This lifted restriction was not documented in the FLS". The language itself has no raw-pointer arithmetic operators to attach the rule to (place projections are specified separately, in the expressions chapter).
The access rule (fls_c3DaCLQEBpYQ) already makes any non-zero-sized memory access through an out-of-bounds pointer UB, which is the part the FLS needs to state here. (Zero-sized accesses need no provenance per core::ptr, which is another reason to keep the bounds rule out of a blanket UB statement.)
There was a problem hiding this comment.
Indeed, the FLS wording would need to depend on details from core. So I agree, the rule should be removed.
Removed.
| .. rubric:: Legality Rules | ||
|
|
||
| :dp:`fls_5MkKtNL9oCsL` | ||
| :t:`Provenance` is an optional property of :t:`[pointer]s` that restricts the addresses a :t:`pointer` may point to, the timespan during which the :t:`pointer` may point to those addresses, and whether the :t:`pointer` can read from and write to those addresses. |
There was a problem hiding this comment.
In the case of this text "restricts the addresses a pointer may point to", there may be some imprecision: provenance itself doesn't restrict the stored address. A pointer may hold any address (that's precisely
why out-of-bounds wrapping_add is legal). Individual operations impose their own validity requirements, but what provenance restricts is access. RFC 3559: "The pointer's provenance says where and when the pointer is allowed to access memory."
| :t:`Provenance` is an optional property of :t:`[pointer]s` that restricts the addresses a :t:`pointer` may point to, the timespan during which the :t:`pointer` may point to those addresses, and whether the :t:`pointer` can read from and write to those addresses. | |
| :t:`Provenance` is an optional property of :t:`[pointer]s` that restricts the | |
| memory locations the :t:`pointer` may access, the timespan during which the | |
| accesses may occur, and whether the accesses may read from or write to memory. |
I like this updated triad (spatial/temporal/mutability) since it now matches core::ptr, stated about access, and "memory locations" matches the wording the PR now uses for :t:pointer type ("values indicate memory locations").
There was a problem hiding this comment.
I like it, incorporated!
| :t:`Provenance` is an optional property of :t:`[pointer]s` that restricts the addresses a :t:`pointer` may point to, the timespan during which the :t:`pointer` may point to those addresses, and whether the :t:`pointer` can read from and write to those addresses. | ||
|
|
||
| :dp:`fls_1NJhTBN1D2qv` | ||
| An :t:`original pointer` always carries :t:`provenance`. |
There was a problem hiding this comment.
Also might be worth tightening fls_1NJhTBN1D2qv from "always carries provenance" to say over what:
| An :t:`original pointer` always carries :t:`provenance`. | |
| An :t:`original pointer` carries :t:`provenance` over all or part of the | |
| :t:`allocated object` it was created from. |
"All or part" because core::ptr only commits to "at least the range that slice describes" for references and notes that whether provenance gets shrunk to fit is not yet determined — the FLS shouldn't promise more extent than upstream does.
There was a problem hiding this comment.
Incorporated.
| .. rubric:: Legality Rules | ||
|
|
||
| :dp:`fls_3qI8FXMsyk0f` | ||
| A :t:`pointer type` is a :t:`type` whose :t:`[value]s` indicate memory locations. |
There was a problem hiding this comment.
Reading through here I think I spotted one scope problem and one naming collision in the new pointer type definition, both fixable with the same edit:
-
Scope. "A type whose values indicate memory locations" describes function pointer types too — a function pointer's value indicates where code lives, which is exactly why
indirection type("a type whose values refer to memory locations") includes them. So as defined, function pointers arepointers, and the provenance rules, the well-formed pointer requirement on const/static values, and the access/offset UB rules all textually apply to them. But whether function pointers have provenance at all is an open question upstream — rust-lang/unsafe-code-guidelines#340, the issue bjorn3 pointed to on Zulip ("References have provenance. I believe it hasn't been decided yet if function pointers have provenance"). The FLS shouldn't answer that question as a side effect of a definition's wording. -
Duplication.
pointer typeand the pre-existingindirection typenow coexist with definitions that don't distinguish them ("indicate memory locations" vs "refer to memory locations"). Two terms, one meaning, in the same section.
Defining pointer type by enumeration fixes both at once, and scopes the term to exactly the two cases that have been decided — references have provenance, and raw pointers uncontroversially do:
| A :t:`pointer type` is a :t:`type` whose :t:`[value]s` indicate memory locations. | |
| A :t:`pointer type` is either a :t:`raw pointer type` or a :t:`reference | |
| type`. |
(same wording with :dt: for the glossary entry).
That keeps the provenance rules from taking a position UCG#340 hasn't, and gives pointer type a
meaning indirection type doesn't already have. It does leave function-pointer values in const/static final values unconstrained by the FLS — deliberate underspecification until UCG#340 resolves, and worth one line on the record (maybe as an issue we open?) so the gap reads as chosen rather than missed.
There was a problem hiding this comment.
Incorporated.
|
|
||
| .. _fls_Dqk4eIvxHloY: | ||
|
|
||
| Provenance |
There was a problem hiding this comment.
I had some readability rather than correctness thoughts on this part.
While forward references are legal in the FLS, this PR splits the material three ways: the definitions of pointer / original pointer / derived pointer sit in the chapter intro, the provenance rules that give them meaning live in a section after Constant Promotion, and the Constants/Statics legality rules reference well-formed pointer, defined at the end of the chapter.
Perhaps we can consolidate all the provenance definitions and rules into the Provenance section and then place that section immediately after the chapter intro, before Constants. Everything then reads top-down, and it keeps the intro focused on the value / allocated-object / overlap fundamentals.
There was a problem hiding this comment.
I like it! (I actually read the first two paragraphs of your message, then wrote up basically what you said in the third paragraphs 😆 It pays to read everything!)
| well-formed indirection value | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| A :dt:`well-formed indirection value` is an :t:`indirection value` with :t:`provenance`, where all bytes that comprise the :t:`indirection value` are part of the same group. |
There was a problem hiding this comment.
The update to pointers makes sense along the lines of what Ralf suggested, cool.
I see two possible gaps remaining in the definition (details and rustc examples on the src/values.rst thread for fls_ffh8mAkebORJ):
- It still requires provenance, so provenance-free pointer constants that rustc accepts (
core::ptr::null(),5 as *const u8) are still ruled out. The fragment restriction only concerns bytes that carry provenance. - "Fragments of the same :t:
original pointer" over-constrains the source: fragments of a derived pointer (e.g. a cast) reassemble fine; the compiler only needs the bytes to rebuild a single pointer value with provenance.
For this entry:
| A :dt:`well-formed indirection value` is an :t:`indirection value` with :t:`provenance`, where all bytes that comprise the :t:`indirection value` are part of the same group. | |
| A :dt:`well-formed pointer` is a :t:`pointer` where either no byte of the | |
| :t:`pointer` carries :t:`provenance`, or every byte of the :t:`pointer` is the | |
| corresponding byte of a single :t:`pointer` with :t:`provenance`. |
("initialized" is fine to keep or drop — it's already implied by fls_6lg0oaaopc26.)
| :t:`constant initializer`. | ||
|
|
||
| :dp:`fls_l1FOH8zt0XRZ` | ||
| If the :t:`value` produced by evaluating a :t:`constant initializer` is a :t:`pointer`, then the :t:`pointer` shall be a :t:`well-formed pointer`. |
There was a problem hiding this comment.
This is not intended to be addressed in #707, but it's something I noticed we could do a follow-up on. If this looks reasonable I can go open an issue for it. Lemme know.
The well-formedness rules being added in #707 cover the pointer-fragment restriction from rust-lang/rust#148259, but rustc enforces a second condition on final values that the FLS won't yet capture: a provenance-carrying pointer in the final value must not point to an allocation freed during evaluation.
const P: *const u8 = { let x = 0u8; &raw const x };Current rustc rejects this with "encountered dangling pointer in final value of constant". Note that it seems this predates rust-lang/rust#148259.
Scope note: the condition attaches to pointers in the final value. Allocations created and freed during evaluation are fine as long as no provenance-carrying pointer to them survives into the final value.
Note the existing FLS dangling term can't be reused as-is: it counts null as dangling, while const N: *const u8 = core::ptr::null(); is a legal constant. A liveness-only condition (the provenance's allocated object still exists at the end of evaluation) would need its own wording.
There was a problem hiding this comment.
How about adding "non-dangling" to the definition of "well-formed pointer"?
A :dt:`well-formed pointer` is a non-dangling :t:`pointer` where either no byte
of the :t:`pointer` carries :t:`provenance`, or every byte of the :t:`pointer`
is the corresponding byte of a single :t:`pointer` with :t:`provenance`.1f1b867 to
e1d4a62
Compare
The information has been taken from