7 ms·

> But such behaviour is still unacceptable from a library perspective: a library should never, ever call abort or otherwise terminate the program. It's true th

> But such behaviour is still unacceptable from a library perspective: a library should never, ever call abort or otherwise terminate the program. It's true that libraries should not abort for a regular, foreseeable error. I fundamentally disagree that they should never abort. If an invariant (something we believe absolutely must be true) is violated, the only sensible thing to do is abort the process. For example, if you set a pointer to NULL and then half an hour later it's time to store a value in that pointer but somehow it's not NULL anymore: clearly something has gone terribly wrong, either in your logic, or potentially in totally unrelated code in some other library which has scribbled on the heap. If execution is allowed to continue, the resultant behaviour is totally undefined: you might corrupt data, or you might allow a permissions check to pass that should have failed, etc. We can't always detect when something has gone wrong, but when we do, it's irresponsible to drive on or try to return an error.

anon-3988 2y ago
That behavior is up to the user. The library should only report the error.
- burntsushi 2y ago
  This is terrible advice, and people suggesting it should put their money where their mouth it and show real world examples. Which libraries in widespread use know how to detect all of their possible bugs due to invariant violations and report them as explicit error values? 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." Nevermind the fact that you're now potentially introducing an error channel into operations that should never actually error. That potentially has performance implications. Nevermind the fact that now your implementation details (internal runtime invariants are implementation details) have now leaked out into your API. Need a new internal runtime invariant? Now you need to add that fact to the API. Need to remove an invariant? Ah well, you still need to leave that possible error value in your API to avoid breaking your users.
  - haberman 2y ago
    Ok, but let's consider the other extreme: a world where no library can ever assume a runtime invariant holds without dynamically checking it first. In this world, Vec::index() would need to perform not only a bounds check but also a check that the pointer is not NonNull::dangling(). Sure, RawVec is supposed to guarantee that the pointer will not be dangling when cap is >0, but RawVec could have a bug in it. I agree that documenting and returning a PtrWasDanglingError error is not good API design. An InternalError for all such cases seems more reasonable. But at some point we need to be able to assume that certain program invariants hold without checking at all (in a release build).
    - burntsushi 2y ago
      We don't have to live in the extreme though. That's one of the great advantages of Rust. :-) In `regex`, for example, there are certainly some cases where I use `unsafe` to elide those dynamic checks because 1) I couldn't do it in safe code and 2) I got a performance bump from it. But of all the dynamic checks in `regex`, this was an extremely small subset of them. And it makes sense to rely on abstractions like `RawVec` to uphold those guarantees. The point is that you're making that trade-off intentionally and for a specific reason (perf). The idea that I would support dogmatically always checking every runtime invariant everywhere is bonkers. :P In contrast, we have someone here who I responded to that is literally suggesting propagating every possible broken runtime invariant into a public API error value.
      - haberman 2y ago
        I am biased towards thinking that low-level libraries should generally avoid panic. That would mean that all invariants are either assumed true or returned as errors to the user. I think this is not an unreasonable design: it's how low-level C libraries are traditionally designed. For example SQLite does what I mentioned and has a single SQLITE_INTERNAL error that is documented as: > The SQLITE_INTERNAL result code indicates an internal malfunction. In a working version of SQLite, an application should never see this result code. If application does encounter this result code, it shows that there is a bug in the database engine. --https://www.sqlite.org/rescode.html#internal https://www.sqlite.org/rescode.html#internal I didn't mean to imply that you are for dogmatic checking of every runtime invariant, but the message that began that thread seems to advocate for that, going so far as to try to detect other buggy code that might have stomped on your memory.
