Re: [PATCH 4/4] bug: mark disabled BUG() as unreachable() code

[Linus Torvalds <torvalds@linux-foundation.org>]

On Fri, Apr 27, 2012 at 10:10 PM, Konstantin Khlebnikov
<khlebnikov@openvz.org> wrote:
>
> With this patch gcc throw out loop at the end of do_exit():

I think that's the least of our problems - that loop only exists to
get rid of a warning, and the return from that schedule() really is
supposed to be unreachable().

What makes me much more worried is that iirc, gcc will use
"unreachable()" to also get rid of function epilogues etc, which is
perfectly fine if it really is something entirely unreachable. But the
kernel use of BUG() is often as a kind of "assert()", and if gcc
generates actively bad code for it (and it does, with unreachable()),
it turns the turned-off BUG() into something *really* horrible that
makes for a debugging disaster.

So some pattern like

   if (badness)
      BUG();

would generate code that is actively insane, instead of (like now)
generating basically a no-op.

And THAT makes me go "Eww". "unreachable()" should only be used in
situations that really are very very unreachable() (eg after an "asm"
that goes off to la-la-land or a call to "exit()" in user land etc).

The kernel kinds of "assert" is *hopefully* not reachable, but if we
ever reach it, we don't want to make things worse. The BUG() was there
to give us a nicer debug message about what went wrong, and I think
your patch actively destroys that whole thing and makes for something
*worse*.

                    Linus