17 ms·

> Its `SQLITE_INTERNAL` error code is atypical in my experience. In my experience it's reasonably common. Here are some other examples in what I would conside

> Its `SQLITE_INTERNAL` error code is atypical in my experience. In my experience it's reasonably common. Here are some other examples in what I would consider quintessential, high-quality C libraries: - zlib has Z_STREAM_ERROR, which is documented in several places as being returned "if the stream structure was inconsistent" - libavcodec has AVERROR_BUG, documented as "Internal bug, also see AVERROR_BUG2". - LMDB has MDB_PANIC, documented as "Update of meta page failed or environment had fatal error". > And for sure, some people have hit bugs that manifest as panics. And those could in turn feasibly be DoS problems. But they definitely weren't RCEs, and that's because I used `unwrap()`. I feel this is conflating two things: (1) whether or not an invariant should get a dynamic check, and (2) when a dynamic check is present, how the failure should be reported. Rust brings safety by forcing (safe) code to use dynamic checks when a safety property cannot be statically guaranteed, which addresses (1). But there's still a degree of freedom for whether failures are reported as panics or as recoverable errors to the caller. I wrote down some of my thinking in this recent blog entry, which actually quotes your excellent summary of when panics are appropriate: https://blog.reverberate.org/2025/02/03/no-panic-rust.html https://blog.reverberate.org/2025/02/03/no-panic-rust.html (ps: I'm a daily rg user and fan of your work!)

