linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
@ 2010-11-17  3:45 Arnaud Lacombe
  2010-11-17  4:07 ` Matt Mackall
  2010-11-17  4:13 ` Grant Likely
  0 siblings, 2 replies; 6+ messages in thread
From: Arnaud Lacombe @ 2010-11-17  3:45 UTC (permalink / raw)
  To: Matt Mackall, linux-kernel
  Cc: dirk.brandewie, linux-arch, linux-kbuild, Grant Likely

Hi,

[CC: list reduced as starting a new thread, most on the context
removed as this concern a different issue.]

On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
>> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>> [...]
> The kernel needs to complain *loudly* if this occurs because it
> represents a bug.  I'm tempted to say use BUG, but that would halt the
> kernel and prevent any possibility of kernel log output.
> [...]
does it ? if CONFIG_BUG is not enabled and the arch has no define for
it, the default does _nothing_:

from `include/asm-generic/bug.h':

#else /* !CONFIG_BUG */
#ifndef HAVE_ARCH_BUG
#define BUG() do {} while(0)
#endif

#ifndef HAVE_ARCH_BUG_ON
#define BUG_ON(condition) do { if (condition) ; } while(0)
#endif
[...]

gcc is triggering about ~30 warnings (like [0]) on code path using
BUG(). Most of these path assume BUG() will never return, which is not
true.

The commit message introducing this behavior state:

commit c8538a7aa5527d02c7191ac5da124efadf6a2827
Author: Matt Mackall <mpm@selenic.com>
Date:   Sun May 1 08:59:01 2005 -0700

    [PATCH] remove all kernel BUGs

    This patch eliminates all kernel BUGs, trims about 35k off the typical
    kernel, and makes the system slightly faster.

    Signed-off-by: Matt Mackall <mpm@selenic.com>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[...]

+config BUG
+       bool "BUG() support" if EMBEDDED
+       default y
+       help
+          Disabling this option eliminates support for BUG and WARN, reducing
+          the size of your kernel image and potentially quietly ignoring
+          numerous fatal conditions. You should only consider disabling this
+          option for embedded systems with no facilities for reporting errors.

So how should BUG() been used ?
 1) assuming it will never _ever_ return ?
 2) assuming it may return ?

if (1), its definition has to be changed, if (2) a lot of path have to be fixed.

Thanks in advance,
 - Arnaud

[0]: fs/dcache.c:1899:1: warning: control reaches end of non-void
function, line match an old revision, but the BUG() is still there.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
  2010-11-17  3:45 Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string] Arnaud Lacombe
@ 2010-11-17  4:07 ` Matt Mackall
  2010-11-17  4:20   ` Arnaud Lacombe
  2010-11-17  4:13 ` Grant Likely
  1 sibling, 1 reply; 6+ messages in thread
From: Matt Mackall @ 2010-11-17  4:07 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: linux-kernel, dirk.brandewie, linux-arch, linux-kbuild,
	Grant Likely

On Tue, 2010-11-16 at 22:45 -0500, Arnaud Lacombe wrote:
> Hi,
> 
> [CC: list reduced as starting a new thread, most on the context
> removed as this concern a different issue.]
> 
> On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> > On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
> >> From: Dirk Brandewie <dirk.brandewie@gmail.com>
> >> [...]
> > The kernel needs to complain *loudly* if this occurs because it
> > represents a bug.  I'm tempted to say use BUG, but that would halt the
> > kernel and prevent any possibility of kernel log output.
> > [...]
> does it ? if CONFIG_BUG is not enabled and the arch has no define for
> it, the default does _nothing_:

That's because CONFIG_BUG=N is intended for machines where logging is
irrelevant/impossible. You are encouraged to ignore all options under
CONFIG_EMBEDDED when trying to have a sensible discussion.

-- 
Mathematics is the supreme nostalgia of our time.



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
  2010-11-17  3:45 Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string] Arnaud Lacombe
  2010-11-17  4:07 ` Matt Mackall
@ 2010-11-17  4:13 ` Grant Likely
  2010-11-17  4:29   ` Arnaud Lacombe
  1 sibling, 1 reply; 6+ messages in thread
From: Grant Likely @ 2010-11-17  4:13 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: Matt Mackall, linux-kernel, dirk.brandewie, linux-arch,
	linux-kbuild

On Tue, Nov 16, 2010 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> [CC: list reduced as starting a new thread, most on the context
> removed as this concern a different issue.]
>
> On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
>>> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>>> [...]
>> The kernel needs to complain *loudly* if this occurs because it
>> represents a bug.  I'm tempted to say use BUG, but that would halt the
>> kernel and prevent any possibility of kernel log output.
>> [...]
> does it ? if CONFIG_BUG is not enabled and the arch has no define for
> it, the default does _nothing_:
>
> from `include/asm-generic/bug.h':
>
> #else /* !CONFIG_BUG */
> #ifndef HAVE_ARCH_BUG
> #define BUG() do {} while(0)
> #endif
>
> #ifndef HAVE_ARCH_BUG_ON
> #define BUG_ON(condition) do { if (condition) ; } while(0)
> #endif
> [...]
>
> gcc is triggering about ~30 warnings (like [0]) on code path using
> BUG(). Most of these path assume BUG() will never return, which is not
> true.

