4 ms·

> The cloudflare bug was not caused by rust Yeah. Rust’s Option::None as like null in C++. Unwrapping an option is like checking if something is null and crash

by josephg 11mo ago

> The cloudflare bug was not caused by rust Yeah. Rust’s Option::None as like null in C++. Unwrapping an option is like checking if something is null and crashing. This "crash from an unwrap" is just a null pointer exception / segfault in any other language. The same bug would be trivial to write in any other language, and with more or less the exact same result. Its just - weirdly news because it was rust code. What? > Honestly, it reads like a lot of critiques of Rust: people haven't spent enough time with it, and are not over the learning curve yet. Exactly. I’ve spent years with rust and my complaints are very different. Well, I still wish it compiled faster but all the other stuff is noobie problems. As someone with more experience, my frustration points are things like how async blocks do compiler magic you can’t write yourself. I hate unnameable types. I think Pin is confusing and weird. And the syntax for interacting with raw pointers is unnecessarily inconvenient and ugly. Yes, rust makes programs with a lot of shared mutable state awkward to write. So don’t program like that. In general, if you write code in language X as if it were a bad version of language Y, you're going to have a bad time. The right question here is if the "rust way" of thinking about your code results in a beautiful program. You can't answer that if you go in with too many preconceptions of how programs should be designed.

tialaramex 11mo ago
The Cloudflare bug was unwrapping a Result::Err not an Option::None. Both Option and Result can be unwrapped and - to some extent not coincidentally - both can also be subject to the Try operator (?) which is arguably more correct here than unwrap because this can fail and perhaps the caller will have some plan to recover. My list of peeves would be very different from yours. I would like to prohibit move of the dodgier as casts (requiring that you write out the conversion call you actually meant explicitly and then handling any edge cases) for example. What sort of "interacting with raw pointers" are you thinking of that's too inconvenient and ugly for your liking?
- josephg 11mo ago
  > Both Option and Result can be unwrapped and - to some extent not coincidentally - both can also be subject to the Try operator (?) which is arguably more correct here than unwrap because this can fail and perhaps the caller will have some plan to recover. I guess its equivalent to Go code like this: value, err := stuff() if err != nil { panic(err) } Or in C: int result = stuff(); assert(result >= 0); If someone wrote code like that and the program crashed, nobody would attack Go or C. In all these cases (including rust), the programmer is expressing clear, explicit intent for the program to panic if the call failed. I've seen some people say .unwrap() should be named .unwrap_or_panic() or something to make that more clear? I dunno. It seems like a nothingburger to me. A buggy program crashed. They'll do that. > What sort of "interacting with raw pointers" are you thinking of that's too inconvenient and ugly for your liking? I have 2 complaints. First, I wish Rust had a -> operator or something like it. This sort of thing is horrible: (*(*(*ptr).foo).bar) Secondly, rust's aliasing rules make writing pointer heavy code very difficult. I like to run my unsafe code through MIRI to make sure I'm not doing anything bad, and its incredibly subtle. Here's an unsafe function which manages the internal state of a skip list: https://github.com/josephg/jumprope-rs/blob/3981256e4e741d8b19efe26ffcfcfe178332eda0/src/jumprope.rs#L965-L1034 https://github.com/josephg/jumprope-rs/blob/3981256e4e741d8b... This code is insanely fragile. There are multiple pointers to different elements floating around. The only way I got miri to calm down was to dereference the node pointer constantly. Eg: *cursor.num_bytes -= (*node).str.len_bytes(); let next = (*node).first_next().node; Basically everything else I tried caused obscure aliasing problems. I think if I rewrote this code today I'd probably use a Vec and safe rust instead. I can much more confident write correct C code than correct unsafe rust code. I wish it were the other way around. As a general principle, unsafe rust should be as simple and easy to reason about as possible so we can spot security problems. But rust's syntax makes unsafe code very complex. I don't think this complexity is in any way necessary.
  - tialaramex 11mo ago
    I see what you mean for -> although personally I haven't missed it (I spent many years writing C) I don't write much pointer twiddling in Rust so I'm the wrong person to have an opinion. However surely the aliasing is actually a problem and so MIRI is annoyed because what you wrote might be wrong? If we hang on to (*node).first_next() but somehow node changes, we're no longer talking about the same thing, it's exactly the aliasing problem. I'd have to (which I have not) examine in detail how your code works to offer an opinion beyond speculation, but I think my instinct is sympathy for the "Don't write aliases" approach.
    - josephg 11mo ago
      > I think my instinct is sympathy for the "Don't write aliases" approach. Yes, requiring that nothing alias it’s much more conceptually elegant. But as I said, in practice it makes programming extremely treacherous. There are no warnings when you alias. No errors. 99% of the time you’ll get a working program with no bugs every time you build. You’re just - quietly - at the whim of llvm. Maybe it won’t compile correctly on some other architecture. Maybe a minor compiler bump, or different flags, will make your program totally broken. The only way to discover there’s even a potential problem is by running the code through MIRI. I get why it’s like this but I think it’s a mistake. I think unsafe rust should be at least as expressive and pleasant to work with as C. Having quiet foot guns here seems antithetical to rust’s philosophy of safe ergonomic programming.
- Klonoar 11mo ago
  Err, you can't use `?` with `Option` unless I've really missed a big feature somewhere. You'd have to map it to a `Result` then unwrap, but at that point you still need to define an error type.
  - tialaramex 11mo ago
    https://doc.rust-lang.org/std/option/enum.Option.html#impl-Try-for-Option%3CT%3E https://doc.rust-lang.org/std/option/enum.Option.html#impl-T... I guess you've "really missed" this feature or more likely you are considering only the case where our function signature says we return Result and so early returning Option won't compile. Because the modern "try v2" operator is a trait, it is implemented for Option, Result, and ControlFlow plus Poll variations. Historically its ancestor the try! macro was provided only for Result, but that's a very long time ago now.
    - Klonoar 11mo ago
      I guess? If it's still languishing in nightly I'm not sure I consider it usable; I'm aware some people are fine to run nightly but I keep to stable releases.
      - tialaramex 11mo ago
        So, all of Try is unstable. This means your stable Rust isn't allowed to implement Try for your own custom types, however the syntax is stable, so since Try is implemented for all of Option, Result and ControlFlow, those all work with the ? syntax even though that implementation is marked unstable. Let me give you another example you've probably used without realising it, when you write password.contains("foo") in Rust. Why can you write "foo" there? Rust doesn't have overloading, and yet you can pass strings, characters, and functions including lambdas - how can this work? They all implement core::str::pattern::Pattern, an unstable trait used to deliver this functionality for all string matching. In fact mechanically what's happening here for Try is that ? always worked for Option and Result, the "try v2" feature isn't about the syntax at all, it's about the behind the scenes implementation of that syntax. But that's not important, the syntax is promised and the final implementation will almost certainly be tryv2 or something very similar.
adastra22 11mo ago
Rust makes it very, very easy to .unwrap() something, making panic-crashes the default error handling behavior for a lot of rust programmers. But it does this without, e.g., Erlang's reliability services that will auto-relaunch code that crashes. The end result is NOT a very good user experience. I say this as a Rust advocate and daily Rust programmer, btw.
- dontlaugh 11mo ago
  It should probably have a different name. But nil panics in Go are even easier, the compiler doesn’t even require you to opt in. I deal with that every day.
- estebank 11mo ago
  When it comes to distributed services, unwrap is likely the behavior you do want, because the service is being run by a scheduler that detects failures, and a panic would be tied to a metric and page an sre. My read of the situation is that they were overwhelmed by their dashboards which made it harder to identify the root cause, but that is a common situation for these kind of events. I'm pretty sure that this exact sequence of events will never happen again to them because they will adjust their observability to be clearer, should this ever happen again. For libraries and cli tools, you don't want panics except for things that are broken beyond any recovery. A cli tool should not panic directly, instead emitting human readable errors.
  - josephg 11mo ago
    Yeah. Its also weird that people online are fixating on unwrap - as if changing or fixing unwrap's semantics would have helped cloudflare here. The best way to think about unwrap is that its like an assert. If an assert trips, the answer isn't to remove the assert and just hope for the best. Asserts are almost never the problem. The problem is whatever happened right before the assert. Ie, the buggy codepath that generated the erroneous state in the first place. In cloudflare's case, the bug was that their code required that a database query returned less than 200 results. Only the database returned more results than that. This isn't a problem with unwrap. It was these two mutually incompatible pieces of behaviour colliding in code. I don't see why rust has anything to do with this at all. Sloppy programmers can make a mess in any language. Rust is no exception.

by josephg 11mo ago

tialaramex 11mo ago

The Cloudflare bug was unwrapping a Result::Err not an Option::None. Both Option and Result can be unwrapped and - to some extent not coincidentally - both can also be subject to the Try operator (?) which is arguably more correct here than unwrap because this can fail and perhaps the caller will have some plan to recover. My list of peeves would be very different from yours. I would like to prohibit move of the dodgier as casts (requiring that you write out the conversion call you actually meant explicitly and then handling any edge cases) for example. What sort of "interacting with raw pointers" are you thinking of that's too inconvenient and ugly for your liking?

josephg 11mo ago
> Both Option and Result can be unwrapped and - to some extent not coincidentally - both can also be subject to the Try operator (?) which is arguably more correct here than unwrap because this can fail and perhaps the caller will have some plan to recover. I guess its equivalent to Go code like this: value, err := stuff() if err != nil { panic(err) } Or in C: int result = stuff(); assert(result >= 0); If someone wrote code like that and the program crashed, nobody would attack Go or C. In all these cases (including rust), the programmer is expressing clear, explicit intent for the program to panic if the call failed. I've seen some people say .unwrap() should be named .unwrap_or_panic() or something to make that more clear? I dunno. It seems like a nothingburger to me. A buggy program crashed. They'll do that. > What sort of "interacting with raw pointers" are you thinking of that's too inconvenient and ugly for your liking? I have 2 complaints. First, I wish Rust had a -> operator or something like it. This sort of thing is horrible: (*(*(*ptr).foo).bar) Secondly, rust's aliasing rules make writing pointer heavy code very difficult. I like to run my unsafe code through MIRI to make sure I'm not doing anything bad, and its incredibly subtle. Here's an unsafe function which manages the internal state of a skip list: https://github.com/josephg/jumprope-rs/blob/3981256e4e741d8b19efe26ffcfcfe178332eda0/src/jumprope.rs#L965-L1034 https://github.com/josephg/jumprope-rs/blob/3981256e4e741d8b... This code is insanely fragile. There are multiple pointers to different elements floating around. The only way I got miri to calm down was to dereference the node pointer constantly. Eg: *cursor.num_bytes -= (*node).str.len_bytes(); let next = (*node).first_next().node; Basically everything else I tried caused obscure aliasing problems. I think if I rewrote this code today I'd probably use a Vec and safe rust instead. I can much more confident write correct C code than correct unsafe rust code. I wish it were the other way around. As a general principle, unsafe rust should be as simple and easy to reason about as possible so we can spot security problems. But rust's syntax makes unsafe code very complex. I don't think this complexity is in any way necessary.
- tialaramex 11mo ago
  I see what you mean for -> although personally I haven't missed it (I spent many years writing C) I don't write much pointer twiddling in Rust so I'm the wrong person to have an opinion. However surely the aliasing is actually a problem and so MIRI is annoyed because what you wrote might be wrong? If we hang on to (*node).first_next() but somehow node changes, we're no longer talking about the same thing, it's exactly the aliasing problem. I'd have to (which I have not) examine in detail how your code works to offer an opinion beyond speculation, but I think my instinct is sympathy for the "Don't write aliases" approach.
  - josephg 11mo ago
    > I think my instinct is sympathy for the "Don't write aliases" approach. Yes, requiring that nothing alias it’s much more conceptually elegant. But as I said, in practice it makes programming extremely treacherous. There are no warnings when you alias. No errors. 99% of the time you’ll get a working program with no bugs every time you build. You’re just - quietly - at the whim of llvm. Maybe it won’t compile correctly on some other architecture. Maybe a minor compiler bump, or different flags, will make your program totally broken. The only way to discover there’s even a potential problem is by running the code through MIRI. I get why it’s like this but I think it’s a mistake. I think unsafe rust should be at least as expressive and pleasant to work with as C. Having quiet foot guns here seems antithetical to rust’s philosophy of safe ergonomic programming.
Klonoar 11mo ago
Err, you can't use `?` with `Option` unless I've really missed a big feature somewhere. You'd have to map it to a `Result` then unwrap, but at that point you still need to define an error type.
- tialaramex 11mo ago
  https://doc.rust-lang.org/std/option/enum.Option.html#impl-Try-for-Option%3CT%3E https://doc.rust-lang.org/std/option/enum.Option.html#impl-T... I guess you've "really missed" this feature or more likely you are considering only the case where our function signature says we return Result and so early returning Option won't compile. Because the modern "try v2" operator is a trait, it is implemented for Option, Result, and ControlFlow plus Poll variations. Historically its ancestor the try! macro was provided only for Result, but that's a very long time ago now.
  - Klonoar 11mo ago
    I guess? If it's still languishing in nightly I'm not sure I consider it usable; I'm aware some people are fine to run nightly but I keep to stable releases.
    - tialaramex 11mo ago
      So, all of Try is unstable. This means your stable Rust isn't allowed to implement Try for your own custom types, however the syntax is stable, so since Try is implemented for all of Option, Result and ControlFlow, those all work with the ? syntax even though that implementation is marked unstable. Let me give you another example you've probably used without realising it, when you write password.contains("foo") in Rust. Why can you write "foo" there? Rust doesn't have overloading, and yet you can pass strings, characters, and functions including lambdas - how can this work? They all implement core::str::pattern::Pattern, an unstable trait used to deliver this functionality for all string matching. In fact mechanically what's happening here for Try is that ? always worked for Option and Result, the "try v2" feature isn't about the syntax at all, it's about the behind the scenes implementation of that syntax. But that's not important, the syntax is promised and the final implementation will almost certainly be tryv2 or something very similar.

adastra22 11mo ago

Rust makes it very, very easy to .unwrap() something, making panic-crashes the default error handling behavior for a lot of rust programmers. But it does this without, e.g., Erlang's reliability services that will auto-relaunch code that crashes. The end result is NOT a very good user experience. I say this as a Rust advocate and daily Rust programmer, btw.

dontlaugh 11mo ago
It should probably have a different name. But nil panics in Go are even easier, the compiler doesn’t even require you to opt in. I deal with that every day.
estebank 11mo ago
When it comes to distributed services, unwrap is likely the behavior you do want, because the service is being run by a scheduler that detects failures, and a panic would be tied to a metric and page an sre. My read of the situation is that they were overwhelmed by their dashboards which made it harder to identify the root cause, but that is a common situation for these kind of events. I'm pretty sure that this exact sequence of events will never happen again to them because they will adjust their observability to be clearer, should this ever happen again. For libraries and cli tools, you don't want panics except for things that are broken beyond any recovery. A cli tool should not panic directly, instead emitting human readable errors.
- josephg 11mo ago
  Yeah. Its also weird that people online are fixating on unwrap - as if changing or fixing unwrap's semantics would have helped cloudflare here. The best way to think about unwrap is that its like an assert. If an assert trips, the answer isn't to remove the assert and just hope for the best. Asserts are almost never the problem. The problem is whatever happened right before the assert. Ie, the buggy codepath that generated the erroneous state in the first place. In cloudflare's case, the bug was that their code required that a database query returned less than 200 results. Only the database returned more results than that. This isn't a problem with unwrap. It was these two mutually incompatible pieces of behaviour colliding in code. I don't see why rust has anything to do with this at all. Sloppy programmers can make a mess in any language. Rust is no exception.