Warning masked by BUG() when CONFIG_BUG is enabled

Warning masked by BUG() when CONFIG_BUG is enabled

[Arnaud Ebalard]

Hi,

While compiling a kernel, I got the following warnings which are due to
missing returns in the following functions:

  CC      fs/xattr.o
  CC      crypto/cryptd.o
  CC      lib/irq_regs.o
  CC      lib/is_single_threaded.o
  CC      lib/klist.o
drivers/gpio/gpio-mvebu.c: In function ‘mvebu_gpioreg_edge_mask’:
drivers/gpio/gpio-mvebu.c:148:1: warning: control reaches end of non-void function [-Wreturn-type]
drivers/gpio/gpio-mvebu.c: In function ‘mvebu_gpioreg_edge_cause’:
drivers/gpio/gpio-mvebu.c:130:1: warning: control reaches end of non-void function [-Wreturn-type]
drivers/gpio/gpio-mvebu.c: In function ‘mvebu_gpioreg_level_mask’:
drivers/gpio/gpio-mvebu.c:166:1: warning: control reaches end of non-void function [-Wreturn-type]
  CC      lib/kobject.o
  CC      lib/kobject_uevent.o
  CC      lib/md5.o
  CC      lib/percpu-refcount.o

I was kind of curious not to have noticed it during kernel builds for
armada 370/xp targets. The reason is the following: on my Armada 370/XP
builds, I had CONFIG_BUG=y which makes BUG() call panic() (which never
returns). Otherwise, BUG() is a nop and the 'default' branch below is no
more a catch-all barrier in previous functions. For the discussion,
those have the same structure:

static inline void __iomem *mvebu_gpioreg_edge_cause(...)
{
	int cpu;

	switch (mvchip->soc_variant) {
	case MVEBU_GPIO_SOC_VARIANT_ORION:
	case MVEBU_GPIO_SOC_VARIANT_MV78200:
		return sth;
	case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
		return sth;
	default:
		BUG();
	}
}

*Out of curiosity*, are there no generic solutions to handle such a case?
In the end, if BUG() has been put here on purpose (i.e. this is the only
solution, which it kinds of looks like here) wouldn't it be more logical
to call panic() here directly?

As a side note, I noticed quite some instances of that pattern (switch
statement w/ a 'default' calling BUG()) while more carefully looking at
compilation output. I may get slapped for this, but just in case this
is indeed something which deserves to be fixed and can be fixed
automagically via some semantic patch, I have CC'ed Coccinelle people.

Cheers,

a+
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Warning masked by BUG() when CONFIG_BUG is enabled

[Russell King - ARM Linux]

On Sat, Nov 16, 2013 at 11:52:08PM +0100, Arnaud Ebalard wrote:
> I was kind of curious not to have noticed it during kernel builds for
> armada 370/xp targets. The reason is the following: on my Armada 370/XP
> builds, I had CONFIG_BUG=y which makes BUG() call panic() (which never
> returns).

You're not the first to spot this, and you won't be the last.

Some very experienced kernel hackers have tried to get this fixed and
failed.  It seems people actually want the CPU to fall through the
BUG() sites when people disable CONFIG_BUG - which I think is idiotic.

Arnd (and myself) have worked on this problem, and we came up with a
very nice solution which didn't increase the size of the kernel and
didn't make things unsafe.  However, it went nowhere.

It's pointless trying to get this fixed - it's just a complete waste of
time because of politics.  Find something else to attack.  Just ensure
you always have CONFIG_BUG enabled if you want a system which will
produce some kind of report when one of these sites gets hit.

Re: Warning masked by BUG() when CONFIG_BUG is enabled

[Arnaud Ebalard]

Hi Russell,

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Sat, Nov 16, 2013 at 11:52:08PM +0100, Arnaud Ebalard wrote:
>> I was kind of curious not to have noticed it during kernel builds for
>> armada 370/xp targets. The reason is the following: on my Armada 370/XP
>> builds, I had CONFIG_BUG=y which makes BUG() call panic() (which never
>> returns).
>
> You're not the first to spot this, and you won't be the last.
>
> Some very experienced kernel hackers have tried to get this fixed and
> failed.  It seems people actually want the CPU to fall through the
> BUG() sites when people disable CONFIG_BUG - which I think is idiotic.
>
> Arnd (and myself) have worked on this problem, and we came up with a
> very nice solution which didn't increase the size of the kernel and
> didn't make things unsafe.  However, it went nowhere.
>
> It's pointless trying to get this fixed - it's just a complete waste of
> time because of politics.  Find something else to attack.  Just ensure
> you always have CONFIG_BUG enabled if you want a system which will
> produce some kind of report when one of these sites gets hit.

Understood. Thanks for the explanation.

Cheers,

a+

Re: [Cocci] Warning masked by BUG() when CONFIG_BUG is enabled

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]


> As a side note, I noticed quite some instances of that pattern (switch
> statement w/ a 'default' calling BUG()) while more carefully looking at
> compilation output. I may get slapped for this, but just in case this
> is indeed something which deserves to be fixed and can be fixed
> automagically via some semantic patch, I have CC'ed Coccinelle people.

I don't think this can be fixed in a generic way. This driver, for
example, should IMO return -EINVAL when hitting default in probe and
simply get rid of the other BUG() calls. No need to halt the kernel
because of a broken match->data.


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Cocci] Warning masked by BUG() when CONFIG_BUG is enabled

[Julia Lawall]

> > As a side note, I noticed quite some instances of that pattern (switch
> > statement w/ a 'default' calling BUG()) while more carefully looking at
> > compilation output. I may get slapped for this, but just in case this
> > is indeed something which deserves to be fixed and can be fixed
> > automagically via some semantic patch, I have CC'ed Coccinelle people.
> 
> I don't think this can be fixed in a generic way. This driver, for
> example, should IMO return -EINVAL when hitting default in probe and
> simply get rid of the other BUG() calls. No need to halt the kernel
> because of a broken match->data.

One could nevertheless perhaps use Coccinelle to find them, if there is 
some possibility to fix them.

julia