5 ms·

Why not exit/panic at that point? Seems like the right thing for any “unreachable” code.

• Eridrus 6y ago
  Ego.
  • kevincox 6y ago
    If you are really sure put __builtin_unreachable()
    • pvarangot 6y ago
      I'm pretty sure he didn't have that on most of the platforms he was targeting with his portable C code, it's not even there on some of them now.
      • jstimpfle 6y ago
        what you do is you define a macro in a common header. Like UNREACHABLE().
    • jstimpfle 6y ago
      Shoot I have done that in the past but it seems I actually wanted __builtin_trap().
      • kevincox 6y ago
        Yeah, that footgun should really have a more explicit name. __dear_compiler_i_promise_that_this_statement_is_unreachable_and_you_may_optimize_based_on_my_good_word_upon_penalty_of_undefined_behaviour()
        • derefr 6y ago
          I mean, what other semantics would anyone expect it to have—presuming they think for a moment about why it exists as its own thing in the first place?
          • dodobirdlord 6y ago
            Rust being Rust, the equivalent causes a safe teardown. https://doc.rust-lang.org/std/macro.unreachable.html https://doc.rust-lang.org/std/macro.unreachable.html The UB causing equivalent is of course unsafe. https://doc.rust-lang.org/std/hint/fn.unreachable_unchecked.html https://doc.rust-lang.org/std/hint/fn.unreachable_unchecked....
          • jstimpfle 6y ago
            Ok so what's the answer if you apply the question to __builtin_trap()?
        • saagarjha 6y ago
          Even better, just actually perform undefined behavior on that branch and the compiler will get rid of it.
          • kevincox 6y ago
            This is literally what __builtin_unreachable() does. But it has the added benefit of the compiler understanding the intent so it works well with diagnostics.
            • saagarjha 6y ago
              But that's a builtin, rather than something that's part of the core language :)
• badrabbit 6y ago
  Because it will never happen I guess. From my experience, it's because of this bad habbit of expecting the program to crash and provide a backtrace,the print is there for debugging. But you're right,it's not the sane way of doing it.
• im3w1l 6y ago
  Didn't think of it? If I saw a line like that I'd be grateful that it printed at all.
• asveikau 6y ago
  I could see an argument (similar to for assert()) to omit such a check in most production builds. Maybe you could enable it for a small amount of hosts, so that if it does happen with any frequency, somebody sees it somewhere. But not every "unthinkable" case being reached represents a security problem as it does in this case. So in the general case of how to approach such assert-type checks, an abort might take down the process when not necessary, turning it into a DoS. Edit: I am not sure if my friend the downvoter realizes I am not trying to justify legit security bugs, just talking abstractly about varying approaches to handling asserts for unforeseen or "allegedly impossible" circumstances.
  • kstrauser 6y ago
    I didn't downvote you and I interpreted your comment the way you said you meant it. I do happen to disagree, though. For a project in a space that's notoriously vulnerability prone - I mean, when it was released it was competing with Sendmail - it seems very reasonably cautious to pepper the code with panic handlers in "impossible" places. They won't slow the code down any because they should never be evaluated, but give nice, noisy explosions when unexpected things happen. There's no downside to doing this.
    • chasil 6y ago
      Solaris also notably bundles 32-bit binaries for most everything in /bin with 64-bit distributions, and that begins to look like a wise choice. I recently observed that SmartOS does the same.
  • tedunangst 6y ago
    If we could tell a priori which unreachable cases were reachable it wouldn't be such a problem. Asserting only in debug builds is rather ineffective because the unthinkable input isn't in your testsuite. If it were, then it wouldn't have been unthinkable.
    • asveikau 6y ago
      > Asserting only in debug builds Right, but I am also suggesting another approach: if you have a large fleet of machines or a client app with a lot of usage, you can enable it in a small percentage of runs and if it hits in the wild you still have a chance of learning about it, without tearing down the process for all users. I have seen that strategy be effective. Very hard to know which behavior is appropriate in advance though. An assert that turns out to represent a benign circumstance that is already handled, multiplied by large numbers of machines, can be pretty annoying.