Re: [PATCH RESEND] bug: When !CONFIG_BUG, simplify WARN_ON_ONCE and family

[Arnd Bergmann <arnd@arndb.de>]

On Monday 24 February 2014 12:09:11 David Howells wrote:
> Josh Triplett <josh@joshtriplett.org> wrote:
> 
> > > This means we actually want BUG() to end with __builtin_unreachable()
> > > as in the CONFIG_BUG=y case, and also ensure it actually is
> > > unreachable. As I have shown in [1], the there is a small overhead
> > > of doing this in terms of code size.
> > 
> > I agree that allowing BUG() to become a no-op seems suboptimal, if only
> > because of the resulting warnings and mis-optimizations.  However, I
> > think the overhead could be cut down massively, such that BUG() just
> > compiles down to a one-byte undefined instruction.  (__builtin_trap()
> > might do the right thing here; worth checking.)
> 
> Is it possible to use an inline function with an empty body in this?  Let the
> compiler prune away all the arguments?
> 

I think you are thinking of a different problem. BUG() already takes no
arguments, but we get warnings for code like this:

int f(void)
{
	switch (global) {
	case 0:
		return 0;
	case 1:
		return do_something();
	default:
		BUG();
	}
}

BUG() normally causes a fault and we print helpful messages before killing
the task, and gcc knows we never continue because of the
__builtin_unreachable() annotation.

If BUG() is defined as 'do { } while (0)' in the example above, we get
a warning because the function may end without returning a number.
If we define it to 'do { unreachable(); } while (0)', we don't get a
warning, but we can get undefined behavior in the case we ever get to
the end of the function.

__builtin_trap() would be nice as an architecture independent method,
but I just checked what it does on ARM, and it just calls abort(),
which is probably not what we want in the kernel, especially as long
as the ARM abort() function calls BUG(), and most architectures don't
define it at all.

'do { } while (1)' is probably a reasonably generic way to not return
and should compile into a single instruction on most architectures.
panic() would be another option to do something that won't cause
additional data corruption, but it's a varargs function, so that won't
help you much when you just try to save object code size.

	Arnd