theamk 2y ago
This logic only works for memory-unsafe languages like C or C++, where the checks are rare and there is a good chance that by the time abnormal condition is detected, things have been going wrong for a while. But that's not true for safe languages - in Rust, if you set a pointer to not-null, it won't end up as null (unless there is buggy unsafe, but let's ignore that). Instead, the panics are likely to be caused by logic errors. Take the decompressor buffer overflow errors author mentioned - the out-of-bounds writes were caused by a bug bit operations generating wrong array index. In Rust, this would be caught by bounds checker, which is good; but Rust would then abort process, which is bad. A hypothetical language that would throw an exception instead of panic'ing would be much better from library perspective - for example a web server might return 500 on that particular request, but it would stay running otherwise.
- deleted 2y ago
  [deleted]
- chrismorgan 2y ago
  Just one problem with your argument: we’re not talking about languages that have orderly null pointer exceptions. We’re expressly talking about languages like C, C++ and Rust. Your web server example is uncompelling, because a panic-based abort is not the only thing that can distress your system. The simplest example is if the library code doesn’t terminate, accidentally triggering an infinite loop. Or (better from some perspectives, worse from others) an infinite loop that allocates until you run out of memory, denying service until maybe an out-of-memory killer kills the process. In such scenarios, your system can easily end up in a state you didn’t write code expecting, where everything is just broken in mysterious ways. No, if you want your web server to be able to return an orderly 500 in as many situations of unforeseen errors as possible, the plain truth is that you need the code that will produce that to run on a different computer (approximate definition, though with some designs it may not quite need to be separate hardware), and that you’ll need various sorts of supervisors to catch deviant states and (attempt to) restore order. In short: for such an example, you already want tools that can abort a runaway process, so it’s actually not so abnormal for a library to be able to abort itself. There’s genuinely a lot to be said for deliberately aborting the entire process early, and handling problems from outside the process. It’s not without its drawbacks, but it is compelling for things like web servers. I would also note that, if you choose to, you can normally catch panics <https://doc.rust-lang.org/std/panic/fn.catch_unwind.html https://doc.rust-lang.org/std/panic/fn.catch_unwind.html>, and Rust web servers tend to do this.
pwdisswordfishz 2y ago
> the only sensible thing to do is abort the process Abort whatever depends on the invariant, which may be less than the whole process.
- int_19h 2y ago
  Yeah, but you need to ensure that you have an actual isolation boundary in place between systems with different invariants to ensure that there's no shared state (since any shared state implies shared invariants). Which is exactly what processes are. Not necessarily OS processes, though - e.g. consider Erlang, where the notion of an ultra-lightweight process provided by the runtime instead is exactly what makes this kind of error handling a natural fit. Now for some languages it is possible to determine that the state strictly through code analysis, without any runtime boundary enforcement. And I think that safe Rust might be in that category, but unsafe Rust definitely isn't - and whether any given library contains unsafe code is an implementation detail...
lelanthran 2y ago
> If an invariant (something we believe absolutely must be true) is violated, the only sensible thing to do is abort the process. For example, if you set a pointer to NULL and then half an hour later it's time to store a value in that pointer but somehow it's not NULL anymore: clearly something has gone terribly wrong, either in your logic, or potentially in totally unrelated code in some other library which has scribbled on the heap. Then you return an error and let the caller deal with it. There is no justification, ever, for a library aborting the caller. Even in the scenario you present, it's still better to let the caller deal with the fallout. > If execution is allowed to continue, the resultant behaviour is totally undefined: you might corrupt data, or you might allow a permissions check to pass that should have failed, etc. So? that corrupted data or failed permissions check already happened before the library gets to abort anyway. Let the caller do whatever they can before the process aborts; don't abort for them before the caller has more context than the library does. If you abort without returning an error to the caller, the caller cannot do things like log "Hey, that previous permission check we allowed might have been allowed by accident". Your way provides absolutely no upside at all.
- burntsushi 2y ago
  > Then you return an error and let the caller deal with it. No, this is absurd. C libraries generally don't do this either. Instead, they might just have UB instead where as Rust tends to panic. So in practice, your suggestion ends up preferring vulnerabilities as a result of UB versus a DoS. See: https://news.ycombinator.com/item?id=43300120 https://news.ycombinator.com/item?id=43300120 Can you show me C libraries you've written that are used by others that follow your strategy of turning broken runtime invariants into error values?
  - lelanthran 2y ago
    > No, this is absurd. Just to be clear, you are making the argument that when a library call detects an error of the form of unexpected NULL/not-NULL, that they abort immediately? To be even more clear, I'm not making the argument that the program should proceed as normal after detecting an error (regardless of the type of error). That is not the argument that I am making which is why I find your "That's absurd" condescension extremely confusing.
imtringued 2y ago
The cultural differences between C developers and non-C developers never ceases to amaze me. The definition of unacceptable behaviour is so different. A program exit on an exploitable vulnerability is considered unacceptable. The program must continue running, even though all hope should have been lost by this point! Meanwhile on the other side of the ocean, it would be unacceptable for a program to enable the complete take over of a system! Getting the panics out of a rust codebase should much simpler than fuzzing out UB. After a few iterations, there won't be any panics whatsoever. Honestly what I'm seeing here is essentially that C is being preferred, because it lets you sweep the problem under the rug, because UB is such a diffuse concept with unusual consequences. Whereas a panic is concrete and requires immediate attention, attention that will make the program better in the long run.
- kazinator 2y ago
  Many (most?) C developers are also non-C developers.
vacuity 2y ago
I disagree. Just as C++ and Rust conceive of "plain old data" (POD) as data that basically has no surprising behaviors on cleanup (probably butchered that explanation), I think of libraries as "plain old code". Unlike services, libraries are part of the address space and should passively serve the main executable portion. That's why they are libraries specifically. A library should, if it detects an error, tell the main executable and then it is up to the main executable to do the responsible thing and address the error seriously. Needless to say, I don't like glibc-style "libraries". Doing too much in the background, in the same address space, no less. A library should be a nice, isolated unit that a programmer chooses to add into their executable at will. My concern is that composability and modularity aren't respected enough.
- GuB-42 2y ago
  I consider a panic to be equivalent to a segfault. It is a bug, either in the library itself, or unproper usage, and the system just stopped you from doing further damage. The only reasonable thing to do in this case is to fix the bug. Imagine you are catching the exception. What are you going to do next? From now on, anything can break, and how do you hope recovering if you couldn't do it right in the first place (that's what caused the panic)? You can have a panic handler, in the same way that you can trap SIGSEGV, for some debugging, but that's about it. If a crash is really problematic, use some process wrapper, and maybe a watchdog. Libraries don't work like that for performance and flexibility reasons, they share address space, but the downside is that if they crash, they take everything else with it.
  - vacuity 2y ago
    Indeed. It's in the name: libraries are there to be browsed, while services wait for customers to call on them (and do management and other things in the meantime). This should be understood among the developers of applications, libraries, and services, that libaries are functional add-ons while services are separate agents. A library has its expertise and should isolate bubble up application errors (to be fixed by the application) and environmental errors (can't really be fixed by anyone, but the application should decide how to proceed). If a library has internal errors, how is the library itself or the application supposed to fix it? It's like position-independent code: if its basic assumptions are met, it should be plug-in-play anywhere without complaint or knowledge.

anon-3988 2y ago

That behavior is up to the user. The library should only report the error.

burntsushi 2y ago
This is terrible advice, and people suggesting it should put their money where their mouth it and show real world examples. Which libraries in widespread use know how to detect all of their possible bugs due to invariant violations and report them as explicit error values? 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." Nevermind the fact that you're now potentially introducing an error channel into operations that should never actually error. That potentially has performance implications. Nevermind the fact that now your implementation details (internal runtime invariants are implementation details) have now leaked out into your API. Need a new internal runtime invariant? Now you need to add that fact to the API. Need to remove an invariant? Ah well, you still need to leave that possible error value in your API to avoid breaking your users.
- haberman 2y ago
  Ok, but let's consider the other extreme: a world where no library can ever assume a runtime invariant holds without dynamically checking it first. In this world, Vec::index() would need to perform not only a bounds check but also a check that the pointer is not NonNull::dangling(). Sure, RawVec is supposed to guarantee that the pointer will not be dangling when cap is >0, but RawVec could have a bug in it. I agree that documenting and returning a PtrWasDanglingError error is not good API design. An InternalError for all such cases seems more reasonable. But at some point we need to be able to assume that certain program invariants hold without checking at all (in a release build).
  - burntsushi 2y ago
    We don't have to live in the extreme though. That's one of the great advantages of Rust. :-) In `regex`, for example, there are certainly some cases where I use `unsafe` to elide those dynamic checks because 1) I couldn't do it in safe code and 2) I got a performance bump from it. But of all the dynamic checks in `regex`, this was an extremely small subset of them. And it makes sense to rely on abstractions like `RawVec` to uphold those guarantees. The point is that you're making that trade-off intentionally and for a specific reason (perf). The idea that I would support dogmatically always checking every runtime invariant everywhere is bonkers. :P In contrast, we have someone here who I responded to that is literally suggesting propagating every possible broken runtime invariant into a public API error value.
    - haberman 2y ago
      I am biased towards thinking that low-level libraries should generally avoid panic. That would mean that all invariants are either assumed true or returned as errors to the user. I think this is not an unreasonable design: it's how low-level C libraries are traditionally designed. For example SQLite does what I mentioned and has a single SQLITE_INTERNAL error that is documented as: > The SQLITE_INTERNAL result code indicates an internal malfunction. In a working version of SQLite, an application should never see this result code. If application does encounter this result code, it shows that there is a bug in the database engine. --https://www.sqlite.org/rescode.html#internal https://www.sqlite.org/rescode.html#internal I didn't mean to imply that you are for dogmatic checking of every runtime invariant, but the message that began that thread seems to advocate for that, going so far as to try to detect other buggy code that might have stomped on your memory.

