Skip to content

1.24.1 announcement #230

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

Merged
merged 5 commits into from
Mar 2, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 14 additions & 19 deletions _posts/2018-03-01-Rust-1.24.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ appropriate page on our website, and check out the [detailed release notes for

## What's in 1.24.1 stable

As you know, we tend not to release point releases. In this case, Several minor
regressions were found in 1.24.0 which collectively merited a release.
Several minor regressions were found in 1.24.0 which collectively merited a
release.

A quick summary of the changes: there are four.
A quick summary of the changes:

* Do not abort when unwinding through FFI (this reverts behavior from 1.24.0)
* Do not abort when unwinding through FFI (this reverts behavior added in 1.24.0)
* Emit UTF-16 files for linker arguments on Windows
* Make the error index generator work again
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can cut this that'd be great, it's really only relevant for distros AFAIK.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it felt really weird to me with so few changes to leave one out

* Cargo will warn on Windows 7 if an update is needed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These last two are less immediately obvious than the first two. Is it worth linking to the respective issues here, even though they're explained below?

Expand All @@ -41,9 +41,11 @@ With that, let's dig into the details!

### Do not abort when unwinding through FFI

TL;DR: the behavior in 1.24.0 broke the `rlua` crate, and is being reverted. If
you have since changed your code to take advantage of the behavior in 1.24.0,
you'll need to revert it for now.
TL;DR: the new behavior in 1.24.0 broke the `rlua` crate, and is being
reverted. If you have since changed your code to take advantage of the behavior
in 1.24.0, you'll need to revert it for now. While we still plan to introduce
this behavior eventually, we will be rolling it out more slowly and with a new
implementation strategy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, better. 👍


Quoting [the 1.24 annoucement](https://blog.rust-lang.org/2018/02/15/Rust-1.24.html):

Expand All @@ -64,16 +66,9 @@ extern "C" fn panic_in_ffi() {
>
> In Rust 1.24, this code will now abort instead of producing undefined behavior.

As implemented, this functionality broke some users, notably, integration with
the Lua language. We have reverted this behavior, and 1.24.1 will reintroduce
the undefined behavior. Fundamentally, the aborting behavior we originally
rolled out is a soundness fix, and we still plan on reintroducing it, but are
going to be rolling it out a bit more slowly, and with a different
implementation.

It started with [a bug filed against the `rlua`
crate](https://github.com/chucklefish/rlua/issues/71). `rlua` is a package
that provides high level bindings between Rust and the [Lua programming
As mentioned above, this caused breakage. It started with [a bug filed against
the `rlua` crate](https://github.com/chucklefish/rlua/issues/71). `rlua` is a
package that provides high level bindings between Rust and the [Lua programming
language](https://www.lua.org/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably worth explicitly acknowledging here that the rlua implementation depends on undefined behavior that just happens to work on all known OSes as of Rust 1.23. (That is correct, right?)

Copy link

@kyren kyren Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is probably worth explicitly acknowledging here that the rlua implementation depends on undefined behavior that just happens to work on all known OSes as of Rust 1.23. (That is correct, right?)

I'm going to attempt to explain the situation so that if I get something wrong somebody knowledgeable will correct me, because I really want to know the precise answer as well!

I think the question of whether what I was doing was / is UB is entirely a question of Rust specification. As far as I am aware, there wasn't much documentation or official decisions on whether or not a longjmp as implemented on unixes was something that should work across Rust frames, but it was clear that if the Rust frame being jumped over was "trivial", this "should not" cause any issues. The problem only arose from a combination of two factors, which is that longjmp on windows was implemented using SEH and I was not aware of this, and Rust's addition of automatic exception handling on extern fn boundaries.

Now, I think it could be argued that since there was definite documentation on unwinding through rust being disallowed, and SEH is certainly the sort of unwinding that was being referred to, rlua with rust 1.23.0 on windows was relying on UB that just happened to work. You could possibly also argue about the precise definition of "unwinding" and say that rlua was relying on UB categorically, I'm honestly not sure whether unwinding refers only to things like exception handling or also to simple stack manipulation. I think there was consensus in the bug report, though, that disallowing interacting with C APIs that longjmp is not great for Rust's C interop story, so it SEEMED that the decision was made that longjmp is something that SHOULD be supported, at least in the precise limited way that rlua was trying to use it, because otherwise there are quite a few C APIs that just become extremely painful to use, and would REQUIRE extremely complex C wrapper APIs. There was also talk about supporting interop with exceptions and unwinding more generally going forward, and doing a better job of checking conditions such as "nothing that implements Drop is on the stack", but obviously that wasn't going to be part of the current fix.

So, the end result is that.. you could argue that rlua was relying on UB or at least unspecified behavior but is now NOT relying on UB, simply because it was decided that it shouldn't be UB. It's kind of a messy question!

If I've gotten any of that wrong let me know, because I don't want to misrepresent the intentions of the people who ACTUALLY make these decisions.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm being verrrry hand wavey with the previous explanation, and if you feel skeptical about my reasoning, I don't blame you.

We were ultimately aware of how weird and potentially bad longjmping over Rust was, and rlua had an old issue with a pretty long discussion about potential fixes in it. The problem though, ultimately, was that the solutions were just really bad, and not being able to do this makes Rust / C interop feel very limited.

I hesitate even still to call the rust behavior a "bug" per se, it felt to me like the bug report was more a process to discuss an edge case of C interop, and decide what Rust should do going forward.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyren

I would assert that UB is always a matter of language specification! (C++ folks will get touchy on this point; anything that is specified by the OS is merely unspecified, not undefined.)

Since Rust didn't provide a guarantee of a particular behavior, I think I would indeed consider the situation (as I understand it) UB. But I also wouldn't necessarily consider that a "bug", because UB just means that the Rust language doesn't impose a behavioral requirement. In this case, it seems that cross-platform behavior was consistent and correct, even without a formal specification of the desired or actual behavior.

I would also say that, regardless of whether we call this "UB" or not, the ideal path would be to make Rust guarantee that this use-case will work in the future, because it seems clear that it's a valuable behavior. And, yes, that would be equivalent to saying that in 1.24.0 and earlier, it was UB, even though it works correctly in 1.23 and earlier; and that in 1.24.1+ it won't be, "simply because it was decided that it shouldn't be UB."

Copy link

@kyren kyren Mar 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, okay we're on the same page then. I've had this conversation a few times, and sometimes when people say UB they mean it as a synonym for "definitely should not do it" or "the specification explicitly states that it is UB" or simply "you have a bug" and not "the specification does not explicitly allow it or does not address it yet". I was attempting to draw a distinction between "unspecified" and "undefined" that lumped in decisions yet to be made into "unspecified", which I guess is not really accurate. I think in C++ parlance they sometimes use the words unspecified or "platform specified" to delineate between behavior that they have actively decided compilers are allowed to do whatever they want, and behavior that is simply not part of the specification.

Communicating and reasoning about this is really hard, man!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kyren Yeah, I think we are on the same page. I didn't want to get too much into my personal gripes with the state of affairs in C and C++, but suffice to say that I think the attitude that "if you have UB you deserve whatever bad thing happens to you" is unhelpful at best. I have also come to believe that it's somewhat disingenuous and ahistorical; supercat's comments here were particularly enlightening to me. (They commented on almost every answer; their comments are, oddly, more valuable to me than their answer.)

You're right that "undefined" and "unspecified" are different, especially in C++ world. But the C++ standard explicitly states that "compilers are allowed to do whatever they want" and "behavior that is simply not part of the specification" is generally the same thing unless explicitly stated otherwise. I think that's...not a great approach, especially when the committee has little to no interest or incentive to reduce the amount of undefined behavior in the language over time.

In Rust things really are a little fuzzier, though, since we don't have a formal language specification the way C++ does. Then again, C++ got by without one for over a decade, which the UB zealots (sorry, can't help myself) conveniently tend to forget...


> Side note: `rlua` is maintained by [Chucklefish](https://chucklefish.org/),
Expand Down Expand Up @@ -187,8 +182,8 @@ we may backport, otherwise, this functionality will be back in 1.26.

### Emit UTF-16 files for linker arguments on Windows

TL;DR: `rustc` stopped working for some Windows users. If it's been working for you,
you were not affected by this bug.
TL;DR: `rustc` stopped working for some Windows users in edge-case situations.
If it's been working for you, you were not affected by this bug.

In constrast with the previous bug, which is very complex and tough to understand,
this bug's impact is simple: if you have non-ASCII paths in the directory where
Expand Down