As far as I know, BUG() is not supposed to return.  Period.  The patch
below is part of the linux-tiny work, and should only ever be used on
embedded systems where small size is more important than debugability.

g.

>
> The commit message introducing this behavior state:
>
> commit c8538a7aa5527d02c7191ac5da124efadf6a2827
> Author: Matt Mackall <mpm@selenic.com>
> Date:   Sun May 1 08:59:01 2005 -0700
>
>    [PATCH] remove all kernel BUGs
>
>    This patch eliminates all kernel BUGs, trims about 35k off the typical
>    kernel, and makes the system slightly faster.
>
>    Signed-off-by: Matt Mackall <mpm@selenic.com>
>    Signed-off-by: Andrew Morton <akpm@osdl.org>
>    Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>
> [...]
>
> +config BUG
> +       bool "BUG() support" if EMBEDDED
> +       default y
> +       help
> +          Disabling this option eliminates support for BUG and WARN, reducing
> +          the size of your kernel image and potentially quietly ignoring
> +          numerous fatal conditions. You should only consider disabling this
> +          option for embedded systems with no facilities for reporting errors.
>
> So how should BUG() been used ?
>  1) assuming it will never _ever_ return ?
>  2) assuming it may return ?
>
> if (1), its definition has to be changed, if (2) a lot of path have to be fixed.

The answer is 1).

g.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
  2010-11-17  4:07 ` Matt Mackall
@ 2010-11-17  4:20   ` Arnaud Lacombe
  0 siblings, 0 replies; 6+ messages in thread
From: Arnaud Lacombe @ 2010-11-17  4:20 UTC (permalink / raw)
  To: Matt Mackall
  Cc: linux-kernel, dirk.brandewie, linux-arch, linux-kbuild,
	Grant Likely

Hi,

On Tue, Nov 16, 2010 at 11:07 PM, Matt Mackall <mpm@selenic.com> wrote:
> On Tue, 2010-11-16 at 22:45 -0500, Arnaud Lacombe wrote:
>> Hi,
>>
>> [CC: list reduced as starting a new thread, most on the context
>> removed as this concern a different issue.]
>>
>> On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>> > On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
>> >> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>> >> [...]
>> > The kernel needs to complain *loudly* if this occurs because it
>> > represents a bug.  I'm tempted to say use BUG, but that would halt the
>> > kernel and prevent any possibility of kernel log output.
>> > [...]
>> does it ? if CONFIG_BUG is not enabled and the arch has no define for
>> it, the default does _nothing_:
>
> That's because CONFIG_BUG=N is intended for machines where logging is
> irrelevant/impossible.
>
Yes, but it is still a no-op, where the original programmer made it
clear he did not want the kernel to go further.

> You are encouraged to ignore all options under
> CONFIG_EMBEDDED when trying to have a sensible discussion.
>
Unless it creates a situation where it does the opposite of what the
intended behavior, I would guess.

 - Arnaud

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
  2010-11-17  4:13 ` Grant Likely
@ 2010-11-17  4:29   ` Arnaud Lacombe
  2010-11-17  4:44     ` Matt Mackall
  0 siblings, 1 reply; 6+ messages in thread
From: Arnaud Lacombe @ 2010-11-17  4:29 UTC (permalink / raw)
  To: Grant Likely
  Cc: Matt Mackall, linux-kernel, dirk.brandewie, linux-arch,
	linux-kbuild

Hi,

On Tue, Nov 16, 2010 at 11:13 PM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Tue, Nov 16, 2010 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> Hi,
>>
>> [CC: list reduced as starting a new thread, most on the context
>> removed as this concern a different issue.]
>>
>> On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
>>>> From: Dirk Brandewie <dirk.brandewie@gmail.com>
>>>> [...]
>>> The kernel needs to complain *loudly* if this occurs because it
>>> represents a bug.  I'm tempted to say use BUG, but that would halt the
>>> kernel and prevent any possibility of kernel log output.
>>> [...]
>> does it ? if CONFIG_BUG is not enabled and the arch has no define for
>> it, the default does _nothing_:
>>
>> from `include/asm-generic/bug.h':
>>
>> #else /* !CONFIG_BUG */
>> #ifndef HAVE_ARCH_BUG
>> #define BUG() do {} while(0)
>> #endif
>>
>> #ifndef HAVE_ARCH_BUG_ON
>> #define BUG_ON(condition) do { if (condition) ; } while(0)
>> #endif
>> [...]
>>
>> gcc is triggering about ~30 warnings (like [0]) on code path using
>> BUG(). Most of these path assume BUG() will never return, which is not
>> true.
>
> As far as I know, BUG() is not supposed to return.  Period.
>
but the code I pointed out _do_ return.