burntsushi 2y ago
I'd have to look more closely at those examples, but I find it hard to believe that every runtime invariant violation manifests as one of those error codes. It certainly isn't true for PCRE2. > Rust brings safety by forcing (safe) code to use dynamic checks when a safety property cannot be statically guaranteed, which addresses (1). But there's still a degree of freedom for whether failures are reported as panics or as recoverable errors to the caller. 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. You might point to C libraries doing the same, but I'd have to investigate what exactly those error codes are actually being used for and _why_ the C library maintainers added them. And the trade-offs in C land are totally different than in Rust. Those error codes might not exist if they had a panicking mechanism available to them. > I wrote down some of my thinking in this recent blog entry, which actually quotes your excellent summary of when panics are appropriate: https://blog.reverberate.org/2025/02/03/no-panic-rust.html https://blog.reverberate.org/2025/02/03/no-panic-rust.html Yes, I've read that. It's a nice blog, but I don't think it's broadly applicable. Like, I don't see why I would write no-panic-Rust outside of extremely niche scenarios. My blog on unwraps is meant to be more broadly applicable: https://burntsushi.net/unwrap/ https://burntsushi.net/unwrap/ (It even covers this case of trying to turn runtime invariant violations into error codes.)
burntsushi 2y ago
Now that I've slept, I decided to take a look at LMDB. It uses MDB_PANIC in exactly two places: https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L3182 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L11420 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... I would say this overall does not even come close to qualifying as an example of a library that "returns errors for invariant violations instead of committing UB." You don't have to look far to see something that would normally be a panicking branch in Rust be a UB branch in C: https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L1771-L1774 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... if (err >= MDB_KEYEXIST && err <= MDB_LAST_ERRCODE) { i = err - MDB_KEYEXIST; return mdb_errstr[i]; } That `mdb_errstr[i]` will have UB if `i` is out of bounds. And `i` could be out of bounds if this code gets out of sync with the defined error constants and `mdb_errstr`. Moreover, it seems quite unlikely that this particular part of the code benefits perf-wise from omitting bounds checks. In other words, if this were Rust code and someone used `unsafe` to opt out of bounds checks here (assuming they weren't already elided automatically), that would be a gross error in judgment IMO. The kind of examples I'm asking for would be C libraries that catch these sorts of runtime invariants and propagate them up as errors. Instead, at least for LMDB, MDB_PANIC isn't really used for this purpose. Now looking at zlib, from what I can tell, Z_STREAM_ERROR is used to validate input arguments. It's not actually being used to detect runtime invariants. zlib is just like most any other C library as far as I can tell. There are UB branches everywhere. I'm sure some of those are important for perf, but I've spent 10 years working on optimizing low level libraries in Rust, and I can say for certain that the vast majority of them are not. libavcodec is more of the same. There are a ton of runtime invariants everywhere that are just UB if they are broken. Again, this is not an example of a library eagerly checking for invariant violations and percolating up errors. From what I can see, AVERROR_BUG is used at various boundaries to detect some kinds of inconsistencies in the data. IMO, your examples are a total misrepresentation of how C libraries typically work. From my review, my prior was totally confirmed: C libraries will happily do UB when runtime invariants are broken, where as Rust code tends to panic. Rust code will opt into the "UB when runtime invariants are broken," but it is far far more limited. And this further demonstrates why "unsafe by default" is so bad.
hyc_symas 2y ago
> LMDB has MDB_PANIC, documented as "Update of meta page failed or environment had fatal error". Yes. That doesn't mean there was anything bad in the program logic. It most likely means your storage device had a fatal I/O error. It means there's something physically wrong with your system. Not that there was any bug in any code.

burntsushi 2y ago

I'd have to look more closely at those examples, but I find it hard to believe that every runtime invariant violation manifests as one of those error codes. It certainly isn't true for PCRE2. > Rust brings safety by forcing (safe) code to use dynamic checks when a safety property cannot be statically guaranteed, which addresses (1). But there's still a degree of freedom for whether failures are reported as panics or as recoverable errors to the caller. 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. You might point to C libraries doing the same, but I'd have to investigate what exactly those error codes are actually being used for and _why_ the C library maintainers added them. And the trade-offs in C land are totally different than in Rust. Those error codes might not exist if they had a panicking mechanism available to them. > I wrote down some of my thinking in this recent blog entry, which actually quotes your excellent summary of when panics are appropriate: https://blog.reverberate.org/2025/02/03/no-panic-rust.html https://blog.reverberate.org/2025/02/03/no-panic-rust.html Yes, I've read that. It's a nice blog, but I don't think it's broadly applicable. Like, I don't see why I would write no-panic-Rust outside of extremely niche scenarios. My blog on unwraps is meant to be more broadly applicable: https://burntsushi.net/unwrap/ https://burntsushi.net/unwrap/ (It even covers this case of trying to turn runtime invariant violations into error codes.)

burntsushi 2y ago

Now that I've slept, I decided to take a look at LMDB. It uses MDB_PANIC in exactly two places: https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L3182 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L11420 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... I would say this overall does not even come close to qualifying as an example of a library that "returns errors for invariant violations instead of committing UB." You don't have to look far to see something that would normally be a panicking branch in Rust be a UB branch in C: https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26ec831d0c24fac7/libraries/liblmdb/mdb.c#L1771-L1774 https://github.com/LMDB/lmdb/blob/f20e41de09d97e4461946b7e26... if (err >= MDB_KEYEXIST && err <= MDB_LAST_ERRCODE) { i = err - MDB_KEYEXIST; return mdb_errstr[i]; } That `mdb_errstr[i]` will have UB if `i` is out of bounds. And `i` could be out of bounds if this code gets out of sync with the defined error constants and `mdb_errstr`. Moreover, it seems quite unlikely that this particular part of the code benefits perf-wise from omitting bounds checks. In other words, if this were Rust code and someone used `unsafe` to opt out of bounds checks here (assuming they weren't already elided automatically), that would be a gross error in judgment IMO. The kind of examples I'm asking for would be C libraries that catch these sorts of runtime invariants and propagate them up as errors. Instead, at least for LMDB, MDB_PANIC isn't really used for this purpose. Now looking at zlib, from what I can tell, Z_STREAM_ERROR is used to validate input arguments. It's not actually being used to detect runtime invariants. zlib is just like most any other C library as far as I can tell. There are UB branches everywhere. I'm sure some of those are important for perf, but I've spent 10 years working on optimizing low level libraries in Rust, and I can say for certain that the vast majority of them are not. libavcodec is more of the same. There are a ton of runtime invariants everywhere that are just UB if they are broken. Again, this is not an example of a library eagerly checking for invariant violations and percolating up errors. From what I can see, AVERROR_BUG is used at various boundaries to detect some kinds of inconsistencies in the data. IMO, your examples are a total misrepresentation of how C libraries typically work. From my review, my prior was totally confirmed: C libraries will happily do UB when runtime invariants are broken, where as Rust code tends to panic. Rust code will opt into the "UB when runtime invariants are broken," but it is far far more limited. And this further demonstrates why "unsafe by default" is so bad.

hyc_symas 2y ago

> LMDB has MDB_PANIC, documented as "Update of meta page failed or environment had fatal error". Yes. That doesn't mean there was anything bad in the program logic. It most likely means your storage device had a fatal I/O error. It means there's something physically wrong with your system. Not that there was any bug in any code.