-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Make mem::uninitialized inaccessible in future editions #98862
Comments
I'm not sure if it makes sense to tie this to an edition. If it's so broken that it needs to be removed or cause a loud warning, we should probably do that in all editions. |
We will never be able to make this hard-error in old editions. There is too much old code out there that uses it. At the same time we should do whatever we can to prevent new code that uses it from being written. I think this means it should hard error for new code. |
New editions are not just for new code, but also old. Do you know of a way to automatically migrate code away from |
There is no automatic migration possible. AFAIK cargo-fixability of code that is already warning-free is a requirement for edition changes. This function is deprecated, so warning-free code will not call it. |
No, but we can make it a deny-by-default lint. Perhaps even a future-incompatability lint, such that it becomes visible even when it occurs in dependencies.
I don't think it's right to mix up "new edition" and "new code". It's true that most new code gets written for the newest edition, and that I personally try not to think of editions as much as 'old' and 'new', but instead as just slightly different dialects that we're all maintaining in parallel. Naming them after years works nicely, but if you think of them as just "Rust Dialect A", "Rust Dialect B", etc., then it feels a bit odd to remove a function from Rust Dialect D but not from Rust Dialect C. |
I don't think this is odd at all. It's a case of "We screwed up and (lacking a time machine) the most we can do is to disallow this terrible thing in the next dialect. Bad enough that we have to deal with it in our previous dialects, let's at least contain this problem as much as we possibly can." |
What value does a hard error provide over a deny-by-default lint? People could turn off the lint, sure, but people could also switch to another edition. |
Fair. But it somewhat corresponds to "maintained code", aka code that anybody still works on. (Not all maintained code is in a new edition due to MSRV concerns, but all new edition code is necessarily maintained.) Getting rid of
It's a very clear message that we wish this thing just was not part of the language. It should not be part of the language. We should send the strongest possible signal to underpin that message. And that is making it a hard error. Editions are the best mechanism we have to un-do past mistakes. But it's not un-doing them if here are ways around it. (I very much doubt people will stick to an old edition to keep this function. That's like saying, they might just pin an old rustc. So why ever deprecate/remove anything.) |
There are many reasons to upgrade to a newer Rust version, including being able to use dependencies that require the newer Rust version. There are very few reasons to use a later Rust edition. Using Rust 2018 for your crate today has almost no downsides. |
But it will never be a hard error, as you explained. All we're discussing now is whether the syntax for disabling the lint should be |
That's not even in the same category of a thing. There is always the So no I don't buy that switching editions is remotely comparable to allowing a lint. |
No, because changing your Rust version can break things in your entire dependency tree. Many of my dependencies will not work with Rust 1.40. All of my dependencies will work fine if I switch my crate to Rust 2015 though, as Rust Edition is a decision per crate, just like enabling/disabling lints. |
Sure, pinning an old Rust has much bigger side-effects than pinning an old edition. Still to me they are much more comparable than allowing a lint. As you said, editions are basically Rust dialects. Different Rust releases are also Rust dialects. But we could and should at least make it so that the latest and greatest dialect doesn't have this thing that should never have existed.
It can be a hard error in some dialects of Rust. That's strictly better than being able (with enough allowed lints) to call this function in all dialects. |
We don't maintain Rust 1.40 or release new versions of it. We do maintain Rust 2015 and release new vesions of it.
Why? There is virtually no cost to using an older edition, which is a per crate decision that doesn't affect anything around it. If someone doesn't care how unsafe the function is and is willing to disable the lint with I'm all for adding a deny-by-default or even a forbid lint for mem::uninitialized to force maintained crates to update, using the unstoppable slowly forward moving force that is Rust version. We don't maintain old Rust versions, so there's a ton of reasons to keep moving to newer versions, otherwise you're missing out on bugfixes and new functionality. That's not the case for editions. Someone using Rust 2015 for their crate still gets all the cool new additions and bug fixes every six weeks. |
In the issue that this comes from, it looks like it was decided that even issuing a panic in the insta-UB cases is not acceptable right now. But even if users of such libraries with insta-UB Or more generally, whatever we do to ratchet up optimizations that exploit UB should come in lock-step with help to fix problems when they arise. I think it would be nice to see |
Rust versions have this "ratchet" effect: they only go up and don't go down. Dependencies all slowly raise their MSRV over time, important bugs are discovered in now unmaintained versions, cool features are added to new versions and not to older ones, etc. If you don't move with it, you get left behind. Rust editions do not have this effect. Any crate can move freely between editions, in both directions. If the older editions were only meant for unmaintained crates, we shouldn't be adding new functionality to them. But we do. Just three days ago we added What's the issue with making this a |
There is the traditional forward compat lint deprecation mechanism which is used to deprecate a lot of features. It's kinda new to have it for a standard library function like But if there is the will to get it out of the language, why not use that mechanism? Add a lint for As a second question, regarding the motivation for this. Is there any upcoming work on optimizers that's being blocked by |
If |
It has been deprecated since 1.39.0, almost exactly 3 years ago, and MaybeUninit was stable since 1.36.0. By the time the next edition rolls around,
I am not personally aware of any miscompilations reported in the wild, but there are certain sectors of the community which tend to keep to themselves even when they encounter a critical issue, much to my dismay. The IR that rustc generates depends on the type you construct. For example, if you create a |
There's #94075 (comment) which seems to have at least a fair few segfaults due to "they're making an uninit enum, so changes to the way we use enums can cause LLVM to notice the UB / something else to go wrong". At least that's what I assume is going on for at least some of them. The work there doesn't seem blocked because of this, though. That, and #52898 was a case of UB for an unused variable causing problems (but curiously, I can't reproduce that on current rustc) |
The issue of
Thanks for the concrete examples, @5225225 ! This should help quell the "this is just FUD" people. |
I do not want to drag this off topic, but I think if Rust is going to live up to its safety claims/reputation, we should work to prevent situations like this and not wait for people to report problems first. |
I view this as a two-dimensional matrix -- there is Rust 2015 version 1.40 and Rust 2018 Version 1.60 and so on. We can't change the past, and we are bound by stability guarnatees within editions, but we can make some of the entries in this matrix be the language Rust would be if we hadn't made mistakes in the past -- in particular, a language without I view this as similar to how we now require
Maybe someone slapped the attribute some place in the crate and other people elsewhere are not aware of it. Or maybe having to downgrade an entire edition gives people second-thoughts. I see a qualitative difference between allowing a lint and switching to a different language dialect.
They have this effect on each edition individually -- the "ratchet" works along the rows of the matrix, if the columns are editions. It's one ratchet per edition, so to speak. So we should use the ratchet where we can, i.e. the edition 2024 row, even if we cannot use it in the edition 2015 row.
I am not aware of anything that is actively blocked. However, LLVM now has the |
I would hope we don't have to "quell" anyone, as that is a sign of fundamental disagreement deeper than anything we can reasonably expect to deal with merely through discourse. Never mind "in the context of a GitHub issue". Not that I did not grok your meaning otherwise. Hard examples are quite useful.
I would, in fact, generically encourage everyone to revisit that document and read it entirely rather than rehashing all the points of discussion that lead to it. It is a complex issue that requires thought and nuance, and Rust has already made very specifically-shaped commitments here. Some of those commitments diverge from what we commonly remember them as (including my own memory, to be honest). |
There is not, technically, a soundness bug here -- Due to that I doubt that we will get consensus for removing I think there is a gradient between "this is so critical that we have to reject it in all editions" and "this is 'fine', regular linting is enough". I think (The maintenance burden argument from the RFC does not apply, since we are not talking about compiler features here, but about library APIs. So there is no issue of ending up with a a combinatorial explosion of different type checkers, for instance.) |
Why don't we make them write both? Deny by default (or future incompat, or whatever) on old editions and hard error in new editions? |
Note:
or
and it really doesn't matter which. The latter error is even significantly more useful, and more what I think Ralf and Josh had in mind. So here's my concrete proposal: Add a new attribute, Ideally, this would be a unique lint per item which has the |
Well, in editions >= 20XX, it should be a hard error. Since you can publish non-building crates to crates.io, and a forbid by default doesn't stop a crate being used as a dependency because of cap-lints. Not that I suspect that's likely to happen, but there's no reason not to. a future compat warning for < 20XX code (probably only for types that we know are UB, don't emit it for But we're currently in the situation where we want code that currently uses, say old crossbeam to panic, but we're not actually telling people that it will, so they need to either use rustsec's audit, or just upgrade for unrelated reasons. |
To be clear, this is proposed as an alternative to removing the function, correct? This is merely an improvement in performance over the current state of things, where IIUC Additionally, achieving a "frozen" arbitrary-but-consistent value wasn't something that LLVM even supported until somewhat recently. When it's available, it may be a reasonable choice, but it shouldn't change any of the cases which panic, because those would still be immediately UB. |
I don't think we should use Also, the 0x01 filling was picked because uses of |
I remember two of my small projects: one using Tokio 0.1 and the other using old version of pest to be buildable (with big compat warnings about So I assume that somebody who developed some private high-performance networking tool 5 years ago, then left it behind (or just used a pre-built executable year after year), and now trying to revive it may be unpleasantly surprised. |
Assuming that is the 0.1 branch of Here's a replication of that layout. So, doesn't look like we have a good way to not make old tokio panic -- sorry. :/ |
Can Imagine some Rust newbie tasked to revive old project with a |
Something basically exactly along those lines (but more general, just saying "you depend on a crate that uses mem::uninitialized") was implemented in #100342 but we're still deciding on the exact situations that it should warn about. I don't think hard-coding specific crates are a good idea, since then it might feel like the compiler is targeting specific crates and not others. That, and it won't lint about the less common crates, or crates we don't know about (internal dependencies in companies?) |
tbf, the futures crate is a semiofficial rust-lang organization crate, so it may be a reasonable target for special behavior ( If they don't already have one, someone should add a RustSec advisory with that information, which random bad ideaIn the case where we know it's UB, and we know there's a This is I'm sure very difficult due to the layering of the compiler, but at least theoretically possible. Unfortunately, it doesn't really solve the problem for all types, and most problem cases probably don't have a |
In the past people (including myself) have suggested the we could emit a valid but unspecified bit pattern for any type. That would keep old code "working" for sure, but
|
Ah, I was about to suggest if futures is a semi-official crate maybe we can get an 0.1.x point release, but seems like that would not help. |
Maybe |
I'm in favour of allowing editions to remove things that are known to be broken. Despite Rust's stability guarantees (which are a wonderful thing!), I don't think it's realistic for there to never be a "we screwed up big time" escape hatch, and I think it's good for Rust to "encourage"/enforce the use of safe replacements, even if it risks some churn/frustration. It also means that if someone sees (e.g. when auditing dependencies) a crate is written against edition 2024 (or any future relevant edition), it means it cannot (or at the very least is very unlikely to) use certain broken features, which is nice. A good example of this are the efforts to either deprecate or restrict |
There has also been some discussion in #100342 which proposed adding a forward compat warning. I want to quote @Nilstrieb 's comment:
That is about FCWs and not edition lints/errors. How much does the |
No, it would require reading uninitialized memory, which is UB. |
Quite a bit. Some callers still trip into UB because they use this function inside a generic and in an instantiation the
Probably. You're asking a bunch of technically-open opsem questions, including whether we have validity behind references. The only catch is that you can't test that in Miri, because Miri disables the (note that I'm trying to answer the question you're trying to ask instead of the literal text, that program in question simply panics because there is an assertion in |
It's still language UB since the
Where does this read uninitialized memory?
This creates a dangling reference; I don't think there are open questions around whether this is language UB even with the mitigated |
Of course, I forgot about that 🤦 . In practical terms, I think the I would advocate for removing this function for two reasons. The mitigation makes it no longer match the documentation, which clearly says
We've already effectively removed this function; the implementation doesn't match the documentation anymore. There are also a few cases where people have used this function and |
@rustbot labels I-lang-nominated We discussed this in the edition call today. Let's nominate this on the lang side to discuss whether or not this is something that we wanted to see happen. |
@rustbot labels -I-lang-nominated -A-edition-2024 A-maybe-future-edition We discussed this in the lang meeting this week. The consensus was that we'd be open to a lint-based approach. We didn't feel that this needed to happen before Rust 2024, and we were open to the idea that perhaps the severity of such a lint could be tied to Rust 2024 even after it ships. |
We have reached the end of what we can do in #66151 (making
mem::uninitialized
panic when it is used incorrectly) without enormous amounts of crater failures. However, there is still one more thing we can do: we can makemem::uninitialized()
inaccessible in future editions. We seem to have the support of at least one lang-team member as well. :) Cc @joshtriplett@bstrie mentioned they have a patch doing this, though the RFC got closed. Reading the Zulip discussion, it looks like some libs team members were concerned, though the concern seems to have been mostly around hiding things from the docs. I am not talking about the docs here, I just want edition 2024 code to not compile when it calls
mem::uninitialized
. @m-ou-se I wonder if you would be fine with that?The text was updated successfully, but these errors were encountered: