5 ms·

I think this is moving the goalposts. My claim was not "these C libraries perfectly avoid UB by dynamically checking every invariant that could lead to UB if b

I think this is moving the goalposts. My claim was not "these C libraries perfectly avoid UB by dynamically checking every invariant that could lead to UB if broken." Clearly they do not, as you have demonstrated. (Neither does unsafe Rust). My claim was that in cases where a (low-level, high quality) C library does check an invariant in a release build, it will generally report failure of that invariant as an explicit error code rather than by crashing the process. To falsify that, you would need to find places where these libraries call abort() or exit() in response to an internal inconsistency, in a release build. I think you are unlikely to find examples of that in these libraries. (After a bit of searching, I see that libavcodec has a few abort()s, but uses AVERROR_BUG an order of magnitude more often). I agree with you that Rust's "safe by default" is important. I am advocating that Rust can be a powerful tool to provide C-like behavior (no crash on inconsistency) with greater safety (checking all relevant inconsistencies by default). In cases where C-like behavior is desired, that's a really appealing proposition. Upthread it seemed like you were objecting to the idea of ever reporting internal inconsistencies as recoverable errors. You argued that creating and documenting error codes for this is not common or practical: > I'd love to see the API docs for them. "This error value is impossible and this library will never return it. If it does, then there is a bug in the library. Since there are no known bugs related to this invariant violation, this cannot happen." That is exactly what SQLITE_INTERNAL and AVERROR_BUG are.

burntsushi 2y ago
> My claim was that in cases where a (low-level, high quality) C library does check an invariant in a release build, it will generally report failure of that invariant as an explicit error code rather than by crashing the process. That just seems very uninteresting though? And it kinda misses the whole point of where this conversation started. It's true that Rust code is going to check more things because of `unwrap()`, but that's a good thing! Because the alternative is clearly what C libraries practice: they'll just have UB. So you give up the possibility of an RCE for the possibility of a DoS. Sounds like a good trade to me. >> I'd love to see the API docs for them. "This error value is impossible and this library will never return it. If it does, then there is a bug in the library. Since there are no known bugs related to this invariant violation, this cannot happen." > > That is exactly what SQLITE_INTERNAL and AVERROR_BUG are. I meant that it should reflect the philosophy of handling broken runtime invariants generally in the library. Just because there's one error code for some restricted subset of cases doesn't mean that's how they deal with broken runtime invariants. In all of your examples so far, the vast majority of broken runtime variants from what I can see lead to UB, not error codes. This is what I meant because this is what makes Rust and its panicking materially different from C. And it's relevant especially in contexts where people say, "well just return an error instead of panicking." But C libraries generally don't do that either! They don't even bother checking most runtime invariants anyway, even when it doesn't matter for perf. This is a big knot to untangle and I'm sure my wording could have been more precise. This is why I wanted to focus on examples, because we can look at real world things. And from my perspective, the examples you've given do not embody the original advice that I was replying to: > That behavior is up to the user. The library should only report the error. Instead, while there is limited support for "this error is a bug," the C libraries you've linked overwhelming prefer UB. That's the relevant point of comparison. I'm not interested in trying to find C libraries that abort. I'm interested in a holistic comparison of actual practice and using that to contextualize the blanket suggestions given in this thread.
- haberman 2y ago
  > It's true that Rust code is going to check more things because of `unwrap()`, but that's a good thing! Because the alternative is clearly what C libraries practice: they'll just have UB. I have been consistently advocating for a third alternative that I happen to like more than either of these. My alternative is: write libraries in No-Panic Rust. That means we have all of the safety, but none of the crashes. It is consistent with the position articulated upthread: > That behavior is up to the user. The library should only report the error. No-Panic Rust means always using "?" instead of unwrap(). This doesn't give up any safety! It just reports errors in a different way. Unfortunately it does mean eschewing the standard library, which isn't generally programmed like this. I won't argue that every library should use this strategy. It is undoubtedly much more work. But in some cases, that extra work might be justified. Isn't it nice that this possibility exists?
  - burntsushi 2y ago
    We're back to square one: show me some real Rust libraries in widespread use actually adhering to this philosophy. And then I want to see some applications built with this philosophy. Then we can look at what the actual user experience difference is when a bug occurs. In one case, you get a panic with a stack trace. In the other, you get an error value that the application does... what with? Prints it as an unactionable error to end users and aborts? If it continues on, does your library make any guarantees about the consistency of its internal state when a runtime invariant is broken? Panicking branches are everywhere in Rust. And even in your blog, you needed to use `unsafe` to avoid some of them. So I don't really get why you claim it is safer. Users of my libraries would 100% be super annoyed by this. Imagine if `Regex::find` returned a `Result` purely because a bug might happen. > But in some cases, that extra work might be justified. Isn't it nice that this possibility exists? What I said above: > Sure, you can propagate an error. I just don't really see a compelling reason to do so. Like, maybe there are niche scenarios where maybe it's worthwhile, but I do not see how it would be compelling to suggest it as general practice. Your blog is an interesting technical exercise, but you spend comparatively little time on whether doing it is actually worth the trouble. And there is effectively no space at all reserved to how this impacts library API design. To be fair, you do acknowledge this: > I should be clear that I have not yet attempted this technique at scale, so I cannot report on how well it works in practice. For now it is an exciting future direction for upb, and one that I hope will pay off. From your blog, you list 3 reasons to do this: binary size, unrecoverability and runtime overhead. I find that binary size is the only legitimate reason here, and for saving 300 KB, I would absolutely call that very niche. And especially so given that you can make panics abort to remove the code size overhead. I find unrecoverability unconvincing because we are talking about bugs here. Panics are just one very convenient manifestation of a bug. But lots of bugs are silent and just make the output incorrect in some way. I just don't see a problem at all with bugs, generally, causing an abort with a useful error message. I find runtime overhead very unconvincing because you can opt out of them on a case-by-case basis when perf demands it. We can go around the maypole all day on this. But I want to see real examples following your philosophy. Because then I can poke and prod at it and point to what I think you're missing. Is the `upd` port publicly available?

burntsushi 2y ago

> My claim was that in cases where a (low-level, high quality) C library does check an invariant in a release build, it will generally report failure of that invariant as an explicit error code rather than by crashing the process. That just seems very uninteresting though? And it kinda misses the whole point of where this conversation started. It's true that Rust code is going to check more things because of `unwrap()`, but that's a good thing! Because the alternative is clearly what C libraries practice: they'll just have UB. So you give up the possibility of an RCE for the possibility of a DoS. Sounds like a good trade to me. >> I'd love to see the API docs for them. "This error value is impossible and this library will never return it. If it does, then there is a bug in the library. Since there are no known bugs related to this invariant violation, this cannot happen." > > That is exactly what SQLITE_INTERNAL and AVERROR_BUG are. I meant that it should reflect the philosophy of handling broken runtime invariants generally in the library. Just because there's one error code for some restricted subset of cases doesn't mean that's how they deal with broken runtime invariants. In all of your examples so far, the vast majority of broken runtime variants from what I can see lead to UB, not error codes. This is what I meant because this is what makes Rust and its panicking materially different from C. And it's relevant especially in contexts where people say, "well just return an error instead of panicking." But C libraries generally don't do that either! They don't even bother checking most runtime invariants anyway, even when it doesn't matter for perf. This is a big knot to untangle and I'm sure my wording could have been more precise. This is why I wanted to focus on examples, because we can look at real world things. And from my perspective, the examples you've given do not embody the original advice that I was replying to: > That behavior is up to the user. The library should only report the error. Instead, while there is limited support for "this error is a bug," the C libraries you've linked overwhelming prefer UB. That's the relevant point of comparison. I'm not interested in trying to find C libraries that abort. I'm interested in a holistic comparison of actual practice and using that to contextualize the blanket suggestions given in this thread.

haberman 2y ago
> It's true that Rust code is going to check more things because of `unwrap()`, but that's a good thing! Because the alternative is clearly what C libraries practice: they'll just have UB. I have been consistently advocating for a third alternative that I happen to like more than either of these. My alternative is: write libraries in No-Panic Rust. That means we have all of the safety, but none of the crashes. It is consistent with the position articulated upthread: > That behavior is up to the user. The library should only report the error. No-Panic Rust means always using "?" instead of unwrap(). This doesn't give up any safety! It just reports errors in a different way. Unfortunately it does mean eschewing the standard library, which isn't generally programmed like this. I won't argue that every library should use this strategy. It is undoubtedly much more work. But in some cases, that extra work might be justified. Isn't it nice that this possibility exists?
- burntsushi 2y ago
  We're back to square one: show me some real Rust libraries in widespread use actually adhering to this philosophy. And then I want to see some applications built with this philosophy. Then we can look at what the actual user experience difference is when a bug occurs. In one case, you get a panic with a stack trace. In the other, you get an error value that the application does... what with? Prints it as an unactionable error to end users and aborts? If it continues on, does your library make any guarantees about the consistency of its internal state when a runtime invariant is broken? Panicking branches are everywhere in Rust. And even in your blog, you needed to use `unsafe` to avoid some of them. So I don't really get why you claim it is safer. Users of my libraries would 100% be super annoyed by this. Imagine if `Regex::find` returned a `Result` purely because a bug might happen. > But in some cases, that extra work might be justified. Isn't it nice that this possibility exists? What I said above: > Sure, you can propagate an error. I just don't really see a compelling reason to do so. Like, maybe there are niche scenarios where maybe it's worthwhile, but I do not see how it would be compelling to suggest it as general practice. Your blog is an interesting technical exercise, but you spend comparatively little time on whether doing it is actually worth the trouble. And there is effectively no space at all reserved to how this impacts library API design. To be fair, you do acknowledge this: > I should be clear that I have not yet attempted this technique at scale, so I cannot report on how well it works in practice. For now it is an exciting future direction for upb, and one that I hope will pay off. From your blog, you list 3 reasons to do this: binary size, unrecoverability and runtime overhead. I find that binary size is the only legitimate reason here, and for saving 300 KB, I would absolutely call that very niche. And especially so given that you can make panics abort to remove the code size overhead. I find unrecoverability unconvincing because we are talking about bugs here. Panics are just one very convenient manifestation of a bug. But lots of bugs are silent and just make the output incorrect in some way. I just don't see a problem at all with bugs, generally, causing an abort with a useful error message. I find runtime overhead very unconvincing because you can opt out of them on a case-by-case basis when perf demands it. We can go around the maypole all day on this. But I want to see real examples following your philosophy. Because then I can poke and prod at it and point to what I think you're missing. Is the `upd` port publicly available?