> The patch
> below is part of the linux-tiny work, and should only ever be used on
> embedded systems where small size is more important than debugability.
>
AFAIK, this is not precised anywhere, but I may not have search enough.

Matt, any reason the generic code does not just spin (or OOPS) and
marked __noreturn in any case ?

 - Arnaud

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string]
  2010-11-17  4:29   ` Arnaud Lacombe
@ 2010-11-17  4:44     ` Matt Mackall
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Mackall @ 2010-11-17  4:44 UTC (permalink / raw)
  To: Arnaud Lacombe
  Cc: Grant Likely, linux-kernel, dirk.brandewie, linux-arch,
	linux-kbuild

On Tue, 2010-11-16 at 23:29 -0500, Arnaud Lacombe wrote:
> Hi,
> 
> On Tue, Nov 16, 2010 at 11:13 PM, Grant Likely
> <grant.likely@secretlab.ca> wrote:
> > On Tue, Nov 16, 2010 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> >> Hi,
> >>
> >> [CC: list reduced as starting a new thread, most on the context
> >> removed as this concern a different issue.]
> >>
> >> On Tue, Nov 16, 2010 at 7:16 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> >>> On Tue, Nov 16, 2010 at 3:41 PM,  <dirk.brandewie@gmail.com> wrote:
> >>>> From: Dirk Brandewie <dirk.brandewie@gmail.com>
> >>>> [...]
> >>> The kernel needs to complain *loudly* if this occurs because it
> >>> represents a bug.  I'm tempted to say use BUG, but that would halt the
> >>> kernel and prevent any possibility of kernel log output.
> >>> [...]
> >> does it ? if CONFIG_BUG is not enabled and the arch has no define for
> >> it, the default does _nothing_:
> >>
> >> from `include/asm-generic/bug.h':
> >>
> >> #else /* !CONFIG_BUG */
> >> #ifndef HAVE_ARCH_BUG
> >> #define BUG() do {} while(0)
> >> #endif
> >>
> >> #ifndef HAVE_ARCH_BUG_ON
> >> #define BUG_ON(condition) do { if (condition) ; } while(0)
> >> #endif
> >> [...]
> >>
> >> gcc is triggering about ~30 warnings (like [0]) on code path using
> >> BUG(). Most of these path assume BUG() will never return, which is not
> >> true.
> >
> > As far as I know, BUG() is not supposed to return.  Period.
> >
> but the code I pointed out _do_ return.
> 
> > The patch
> > below is part of the linux-tiny work, and should only ever be used on
> > embedded systems where small size is more important than debugability.
> >
> AFAIK, this is not precised anywhere, but I may not have search enough.

Read the help for CONFIG_BUG and CONFIG_EMBEDDED.

> Matt, any reason the generic code does not just spin (or OOPS) and
> marked __noreturn in any case ?

Yes. Spinning and oopsing take space. Function calls take space. The
whole damn point of this option is to say is _we don't care about BUGs,
we care about space_. In other words, this code is a hack and is marked
as such. Arguing about "correctness" here is a waste of time, it's
intentionally not correct.

I seem to recall attempting to fool GCC with a __noreturn and failing.

-- 
Mathematics is the supreme nostalgia of our time.



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-11-17  4:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-17  3:45 Behavior of BUG() [Was: Re: [PATCH 2/5] of/fdt: add kernel command line option for dtb_compat string] Arnaud Lacombe
2010-11-17  4:07 ` Matt Mackall
2010-11-17  4:20   ` Arnaud Lacombe
2010-11-17  4:13 ` Grant Likely
2010-11-17  4:29   ` Arnaud Lacombe
2010-11-17  4:44     ` Matt Mackall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).