Stream: project-ffi-unwind

Topic: UB to skip destructors


view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:00):

@nikomatsakis I'm still not quite clear on the reason why it's UB to skip destructors with longjmp. You wrote:

In other words, it's true that unsafe code can actively cause destructors not to run, either by using mem::forget or by creating a Rc cycle.

longjmp is an "active" choice, too, and it requires unsafe. Unlike mem::forget, it has a non-local influence, but I think it's true in general of unsafe code. So the question of "is it UB" doesn't seem to be one of "should we break rayon?" but rather "is it the programmer's responsibility to avoid using rayon and longjmp in the same thread?" I believe the reasoning is the same for pthread_exit.

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:20):

I think I partially agree with that framing

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:20):

It's true that I was conflating two different definitions of UB

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:20):

i.e., there is a notion of "minimal UB" that must be avoided

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:20):

but there is also a definition of "what can unsafe code assume other code won't do"

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:20):

it's probably best not to think of that a single set

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:21):

but still

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:21):

if rayon is going to call itself safe

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:21):

that is implicitly quantified by some definition of limits on unsafe code

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:21):

and violating those limits is what I am calling "UB"

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:21):

I'm not sure if there's an alternative term that e.g. the unsafe code guidelines has settled on

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:22):

relevant blog post

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:22):

I don't think the answer of "it's the programmer's responsibility" is good enough, in other words, but it's also got truth in it

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:23):

in any case, the locality is the key bit -

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:23):

mem::forget can't cause somebody else's destructors not to run

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:23):

but longjmp can

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:24):

and you aren't really allowed to assume that other frames don't have dtors

view this post on Zulip nikomatsakis (Jun 09 2020 at 22:24):

in particular frames can add them as a minor semver bump :)

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:29):

hmmmmmm

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:29):

that is true

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:29):

I am reminded of our discussion about "LLVM UB" versus other "UB" meanings

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:33):

And, as always, I think of "UB" as "the compiler has free reign to optimize in ways that break code"; so in this case, I am wondering, do we _want_ to give the compiler extra optimization potential by assuming that longjmp can never cross non-POFs?

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:38):

I guess the distinction I'm trying to draw is between "coding such a thing would be inherently broken, regardless of the compiler's optimizations" and "this code could hypothetically behave as you'd expect if the compiler doesn't optimize aggressively." Clearly there are instances where longjmp over non-POFs falls into the former category, but it seems like many or even most cases would fall into the latter.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:39):

Obviously such a distinction is useless (or actively harmful) when writing code in a language, but I think it might be a helpful distinction when deciding whether a particular construct should be formally UB in the design of the language.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:40):

I think much of Chandler Carruth's point here is that more UB than people realize falls into the first category rather than the second: https://youtu.be/yG1OZ69H_-o

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 22:40):

.....but I think it's pretty clear that C++ is also full of UB in the second category.

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:31):

So I don't have time for a lengthy reply --

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:31):

but I just thought of one thing, and I'm going to cc @RalfJ on this point,

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:32):

if we say that a POF frame can be longjmp'd

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:32):

it occurs to me that if you have

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:32):

fn foo(x: &mut u32) {
    *x += 1;
    bar();
}

then we can't move that *x += 1 down below bar()

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:32):

I guess that may never have been something we planned to do anyway

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:32):

I think mostly we've thought about pushing reads down below

view this post on Zulip nikomatsakis (Jun 09 2020 at 23:33):

but I'm not 100% sure about that, @RalfJ would remember

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 23:37):

Hmmmm.... that's a really interesting point.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 23:38):

That definitely sounds like the sort of optimization that LLVM should "usually" be free to make.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 23:39):

....though, on second thought, that's probably not true, since that wouldn't be valid in C++ (since the compiler can't know whether there are other mutable references to x that may be accessed in the course of executing bar()).

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2020 at 23:40):

So that definitely seems like it invalidates an optimization that would be valid in Rust, but probably isn't done by LLVM as part of any "standard" optimization pass.

view this post on Zulip nagisa (Jun 09 2020 at 23:45):

nikomatsakis said:

then we can't move that *x += 1 down below bar()

Without thinking too much about this, I believe this kind of transformation (and the reverse of moving assignments before function call) is only valid if bar is known to be side-effect free, and you need to take a conservative stance (i.e. it has side effects) if you can’t say otherwise. bar containing a longjmp, panic! etc, would make it side-effectful.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 10 2020 at 00:26):

That's a good point.

view this post on Zulip bjorn3 (Jun 10 2020 at 06:54):

I thought stacked borrows would enable exactly this kind of optimizations, as we know bar can't access x without violating the exclusiveness of the mutable reference. The compiler would have to revert the assignment on unwinding though.

view this post on Zulip nikomatsakis (Jun 10 2020 at 13:17):

