Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

On 09/30/2013 12:03 PM, Geert Uytterhoeven wrote:

On Mon, Sep 30, 2013 at 7:45 PM, David Daney  wrote:

What about using __builtin_unreachable when we can but turn off warnings
and use do{}while(0) when __builtin_unreachable does not exist?  This seems
the both worlds.  Newer compilers produce better code with unreachable
anyways.



Simply not true.

do{}while(0) is a NOP it is no more useful than an ';' statement.  It
doesn't serve as a magic uninitialized variable hiding mechanism.


You missed the "turn off warnings" part of the "and".


You are correct, I did miss it.

The real problem here is that the kernel is written to expect that BUG() 
never returns.  Any implementation that has BUG() return, is almost 
certainly *not* what we want.


But wieh people select CONFIG_BUG=n they expect the smallest possible code.

These two criteria are mutually exclusive, so something should change.

It is not just the uninitialized variable warning, there can be others 
as well (control reaching the end of a non-void function comes to mind). 
 So I don't think turning off the warnings is a good solution.


That leaves:

1) Remove CONFIG_BUG and make it unconditionally enabled.

2) Make CONFIG_BUG=n imply "static inline void BUG(void){do{}while(1);}" 
which might be bigger than with CONFIG_BUG=y


David Daney




Gr{oetje,eeting}s,

 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
 -- Linus Torvalds




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


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Geert Uytterhoeven
On Mon, Sep 30, 2013 at 7:45 PM, David Daney  wrote:
>> What about using __builtin_unreachable when we can but turn off warnings
>> and use do{}while(0) when __builtin_unreachable does not exist?  This seems
>> the both worlds.  Newer compilers produce better code with unreachable
>> anyways.
>>
>
> Simply not true.
>
> do{}while(0) is a NOP it is no more useful than an ';' statement.  It
> doesn't serve as a magic uninitialized variable hiding mechanism.

You missed the "turn off warnings" part of the "and".

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

On 09/30/2013 10:15 AM, Pinski, Andrew wrote:

On Sep 30, 2013, at 9:20 AM, "David Daney"  wrote:





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.


What about using __builtin_unreachable when we can but turn off warnings and 
use do{}while(0) when __builtin_unreachable does not exist?  This seems the 
both worlds.  Newer compilers produce better code with unreachable anyways.



Simply not true.

do{}while(0) is a NOP it is no more useful than an ';' statement.  It 
doesn't serve as a magic uninitialized variable hiding mechanism.


David Daney



Thanks,
Andrew




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 

  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Pinski, Andrew
> On Sep 30, 2013, at 9:20 AM, "David Daney"  wrote:

> 
>> 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.

What about using __builtin_unreachable when we can but turn off warnings and 
use do{}while(0) when __builtin_unreachable does not exist?  This seems the 
both worlds.  Newer compilers produce better code with unreachable anyways.

Thanks,
Andrew


> 
> 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 
>> 
>>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

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 

  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Ralf Baechle
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()

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).

__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 

 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Ralf Baechle
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()

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).

__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 r...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

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 r...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Pinski, Andrew
 On Sep 30, 2013, at 9:20 AM, David Daney ddaney.c...@gmail.com wrote:

 
 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.

What about using __builtin_unreachable when we can but turn off warnings and 
use do{}while(0) when __builtin_unreachable does not exist?  This seems the 
both worlds.  Newer compilers produce better code with unreachable anyways.

Thanks,
Andrew


 
 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 r...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

On 09/30/2013 10:15 AM, Pinski, Andrew wrote:

On Sep 30, 2013, at 9:20 AM, David Daney ddaney.c...@gmail.com wrote:





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.


What about using __builtin_unreachable when we can but turn off warnings and 
use do{}while(0) when __builtin_unreachable does not exist?  This seems the 
both worlds.  Newer compilers produce better code with unreachable anyways.



Simply not true.

do{}while(0) is a NOP it is no more useful than an ';' statement.  It 
doesn't serve as a magic uninitialized variable hiding mechanism.


David Daney



Thanks,
Andrew




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 r...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread Geert Uytterhoeven
On Mon, Sep 30, 2013 at 7:45 PM, David Daney ddaney.c...@gmail.com wrote:
 What about using __builtin_unreachable when we can but turn off warnings
 and use do{}while(0) when __builtin_unreachable does not exist?  This seems
 the both worlds.  Newer compilers produce better code with unreachable
 anyways.


 Simply not true.

 do{}while(0) is a NOP it is no more useful than an ';' statement.  It
 doesn't serve as a magic uninitialized variable hiding mechanism.

You missed the turn off warnings part of the and.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Issue with BUG() in asm-gemeric/bug.h if CONFIG_BUG=n

2013-09-30 Thread David Daney

On 09/30/2013 12:03 PM, Geert Uytterhoeven wrote:

On Mon, Sep 30, 2013 at 7:45 PM, David Daney ddaney.c...@gmail.com wrote:

What about using __builtin_unreachable when we can but turn off warnings
and use do{}while(0) when __builtin_unreachable does not exist?  This seems
the both worlds.  Newer compilers produce better code with unreachable
anyways.



Simply not true.

do{}while(0) is a NOP it is no more useful than an ';' statement.  It
doesn't serve as a magic uninitialized variable hiding mechanism.


You missed the turn off warnings part of the and.


You are correct, I did miss it.

The real problem here is that the kernel is written to expect that BUG() 
never returns.  Any implementation that has BUG() return, is almost 
certainly *not* what we want.


But wieh people select CONFIG_BUG=n they expect the smallest possible code.

These two criteria are mutually exclusive, so something should change.

It is not just the uninitialized variable warning, there can be others 
as well (control reaching the end of a non-void function comes to mind). 
 So I don't think turning off the warnings is a good solution.


That leaves:

1) Remove CONFIG_BUG and make it unconditionally enabled.

2) Make CONFIG_BUG=n imply static inline void BUG(void){do{}while(1);} 
which might be bigger than with CONFIG_BUG=y


David Daney




Gr{oetje,eeting}s,

 Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say programmer or something like that.
 -- Linus Torvalds




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