theamk 2y ago

This logic only works for memory-unsafe languages like C or C++, where the checks are rare and there is a good chance that by the time abnormal condition is detected, things have been going wrong for a while. But that's not true for safe languages - in Rust, if you set a pointer to not-null, it won't end up as null (unless there is buggy unsafe, but let's ignore that). Instead, the panics are likely to be caused by logic errors. Take the decompressor buffer overflow errors author mentioned - the out-of-bounds writes were caused by a bug bit operations generating wrong array index. In Rust, this would be caught by bounds checker, which is good; but Rust would then abort process, which is bad. A hypothetical language that would throw an exception instead of panic'ing would be much better from library perspective - for example a web server might return 500 on that particular request, but it would stay running otherwise.

deleted 2y ago
[deleted]
chrismorgan 2y ago
Just one problem with your argument: we’re not talking about languages that have orderly null pointer exceptions. We’re expressly talking about languages like C, C++ and Rust. Your web server example is uncompelling, because a panic-based abort is not the only thing that can distress your system. The simplest example is if the library code doesn’t terminate, accidentally triggering an infinite loop. Or (better from some perspectives, worse from others) an infinite loop that allocates until you run out of memory, denying service until maybe an out-of-memory killer kills the process. In such scenarios, your system can easily end up in a state you didn’t write code expecting, where everything is just broken in mysterious ways. No, if you want your web server to be able to return an orderly 500 in as many situations of unforeseen errors as possible, the plain truth is that you need the code that will produce that to run on a different computer (approximate definition, though with some designs it may not quite need to be separate hardware), and that you’ll need various sorts of supervisors to catch deviant states and (attempt to) restore order. In short: for such an example, you already want tools that can abort a runaway process, so it’s actually not so abnormal for a library to be able to abort itself. There’s genuinely a lot to be said for deliberately aborting the entire process early, and handling problems from outside the process. It’s not without its drawbacks, but it is compelling for things like web servers. I would also note that, if you choose to, you can normally catch panics <https://doc.rust-lang.org/std/panic/fn.catch_unwind.html https://doc.rust-lang.org/std/panic/fn.catch_unwind.html>, and Rust web servers tend to do this.

pwdisswordfishz 2y ago

> the only sensible thing to do is abort the process Abort whatever depends on the invariant, which may be less than the whole process.

int_19h 2y ago
Yeah, but you need to ensure that you have an actual isolation boundary in place between systems with different invariants to ensure that there's no shared state (since any shared state implies shared invariants). Which is exactly what processes are. Not necessarily OS processes, though - e.g. consider Erlang, where the notion of an ultra-lightweight process provided by the runtime instead is exactly what makes this kind of error handling a natural fit. Now for some languages it is possible to determine that the state strictly through code analysis, without any runtime boundary enforcement. And I think that safe Rust might be in that category, but unsafe Rust definitely isn't - and whether any given library contains unsafe code is an implementation detail...

lelanthran 2y ago

> If an invariant (something we believe absolutely must be true) is violated, the only sensible thing to do is abort the process. For example, if you set a pointer to NULL and then half an hour later it's time to store a value in that pointer but somehow it's not NULL anymore: clearly something has gone terribly wrong, either in your logic, or potentially in totally unrelated code in some other library which has scribbled on the heap. Then you return an error and let the caller deal with it. There is no justification, ever, for a library aborting the caller. Even in the scenario you present, it's still better to let the caller deal with the fallout. > If execution is allowed to continue, the resultant behaviour is totally undefined: you might corrupt data, or you might allow a permissions check to pass that should have failed, etc. So? that corrupted data or failed permissions check already happened before the library gets to abort anyway. Let the caller do whatever they can before the process aborts; don't abort for them before the caller has more context than the library does. If you abort without returning an error to the caller, the caller cannot do things like log "Hey, that previous permission check we allowed might have been allowed by accident". Your way provides absolutely no upside at all.

burntsushi 2y ago
> Then you return an error and let the caller deal with it. No, this is absurd. C libraries generally don't do this either. Instead, they might just have UB instead where as Rust tends to panic. So in practice, your suggestion ends up preferring vulnerabilities as a result of UB versus a DoS. See: https://news.ycombinator.com/item?id=43300120 https://news.ycombinator.com/item?id=43300120 Can you show me C libraries you've written that are used by others that follow your strategy of turning broken runtime invariants into error values?
- lelanthran 2y ago
  > No, this is absurd. Just to be clear, you are making the argument that when a library call detects an error of the form of unexpected NULL/not-NULL, that they abort immediately? To be even more clear, I'm not making the argument that the program should proceed as normal after detecting an error (regardless of the type of error). That is not the argument that I am making which is why I find your "That's absurd" condescension extremely confusing.

imtringued 2y ago

The cultural differences between C developers and non-C developers never ceases to amaze me. The definition of unacceptable behaviour is so different. A program exit on an exploitable vulnerability is considered unacceptable. The program must continue running, even though all hope should have been lost by this point! Meanwhile on the other side of the ocean, it would be unacceptable for a program to enable the complete take over of a system! Getting the panics out of a rust codebase should much simpler than fuzzing out UB. After a few iterations, there won't be any panics whatsoever. Honestly what I'm seeing here is essentially that C is being preferred, because it lets you sweep the problem under the rug, because UB is such a diffuse concept with unusual consequences. Whereas a panic is concrete and requires immediate attention, attention that will make the program better in the long run.

kazinator 2y ago
Many (most?) C developers are also non-C developers.

vacuity 2y ago

I disagree. Just as C++ and Rust conceive of "plain old data" (POD) as data that basically has no surprising behaviors on cleanup (probably butchered that explanation), I think of libraries as "plain old code". Unlike services, libraries are part of the address space and should passively serve the main executable portion. That's why they are libraries specifically. A library should, if it detects an error, tell the main executable and then it is up to the main executable to do the responsible thing and address the error seriously. Needless to say, I don't like glibc-style "libraries". Doing too much in the background, in the same address space, no less. A library should be a nice, isolated unit that a programmer chooses to add into their executable at will. My concern is that composability and modularity aren't respected enough.

GuB-42 2y ago
I consider a panic to be equivalent to a segfault. It is a bug, either in the library itself, or unproper usage, and the system just stopped you from doing further damage. The only reasonable thing to do in this case is to fix the bug. Imagine you are catching the exception. What are you going to do next? From now on, anything can break, and how do you hope recovering if you couldn't do it right in the first place (that's what caused the panic)? You can have a panic handler, in the same way that you can trap SIGSEGV, for some debugging, but that's about it. If a crash is really problematic, use some process wrapper, and maybe a watchdog. Libraries don't work like that for performance and flexibility reasons, they share address space, but the downside is that if they crash, they take everything else with it.
- vacuity 2y ago
  Indeed. It's in the name: libraries are there to be browsed, while services wait for customers to call on them (and do management and other things in the meantime). This should be understood among the developers of applications, libraries, and services, that libaries are functional add-ons while services are separate agents. A library has its expertise and should isolate bubble up application errors (to be fixed by the application) and environmental errors (can't really be fixed by anyone, but the application should decide how to proceed). If a library has internal errors, how is the library itself or the application supposed to fix it? It's like position-independent code: if its basic assumptions are met, it should be plug-in-play anywhere without complaint or knowledge.