@nagisa it's true that, for panic, you would have to duplicate the effect on both paths, but it is in principle possible. longjmp is different, though, as there is no way to intervene. I recall that we've also discussed this question in the context of inter-process shared memory, where writes would be visible outside the process (and in that case, the transformation is simply not possible at all). There is a valid question then as to whether &mut should be able to refer to shared memory. I sort of lean towards "yes" but that seems to imply a lot of limits on whether we can move writes.

view this post on Zulip nikomatsakis (Jun 10 2020 at 13:20):

BatmanAoD (Kyle Strand) said:

And, as always, I think of "UB" as "the compiler has free reign to optimize in ways that break code"; so in this case, I am wondering, do we _want_ to give the compiler extra optimization potential by assuming that longjmp can never cross non-POFs?

as far as the origin topic goes, I think we definitely ought to start distinguishing the question of "strict UB" versus "compositional UB". In other words, what kinds of things can we do but only if we control all the stack frames in between (longjmp may fall in that category, but as we pointed out here there is actually a choice being made in terms of potential optimizations that I don't think I at least was fully aware of).

view this post on Zulip RalfJ (Jun 10 2020 at 16:55):

nikomatsakis said:

but I'm not 100% sure about that, RalfJ would remember

I think I am missing a lot of context here... what's a POF?

view this post on Zulip RalfJ (Jun 10 2020 at 16:55):

I got pinged on https://github.com/rust-lang/project-ffi-unwind/pull/29#discussion_r438088431, does that question subsume this one here?

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 10 2020 at 16:59):

@RalfJ Not really, sorry! Niko's question here, as I understand it, is whether reordering mutations and function calls is something the compiler team has discussed permitting the optimizer to do.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 10 2020 at 17:02):

The linked PR does explain the "POF" terminology, though. (If you would like to see the discussion that led to that term, it's here: https://rust-lang.zulipchat.com/#narrow/stream/210922-project-ffi-unwind/topic/posting.20the.20RFc/near/197461120. And of course feel free to opine on the subject, especially if you can do so before the RFC is posted.)

view this post on Zulip RalfJ (Jun 10 2020 at 17:06):

whether reordering mutations and function calls is something the compiler team has discussed permitting the optimizer to do

It's something we proved correct for Stacked Borrows in https://plv.mpi-sws.org/rustbelt/stacked-borrows/ :D

view this post on Zulip RalfJ (Jun 10 2020 at 17:06):

but that doesnt model panics

view this post on Zulip RalfJ (Jun 10 2020 at 17:06):

however, as mentioned above in this thread, moving the mutation down into both the normal-return and the unwind-return block ought to be a correct optimization

view this post on Zulip RalfJ (Jun 10 2020 at 17:08):

does that answer the question?^^ I'll look at that PR tomorrow, and then maybe if you have a self-contained question assuming only knowledge that is described in that PR, that would help :D

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 10 2020 at 17:14):

Yes, it does! Thanks. And, yes, I believe Niko's question in the PR comments only requires knowledge self-contained in the PR.

view this post on Zulip Amanieu (Jun 10 2020 at 23:03):

@RalfJ Basically the issue that longjmp breaks that assumption since you are explicitly allowed to longjmp out of nounwind functions.

view this post on Zulip RalfJ (Jun 11 2020 at 07:16):

@Amanieu that is quoting LLVM's rules, right? Rust could presumably have more restrictive rules and also limit what functions can be longjumped out of?

view this post on Zulip Amanieu (Jun 11 2020 at 07:18):

The LLVM rules are somewhat unclear, but I have to assume they allow longjmp since that's used in C code and C code is all compiled as nounwind.

view this post on Zulip RalfJ (Jun 11 2020 at 08:26):

sure. but Rust is not bound to these rules. we can have functions that are nounwindandnolongjmp, and optimize MIR accordingly, even if LLVM IR cannot express this.

view this post on Zulip nikomatsakis (Jun 11 2020 at 13:52):

@RalfJ I think that's right. The point is more that the rules we were on the point of proposing would, in fact, permit longjmp over frames that don't have destructors without any sort of "opt-in"

view this post on Zulip nikomatsakis (Jun 11 2020 at 13:53):

But this seems to close a door that I'm not sure we want to close

view this post on Zulip nikomatsakis (Jun 11 2020 at 13:55):

OTOH I'm not sure, the alternatives are kind of messy. One side note is that I remember now talking to @Yehuda Katz some time back about how one might want compiler support to validate that if you invoke a fn that may longjmp, the compiler can check that no destructors are in scope (it's a "Plain Old Frame"...).

view this post on Zulip nikomatsakis (Jun 11 2020 at 13:56):

Technically the main thing we've been trying to decide is what to do with panics that propagate (at least that's @acfoltzer's main interest). I guess we could leave longjmp for later work as part of this RFC and discuss it a bit more, though the constraints all remain sorta the same (e.g., existing functions in libc that are a pain, etc)

