lkml.org 
[lkml]   [2013]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n
On 09/30/2013 07:56 AM, Ralf Baechle wrote:
> Lately I received several patches for build issues that only strike if
> CONFIG_BUG is disabled. Here's a test case extracted from one of them:
>
> /*
> * Definition of BUG taken from asm-generic/bug.h for the CONFIG_BUG=n case
> */
> #define BUG() do {} while(0)
>
> int foo(int arg)
> {
> int res;
>
> if (arg == 1)
> res = 23;
> else if (arg == 2)
> res = 42;
> else
> BUG();
>
> return res;
> }
>
> [ralf@h7 ~]$ gcc -O2 -Wall -c bug.c
> bug.c: In function ‘foo’:
> bug.c:17:2: warning: ‘res’ may be used uninitialized in this function [-Wmaybe-uninitialized]
> return res;
> ^
>
> It's fairly obvious to see what's happening here - GCC doesn't know that
> the else case can not be reached, thus razorsharply concludes that res
> may be used uninitialized.
>
> There several locations where MIPS - possibly other architectures as well -
> is affected by this.
>
> I think the definition of BUG should be changed to something like
>
> #define BUG() unreachable()
> 16304
> unreachable() will depending on the compiler being used, expand either
> into a call to __builtin_unreachable() or where that function is
> unavailable, into do {} while (1).

The *only* reason we have CONFIG_BUG=n is to reduce code size.

Sticking in that empty loop, negates the entire point.

IMHO: We should do one of:
o Make CONFIG_BUG=y mandatory
o Ignore the warnings.
o Fix the warning sites so they quit Warning.

So I don't think the patch is really an improvement over the status quo.

David Daney

>
> __builtin_unreachable() was introduce for GCC 4.5.0.
>
> This means there'd be minor bloat for antique compilers - but probably
> even better code generation for compilers supporting __builtin_unreachable().
>
> Ralf
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
>
> include/asm-generic/bug.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 7d10f96..6f78771 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -108,7 +108,7 @@ extern void warn_slowpath_null(const char *file, const int line);
>
> #else /* !CONFIG_BUG */
> #ifndef HAVE_ARCH_BUG
> -#define BUG() do {} while(0)
> +#define BUG() unreachable()
> #endif
>
> #ifndef HAVE_ARCH_BUG_ON
>
> ----- End forwarded message -----
>
> Ralf
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2013-09-30 18:01    [W:0.060 / U:0.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site