Uh oh!
There was an error while loading. Please reload this page.
Ensure that evaluating or validating a constant never reads from a static - #67337
Conversation
This comment has been minimized.
This comment has been minimized.
oli-obk
commented
Dec 16, 2019
Oh I just realized this PR makes no sense. If the user points into the static, this PR won't detect that. We could poison |
This comment has been minimized.
This comment has been minimized.
RalfJung
commented
Dec 16, 2019
So if I recall correctly, in rust-lang/const-eval#17 (comment) and https://github.com/rust-lang/const-eval/blob/master/const.md#reading-statics, I also came to the conclusion that we want to guard accesses during execution, and not the final value. |
0f986ca to
c0faa51CompareRalfJung
commented
Dec 18, 2019
That comment talks about not reading from (mutable) statics. I don't understand the connection to the final value not containing certain relocations.
Also, isn't code like this allowed? staticFOO:i32 = 13;constFOOPTR:&i32 = &FOO;So, I don't entirely understand how we even could backwards-compatibly forbid consts pointing to statics, and why we would want that? |
Uh oh!
There was an error while loading. Please reload this page.
oli-obk
commented
Dec 18, 2019
No: |
oli-obk
commented
Dec 18, 2019
Hmm... I guess I went overboard when looking at it from the perspective of the comment, but just referencing a static is not allowed on stable, so this is what we want to forbid right now. |
bors
commented
Dec 21, 2019
☔ The latest upstream changes (presumably #67485) made this pull request unmergeable. Please resolve the merge conflicts. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
RalfJung
commented
Dec 22, 2019
Well, what this PR implement is not accessing a static. So the PR description is wrong, IMO. Except that, somehow, this also excludes referencing a static, and I am not sure why. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
c0faa51 to
fc0c48cComparebors
commented
Dec 23, 2019
☔ The latest upstream changes (presumably #67540) made this pull request unmergeable. Please resolve the merge conflicts. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
and some uses of it will be illegal forever (e.g. mutable or interior mutable statics)
Co-Authored-By: Ralf Jung <post@ralfj.de>
3168051 to
89250b9Comparerust-highfive
commented
Dec 23, 2019
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
RalfJung
commented
Dec 23, 2019
Looking good! r=me with tests blessed. |
oli-obk
commented
Dec 23, 2019
@bors r=RalfJung |
bors
commented
Dec 23, 2019
📌 Commit 87fea04 has been approved by |
bors
commented
Dec 23, 2019
🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened |
…t, r=RalfJung Ensure that evaluating or validating a constant never reads from a static r? @RalfJung as per rust-lang#66302 (comment) This does not yet address the fact that evaluation of a constant can read from a static (under unleash-miri)
Rollup of 7 pull requests Successful merges: - #67337 (Ensure that evaluating or validating a constant never reads from a static) - #67543 (Add regression tests for fixed ICEs) - #67547 (Cleanup err codes) - #67551 (Add long error code explanation message for E0627) - #67561 (remove `description` from `Error` impls in docs) - #67569 (Clean up unsafety in char::encode_utf8) - #67572 (Use the chocolatey CDN directly to avoid the flaky API) Failed merges: r? @ghost
r? @RalfJung
as per #66302 (comment)
This does not yet address the fact that evaluation of a constant can read from a static (under unleash-miri)