view this post on Zulip Amanieu (Jun 11 2020 at 20:01):

It occurs to me that this optimization might be unsound even without longjmp. Consider the case where x points to a memory-mapped file and bar calls exit(). You would expect the write to x to be reflected in the file on exit, but that won't happen if the write is moved after the call.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 11 2020 at 20:30):

I'm not sure I would expect that, actually. Don't call exit() if you want cleanup to happen.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 11 2020 at 20:31):

File writes have tricky edge conditions.

view this post on Zulip Amanieu (Jun 12 2020 at 00:46):

IMO any optimization moving globally visible side effects across a function call are only sound if that function call is guaranteed to eventually return normally.

view this post on Zulip bjorn3 (Jun 12 2020 at 08:47):

For writing memory mapped files you must to use write_volatile, as writing the memory has side-effects outside of the knowledge of the compiler. When you don't use write_volatile, the compiler is free to assume that writes don't have any side-effects other than affecting later reads of the same location. This means that the compiler is free to remove writes or insert spurious writes to memory locations it knows will never be read again, or only be read after a later write.

view this post on Zulip Amanieu (Jun 12 2020 at 18:45):

That is not true. Accessing a memory mapped file is not a side-effect, you're just writing to memory that is shared with another thread (the kernel).

view this post on Zulip bjorn3 (Jun 12 2020 at 18:58):

You are right. For memory mapped files you should use atomic instructions, not volatile.

view this post on Zulip Vadim Petrochenkov (Jun 12 2020 at 19:16):

That's pretty impractical though?
One point of mapping something into memory is to use some pre-existing or generic code reading or writing memory buffers.

view this post on Zulip Vadim Petrochenkov (Jun 12 2020 at 19:18):

I.e. using some separation of concerns - the "algorithmic" code just works with memory buffers, the "backend" code either maps those buffers into files directly, or e.g. zips those buffers and stores them to files in some other way.

view this post on Zulip nikomatsakis (Jun 12 2020 at 19:28):

there's definitely a tension here, I don't know what I think yet. One thing I am thinking is that this discussion would benefit from bringing in more of the "unsafe code guidelines" regulars, more so than the relatively narrow ffi-unwind audience thus far. It feels clear that there is a "host of entangled patterns" going on here, and I'm guessing there's been some good conversation to dig up.

view this post on Zulip Josh Triplett (Jun 12 2020 at 20:19):

@bjorn3 You don't need to use atomics on mmaped files; they have a weak ordering model, and if you care about the ordering you have to call msync (or something else that requires consistency).

view this post on Zulip RalfJ (Jun 13 2020 at 10:15):

Amanieu said:

It occurs to me that this optimization might be unsound even without longjmp. Consider the case where x points to a memory-mapped file and bar calls exit(). You would expect the write to x to be reflected in the file on exit, but that won't happen if the write is moved after the call.

that would break the really fundamental assumption that memory is an unobservable internal implementation detail that the compiler is free to optimize around, unless marked otherwise using things such as volatile.

view this post on Zulip RalfJ (Jun 13 2020 at 10:16):

if mmaped files are allowed to communicate without such things, they must obey the aliasing rules

view this post on Zulip RalfJ (Jun 13 2020 at 10:17):

and one of the rules is that an &mut passed to a function must be a unique pointer until that function returns. if the function never returns for one reason or another, that means it must be a unique pointer forever.

view this post on Zulip Amanieu (Jun 13 2020 at 10:26):

I guess we're back to the original issue then, does exit count as "returning" the same way longjmp does?

view this post on Zulip RalfJ (Jun 13 2020 at 10:26):

yeah...

view this post on Zulip Amanieu (Jun 13 2020 at 19:10):

It occurs to me that if we do allow the compiler to perform this optimization then it makes the whole POF concept useless: the compiler can arbitrarily add destructors where there are none by moving code after a function call and duplicating it on the unwind and normal return paths.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 13 2020 at 19:43):

At this point I believe that the POF concept will only be useful in conjunction with the "cancelable" annotation we plan to introduce.

view this post on Zulip nikomatsakis (Jun 16 2020 at 14:59):

right, I wouldn't say that makes POF useless

view this post on Zulip nikomatsakis (Jun 16 2020 at 14:59):

it's "necessary but not sufficient"

view this post on Zulip RalfJ (Jun 20 2020 at 10:29):

@Amanieu you mean in the sense that, adding code to the unwind path adds a destructor? good point.

view this post on Zulip RalfJ (Jun 20 2020 at 10:30):

I guess there is a discussion to be had if this is an optimization that we want. I am really proud that Stacked Borrows can provide it, but there's clearly a trade-off. OTOH, this is an optimization that can benefit all code vs a rather niche feature. Still, worth a discussion it seems.


Last updated: Jan 26 2022 at 08:34 UTC