[PATCH 00/11] Add support for GCC's __builtin_unreachable() and use it in BUG (v2).

[PATCH 00/11] Add support for GCC's __builtin_unreachable() and use it in BUG (v2).

[David Daney]

When I sent the first version, I had not realized that Roland McGrath
had only a day or two earlier submitted a very similar patch (although
one that only fixed up the x86 case).

I have been working on this quite a while now, starting with adding
the required support to GCC, so with an eye towards finishing it up I
have this new version.

 From the announcement of the first version:

Starting with version 4.5, GCC has a new built-in function called
__builtin_unreachable().  The function tells the compiler that control
flow will never reach that point.  Currently we trick the compiler by
putting in for(;;); but this has the disadvantage that extra code is
emitted for an endless loop.  For an i386 kernel using
__builtin_unreachable() results in an defaultconfig that is nearly 4000
bytes smaller.

This patch set adds support to compiler.h creating a
new macro usable in the kernel called unreachable().  If the compiler
lacks __builtin_unreachable(), it just expands to for(;;).

The x86 and MIPS patches I actually tested with a GCC-4.5 snapshot.
Lacking the ability to test the rest of the architectures, I just did
what seemed right without even trying to compile the kernel.

For version 2:

I fixed a couple of checkpatch issues, and simplified the
unreachable() macro for the pre-GCC-4.5 case (as suggested by Richard
Henderson).  Also several Acked-by: were added.

New in this version (as suggested by Ingo Molnar) I added 11/11 which
uses unreachable() in asm-generic/bug.h for !CONFIG_BUG case.  This
one may be a little controversial as it will end up making code
slightly larger when !CONFIG_BUG and you are using a pre-GCC-4.5
compiler.

I will reply with the 11 patches.

David Daney (11):
   Add support for GCC-4.5's __builtin_unreachable() to compiler.h (v2)
   x86: Convert BUG() to use unreachable()
   MIPS: Convert BUG() to use unreachable()
   s390: Convert BUG() to use unreachable()
   mn10300: Convert BUG() to use unreachable()
   parisc: Convert BUG() to use unreachable()
   powerpc: Convert BUG() to use unreachable()
   alpha: Convert BUG() to use unreachable()
   avr32: Convert BUG() to use unreachable()
   blackfin: Convert BUG() to use unreachable()
   Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

  arch/alpha/include/asm/bug.h    |    3 ++-
  arch/avr32/include/asm/bug.h    |    2 +-
  arch/blackfin/include/asm/bug.h |    2 +-
  arch/mips/include/asm/bug.h     |    4 +---
  arch/mn10300/include/asm/bug.h  |    3 ++-
  arch/parisc/include/asm/bug.h   |    4 ++--
  arch/powerpc/include/asm/bug.h  |    2 +-
  arch/s390/include/asm/bug.h     |    2 +-
  arch/x86/include/asm/bug.h      |    4 ++--
  include/asm-generic/bug.h       |    4 ++--
  include/linux/compiler-gcc4.h   |   14 ++++++++++++++
  include/linux/compiler.h        |    5 +++++
  12 files changed, 34 insertions(+), 15 deletions(-)

[PATCH 00/11] Add support for GCC's __builtin_unreachable() and use it in BUG (v2).

[David Daney]

When I sent the first version, I had not realized that Roland McGrath
had only a day or two earlier submitted a very similar patch (although
one that only fixed up the x86 case).

I have been working on this quite a while now, starting with adding
the required support to GCC, so with an eye towards finishing it up I
have this new version.

 From the announcement of the first version:

Starting with version 4.5, GCC has a new built-in function called
__builtin_unreachable().  The function tells the compiler that control
flow will never reach that point.  Currently we trick the compiler by
putting in for(;;); but this has the disadvantage that extra code is
emitted for an endless loop.  For an i386 kernel using
__builtin_unreachable() results in an defaultconfig that is nearly 4000
bytes smaller.

This patch set adds support to compiler.h creating a
new macro usable in the kernel called unreachable().  If the compiler
lacks __builtin_unreachable(), it just expands to for(;;).

The x86 and MIPS patches I actually tested with a GCC-4.5 snapshot.
Lacking the ability to test the rest of the architectures, I just did
what seemed right without even trying to compile the kernel.

For version 2:

I fixed a couple of checkpatch issues, and simplified the
unreachable() macro for the pre-GCC-4.5 case (as suggested by Richard
Henderson).  Also several Acked-by: were added.

New in this version (as suggested by Ingo Molnar) I added 11/11 which
uses unreachable() in asm-generic/bug.h for !CONFIG_BUG case.  This
one may be a little controversial as it will end up making code
slightly larger when !CONFIG_BUG and you are using a pre-GCC-4.5
compiler.

I will reply with the 11 patches.

David Daney (11):
   Add support for GCC-4.5's __builtin_unreachable() to compiler.h (v2)
   x86: Convert BUG() to use unreachable()
   MIPS: Convert BUG() to use unreachable()
   s390: Convert BUG() to use unreachable()
   mn10300: Convert BUG() to use unreachable()
   parisc: Convert BUG() to use unreachable()
   powerpc: Convert BUG() to use unreachable()
   alpha: Convert BUG() to use unreachable()
   avr32: Convert BUG() to use unreachable()
   blackfin: Convert BUG() to use unreachable()
   Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

  arch/alpha/include/asm/bug.h    |    3 ++-
  arch/avr32/include/asm/bug.h    |    2 +-
  arch/blackfin/include/asm/bug.h |    2 +-
  arch/mips/include/asm/bug.h     |    4 +---
  arch/mn10300/include/asm/bug.h  |    3 ++-
  arch/parisc/include/asm/bug.h   |    4 ++--
  arch/powerpc/include/asm/bug.h  |    2 +-
  arch/s390/include/asm/bug.h     |    2 +-
  arch/x86/include/asm/bug.h      |    4 ++--
  include/asm-generic/bug.h       |    4 ++--
  include/linux/compiler-gcc4.h   |   14 ++++++++++++++
  include/linux/compiler.h        |    5 +++++
  12 files changed, 34 insertions(+), 15 deletions(-)

[PATCH 00/11] Add support for GCC's __builtin_unreachable() and use it in BUG (v2).

[David Daney]

When I sent the first version, I had not realized that Roland McGrath
had only a day or two earlier submitted a very similar patch (although
one that only fixed up the x86 case).

I have been working on this quite a while now, starting with adding
the required support to GCC, so with an eye towards finishing it up I
have this new version.

 From the announcement of the first version:

Starting with version 4.5, GCC has a new built-in function called
__builtin_unreachable().  The function tells the compiler that control
flow will never reach that point.  Currently we trick the compiler by
putting in for(;;); but this has the disadvantage that extra code is
emitted for an endless loop.  For an i386 kernel using
__builtin_unreachable() results in an defaultconfig that is nearly 4000
bytes smaller.

This patch set adds support to compiler.h creating a
new macro usable in the kernel called unreachable().  If the compiler
lacks __builtin_unreachable(), it just expands to for(;;).

The x86 and MIPS patches I actually tested with a GCC-4.5 snapshot.
Lacking the ability to test the rest of the architectures, I just did
what seemed right without even trying to compile the kernel.

For version 2:

I fixed a couple of checkpatch issues, and simplified the
unreachable() macro for the pre-GCC-4.5 case (as suggested by Richard
Henderson).  Also several Acked-by: were added.

New in this version (as suggested by Ingo Molnar) I added 11/11 which
uses unreachable() in asm-generic/bug.h for !CONFIG_BUG case.  This
one may be a little controversial as it will end up making code
slightly larger when !CONFIG_BUG and you are using a pre-GCC-4.5
compiler.

I will reply with the 11 patches.

David Daney (11):
   Add support for GCC-4.5's __builtin_unreachable() to compiler.h (v2)
   x86: Convert BUG() to use unreachable()
   MIPS: Convert BUG() to use unreachable()
   s390: Convert BUG() to use unreachable()
   mn10300: Convert BUG() to use unreachable()
   parisc: Convert BUG() to use unreachable()
   powerpc: Convert BUG() to use unreachable()
   alpha: Convert BUG() to use unreachable()
   avr32: Convert BUG() to use unreachable()
   blackfin: Convert BUG() to use unreachable()
   Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

  arch/alpha/include/asm/bug.h    |    3 ++-
  arch/avr32/include/asm/bug.h    |    2 +-
  arch/blackfin/include/asm/bug.h |    2 +-
  arch/mips/include/asm/bug.h     |    4 +---
  arch/mn10300/include/asm/bug.h  |    3 ++-
  arch/parisc/include/asm/bug.h   |    4 ++--
  arch/powerpc/include/asm/bug.h  |    2 +-
  arch/s390/include/asm/bug.h     |    2 +-
  arch/x86/include/asm/bug.h      |    4 ++--
  include/asm-generic/bug.h       |    4 ++--
  include/linux/compiler-gcc4.h   |   14 ++++++++++++++
  include/linux/compiler.h        |    5 +++++
  12 files changed, 34 insertions(+), 15 deletions(-)

[PATCH 00/11] Add support for GCC's __builtin_unreachable() and use it in BUG (v2).

[David Daney]

When I sent the first version, I had not realized that Roland McGrath
had only a day or two earlier submitted a very similar patch (although
one that only fixed up the x86 case).

I have been working on this quite a while now, starting with adding
the required support to GCC, so with an eye towards finishing it up I
have this new version.

 From the announcement of the first version:

Starting with version 4.5, GCC has a new built-in function called
__builtin_unreachable().  The function tells the compiler that control
flow will never reach that point.  Currently we trick the compiler by
putting in for(;;); but this has the disadvantage that extra code is
emitted for an endless loop.  For an i386 kernel using
__builtin_unreachable() results in an defaultconfig that is nearly 4000
bytes smaller.

This patch set adds support to compiler.h creating a
new macro usable in the kernel called unreachable().  If the compiler
lacks __builtin_unreachable(), it just expands to for(;;).

The x86 and MIPS patches I actually tested with a GCC-4.5 snapshot.
Lacking the ability to test the rest of the architectures, I just did
what seemed right without even trying to compile the kernel.

For version 2:

I fixed a couple of checkpatch issues, and simplified the
unreachable() macro for the pre-GCC-4.5 case (as suggested by Richard
Henderson).  Also several Acked-by: were added.

New in this version (as suggested by Ingo Molnar) I added 11/11 which
uses unreachable() in asm-generic/bug.h for !CONFIG_BUG case.  This
one may be a little controversial as it will end up making code
slightly larger when !CONFIG_BUG and you are using a pre-GCC-4.5
compiler.

I will reply with the 11 patches.

David Daney (11):
   Add support for GCC-4.5's __builtin_unreachable() to compiler.h (v2)
   x86: Convert BUG() to use unreachable()
   MIPS: Convert BUG() to use unreachable()
   s390: Convert BUG() to use unreachable()
   mn10300: Convert BUG() to use unreachable()
   parisc: Convert BUG() to use unreachable()
   powerpc: Convert BUG() to use unreachable()
   alpha: Convert BUG() to use unreachable()
   avr32: Convert BUG() to use unreachable()
   blackfin: Convert BUG() to use unreachable()
   Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

  arch/alpha/include/asm/bug.h    |    3 ++-
  arch/avr32/include/asm/bug.h    |    2 +-
  arch/blackfin/include/asm/bug.h |    2 +-
  arch/mips/include/asm/bug.h     |    4 +---
  arch/mn10300/include/asm/bug.h  |    3 ++-
  arch/parisc/include/asm/bug.h   |    4 ++--
  arch/powerpc/include/asm/bug.h  |    2 +-
  arch/s390/include/asm/bug.h     |    2 +-
  arch/x86/include/asm/bug.h      |    4 ++--
  include/asm-generic/bug.h       |    4 ++--
  include/linux/compiler-gcc4.h   |   14 ++++++++++++++
  include/linux/compiler.h        |    5 +++++
  12 files changed, 34 insertions(+), 15 deletions(-)

[PATCH 01/11] Add support for GCC-4.5's __builtin_unreachable() to compiler.h (v2)

[David Daney]

Starting with version 4.5, GCC has a new built-in function
__builtin_unreachable() that can be used in places like the kernel's
BUG() where inline assembly is used to transfer control flow.  This
eliminated the need for an endless loop in these places.

The patch adds a new macro 'unreachable()' that will expand to either
__builtin_unreachable() or an endless loop depending on the compiler
version.

Change from v1: Simplify unreachable() for non-GCC 4.5 case.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
---
 include/linux/compiler-gcc4.h |   14 ++++++++++++++
 include/linux/compiler.h      |    5 +++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 450fa59..ab3af40 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -36,4 +36,18 @@
    the kernel context */
 #define __cold			__attribute__((__cold__))
 
+
+#if __GNUC_MINOR__ >= 5
+/*
+ * Mark a position in code as unreachable.  This can be used to
+ * suppress control flow warnings after asm blocks that transfer
+ * control elsewhere.
+ *
+ * Early snapshots of gcc 4.5 don't support this and we can't detect
+ * this in the preprocessor, but we can live with this because they're
+ * unreleased.  Really, we need to have autoconf for the kernel.
+ */
+#define unreachable() __builtin_unreachable()
+#endif
+
 #endif
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 04fb513..59f2089 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -144,6 +144,11 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
 # define barrier() __memory_barrier()
 #endif
 
+/* Unreachable code */
+#ifndef unreachable
+# define unreachable() do { } while (1)
+#endif
+
 #ifndef RELOC_HIDE
 # define RELOC_HIDE(ptr, off)					\
   ({ unsigned long __ptr;					\
-- 
1.6.2.5

[PATCH 02/11] x86: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);.  When
allyesconfig is built with a GCC-4.5 snapshot on i686 the size of the
text segment is reduced by 3987 bytes (from 6827019 to 6823032).

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Acked-by: "H. Peter Anvin" <hpa@zytor.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: Ingo Molnar <mingo@redhat.com>
CC: x86@kernel.org
---
 arch/x86/include/asm/bug.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index d9cf1cd..f654d1b 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -22,14 +22,14 @@ do {								\
 		     ".popsection"				\
 		     : : "i" (__FILE__), "i" (__LINE__),	\
 		     "i" (sizeof(struct bug_entry)));		\
-	for (;;) ;						\
+	unreachable();						\
 } while (0)
 
 #else
 #define BUG()							\
 do {								\
 	asm volatile("ud2");					\
-	for (;;) ;						\
+	unreachable();						\
 } while (0)
 #endif
 
-- 
1.6.2.5

[PATCH 03/11] MIPS: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of while(1);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
---
 arch/mips/include/asm/bug.h |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/mips/include/asm/bug.h b/arch/mips/include/asm/bug.h
index 6cf29c2..540c98a 100644
--- a/arch/mips/include/asm/bug.h
+++ b/arch/mips/include/asm/bug.h
@@ -11,9 +11,7 @@
 static inline void __noreturn BUG(void)
 {
 	__asm__ __volatile__("break %0" : : "i" (BRK_BUG));
-	/* Fool GCC into thinking the function doesn't return. */
-	while (1)
-		;
+	unreachable();
 }
 
 #define HAVE_ARCH_BUG
-- 
1.6.2.5

[PATCH] MIPS: Make more use of unreachable()

[Ralf Baechle]

Further uses of unreachable().

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

 arch/mips/fw/arc/memory.c     |    5 +++--
 arch/mips/kernel/signal.c     |    5 +++--
 arch/mips/kernel/signal32.c   |    5 +++--
 arch/mips/kernel/signal_n32.c |    3 ++-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/mips/fw/arc/memory.c b/arch/mips/fw/arc/memory.c
index 8b8eea2..d248f64 100644
--- a/arch/mips/fw/arc/memory.c
+++ b/arch/mips/fw/arc/memory.c
@@ -11,6 +11,7 @@
  * because on some machines like SGI IP27 the ARC memory configuration data
  * completly bogus and alternate easier to use mechanisms are available.
  */
+#include <linux/compiler.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -80,7 +81,7 @@ static inline int memtype_classify_arcs(union linux_memtypes type)
 	default:
 		BUG();
 	}
-	while(1);				/* Nuke warning.  */
+	unreachable();				/* Nuke warning.  */
 }
 
 static inline int memtype_classify_arc(union linux_memtypes type)
@@ -100,7 +101,7 @@ static inline int memtype_classify_arc(union linux_memtypes type)
 	default:
 		BUG();
 	}
-	while(1);				/* Nuke warning.  */
+	unreachable();				/* Nuke warning.  */
 }
 
 static int __init prom_memtype_classify(union linux_memtypes type)
diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index 6254041..eefd278 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -8,6 +8,7 @@
  * Copyright (C) 1999, 2000 Silicon Graphics, Inc.
  */
 #include <linux/cache.h>
+#include <linux/compiler.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
 #include <linux/personality.h>
@@ -422,7 +423,7 @@ asmlinkage void sys_sigreturn(nabi_no_regargs struct pt_regs regs)
 		"j\tsyscall_exit"
 		:/* no outputs */
 		:"r" (&regs));
-	/* Unreached */
+	unreachable();
 
 badframe:
 	force_sig(SIGSEGV, current);
@@ -468,7 +469,7 @@ asmlinkage void sys_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		"j\tsyscall_exit"
 		:/* no outputs */
 		:"r" (&regs));
-	/* Unreached */
+	unreachable();
 
 badframe:
 	force_sig(SIGSEGV, current);
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index 2e74075..d41e267 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -9,6 +9,7 @@
  */
 #include <linux/cache.h>
 #include <linux/compat.h>
+#include <linux/compiler.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
 #include <linux/smp.h>
@@ -526,7 +527,7 @@ asmlinkage void sys32_sigreturn(nabi_no_regargs struct pt_regs regs)
 		"j\tsyscall_exit"
 		:/* no outputs */
 		:"r" (&regs));
-	/* Unreached */
+	unreachable();
 
 badframe:
 	force_sig(SIGSEGV, current);
@@ -583,7 +584,7 @@ asmlinkage void sys32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		"j\tsyscall_exit"
 		:/* no outputs */
 		:"r" (&regs));
-	/* Unreached */
+	unreachable();
 
 badframe:
 	force_sig(SIGSEGV, current);
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index bb277e8..24ebaa5 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307, USA.
  */
 #include <linux/cache.h>
+#include <linux/compiler.h>
 #include <linux/sched.h>
 #include <linux/mm.h>
 #include <linux/smp.h>
@@ -167,7 +168,7 @@ asmlinkage void sysn32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		"j\tsyscall_exit"
 		:/* no outputs */
 		:"r" (&regs));
-	/* Unreached */
+	unreachable();
 
 badframe:
 	force_sig(SIGSEGV, current);

[PATCH 04/11] s390: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
CC: Heiko Carstens <heiko.carstens@de.ibm.com>
CC: linux390@de.ibm.com
CC: linux-s390@vger.kernel.org
---
 arch/s390/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/s390/include/asm/bug.h b/arch/s390/include/asm/bug.h
index 7efd0ab..efb74fd 100644
--- a/arch/s390/include/asm/bug.h
+++ b/arch/s390/include/asm/bug.h
@@ -49,7 +49,7 @@
 
 #define BUG() do {					\
 	__EMIT_BUG(0);					\
-	for (;;);					\
+	unreachable();					\
 } while (0)
 
 #define WARN_ON(x) ({					\
-- 
1.6.2.5

[PATCH 05/11] mn10300: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of while(1).

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: David Howells <dhowells@redhat.com>
CC: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
CC: linux-am33-list@redhat.com
---
 arch/mn10300/include/asm/bug.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/mn10300/include/asm/bug.h b/arch/mn10300/include/asm/bug.h
index aa6a388..447a7e6 100644
--- a/arch/mn10300/include/asm/bug.h
+++ b/arch/mn10300/include/asm/bug.h
@@ -27,7 +27,8 @@ do {								\
 		:						\
 		: "i"(__FILE__), "i"(__LINE__)			\
 		);						\
-} while (1)
+	unreachable();						\
+} while (0)
 
 #define HAVE_ARCH_BUG
 #endif /* CONFIG_BUG */
-- 
1.6.2.5

[PATCH 06/11] parisc: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Kyle McMartin <kyle@mcmartin.ca>
CC: Helge Deller <deller@gmx.de>
CC: linux-parisc@vger.kernel.org
---
 arch/parisc/include/asm/bug.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/parisc/include/asm/bug.h b/arch/parisc/include/asm/bug.h
index 8cfc553..75e46c5 100644
--- a/arch/parisc/include/asm/bug.h
+++ b/arch/parisc/include/asm/bug.h
@@ -32,14 +32,14 @@
 			     "\t.popsection"				\
 			     : : "i" (__FILE__), "i" (__LINE__),	\
 			     "i" (0), "i" (sizeof(struct bug_entry)) ); \
-		for(;;) ;						\
+		unreachable();						\
 	} while(0)
 
 #else
 #define BUG()								\
 	do {								\
 		asm volatile(PARISC_BUG_BREAK_ASM : : );		\
-		for(;;) ;						\
+		unreachable();						\
 	} while(0)
 #endif
 
-- 
1.6.2.5

[PATCH 07/11] powerpc: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: linuxppc-dev@ozlabs.org
---
 arch/powerpc/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 64e1fdc..2c15212 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -68,7 +68,7 @@
 		_EMIT_BUG_ENTRY					\
 		: : "i" (__FILE__), "i" (__LINE__),		\
 		    "i" (0), "i"  (sizeof(struct bug_entry)));	\
-	for(;;) ;						\
+	unreachable();						\
 } while (0)
 
 #define BUG_ON(x) do {						\
-- 
1.6.2.5

[PATCH 07/11] powerpc: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: linuxppc-dev@ozlabs.org
---
 arch/powerpc/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index 64e1fdc..2c15212 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -68,7 +68,7 @@
 		_EMIT_BUG_ENTRY					\
 		: : "i" (__FILE__), "i" (__LINE__),		\
 		    "i" (0), "i"  (sizeof(struct bug_entry)));	\
-	for(;;) ;						\
+	unreachable();						\
 } while (0)
 
 #define BUG_ON(x) do {						\
-- 
1.6.2.5

[PATCH 08/11] alpha: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Richard Henderson <rth@twiddle.net>
CC: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
CC: linux-alpha@vger.kernel.org
---
 arch/alpha/include/asm/bug.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/alpha/include/asm/bug.h b/arch/alpha/include/asm/bug.h
index 1720c8a..f091682 100644
--- a/arch/alpha/include/asm/bug.h
+++ b/arch/alpha/include/asm/bug.h
@@ -13,7 +13,8 @@
 		"call_pal %0  # bugchk\n\t"				\
 		".long %1\n\t.8byte %2"					\
 		: : "i"(PAL_bugchk), "i"(__LINE__), "i"(__FILE__));	\
-	for ( ; ; ); } while (0)
+	unreachable();							\
+  } while (0)
 
 #define HAVE_ARCH_BUG
 #endif
-- 
1.6.2.5

[PATCH 09/11] avr32: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>
---
 arch/avr32/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/avr32/include/asm/bug.h b/arch/avr32/include/asm/bug.h
index 331d45b..2aa373c 100644
--- a/arch/avr32/include/asm/bug.h
+++ b/arch/avr32/include/asm/bug.h
@@ -52,7 +52,7 @@
 #define BUG()								\
 	do {								\
 		_BUG_OR_WARN(0);					\
-		for (;;);						\
+		unreachable();						\
 	} while (0)
 
 #define WARN_ON(condition)							\
-- 
1.6.2.5

[PATCH 10/11] blackfin: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
CC: Mike Frysinger <vapier@gentoo.org>
CC: uclinux-dist-devel@blackfin.uclinux.org
---
 arch/blackfin/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/include/asm/bug.h b/arch/blackfin/include/asm/bug.h
index 655e495..0b213a9 100644
--- a/arch/blackfin/include/asm/bug.h
+++ b/arch/blackfin/include/asm/bug.h
@@ -41,7 +41,7 @@
 #define BUG()								\
 	do {								\
 		_BUG_OR_WARN(0);					\
-		for (;;);						\
+		unreachable();						\
 	} while (0)
 
 #define WARN_ON(condition)							\
-- 
1.6.2.5

[PATCH 10/11] blackfin: Convert BUG() to use unreachable()

[David Daney]

Use the new unreachable() macro instead of for(;;);

Signed-off-by: David Daney <ddaney-M3mlKVOIwJVv6pq1l3V1OdBPR1lH4CV8@public.gmane.org>
CC: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
CC: uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b@public.gmane.org
---
 arch/blackfin/include/asm/bug.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/blackfin/include/asm/bug.h b/arch/blackfin/include/asm/bug.h
index 655e495..0b213a9 100644
--- a/arch/blackfin/include/asm/bug.h
+++ b/arch/blackfin/include/asm/bug.h
@@ -41,7 +41,7 @@
 #define BUG()								\
 	do {								\
 		_BUG_OR_WARN(0);					\
-		for (;;);						\
+		unreachable();						\
 	} while (0)
 
 #define WARN_ON(condition)							\
-- 
1.6.2.5

[PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[David Daney]

The subject says it all (most).  The only drawback here is that for a
pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
BUG() to an endless loop.  Before the patch when configured with
!CONFIG_BUG() you might get some warnings, but the code would be
small.  After the patch there are no warnings, but there is an endless
loop at each BUG() site.

Of course for the GCC-4.5 case we get the best of both worlds.

Signed-off-by: David Daney <ddaney@caviumnetworks.com>
Suggested-by: Ingo Molnar <mingo@elte.hu>
CC: Ingo Molnar <mingo@elte.hu>
---
 include/asm-generic/bug.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
index 4b67559..e952242 100644
--- a/include/asm-generic/bug.h
+++ b/include/asm-generic/bug.h
@@ -89,11 +89,11 @@ 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
-#define BUG_ON(condition) do { if (condition) ; } while(0)
+#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
 #endif
 
 #ifndef HAVE_ARCH_WARN_ON
-- 
1.6.2.5

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Brian Gerst]

On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> The subject says it all (most).  The only drawback here is that for a
> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
> BUG() to an endless loop.  Before the patch when configured with
> !CONFIG_BUG() you might get some warnings, but the code would be
> small.  After the patch there are no warnings, but there is an endless
> loop at each BUG() site.
>
> Of course for the GCC-4.5 case we get the best of both worlds.
>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
> Suggested-by: Ingo Molnar <mingo@elte.hu>
> CC: Ingo Molnar <mingo@elte.hu>
> ---
>  include/asm-generic/bug.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 4b67559..e952242 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -89,11 +89,11 @@ 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
> -#define BUG_ON(condition) do { if (condition) ; } while(0)
> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>  #endif
>
>  #ifndef HAVE_ARCH_WARN_ON
> --

This seems wrong to me.  Wouldn't you always want to do the endless
loop?  In the absence of an arch-specific method to jump to an
exception handler, it isn't really unreachable.  On gcc 4.5 this would
essentially become a no-op.

--
Brian Gerst

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Brian Gerst]

On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> The subject says it all (most).  The only drawback here is that for a
> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
> BUG() to an endless loop.  Before the patch when configured with
> !CONFIG_BUG() you might get some warnings, but the code would be
> small.  After the patch there are no warnings, but there is an endless
> loop at each BUG() site.
>
> Of course for the GCC-4.5 case we get the best of both worlds.
>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
> Suggested-by: Ingo Molnar <mingo@elte.hu>
> CC: Ingo Molnar <mingo@elte.hu>
> ---
>  include/asm-generic/bug.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 4b67559..e952242 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -89,11 +89,11 @@ 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
> -#define BUG_ON(condition) do { if (condition) ; } while(0)
> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>  #endif
>
>  #ifndef HAVE_ARCH_WARN_ON
> --

This seems wrong to me.  Wouldn't you always want to do the endless
loop?  In the absence of an arch-specific method to jump to an
exception handler, it isn't really unreachable.  On gcc 4.5 this would
essentially become a no-op.

--
Brian Gerst

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[David Daney]

Brian Gerst wrote:
> On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>> The subject says it all (most).  The only drawback here is that for a
>> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
>> BUG() to an endless loop.  Before the patch when configured with
>> !CONFIG_BUG() you might get some warnings, but the code would be
>> small.  After the patch there are no warnings, but there is an endless
>> loop at each BUG() site.
>>
>> Of course for the GCC-4.5 case we get the best of both worlds.
>>
>> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
>> Suggested-by: Ingo Molnar <mingo@elte.hu>
>> CC: Ingo Molnar <mingo@elte.hu>
>> ---
>>  include/asm-generic/bug.h |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>> index 4b67559..e952242 100644
>> --- a/include/asm-generic/bug.h
>> +++ b/include/asm-generic/bug.h
>> @@ -89,11 +89,11 @@ 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
>> -#define BUG_ON(condition) do { if (condition) ; } while(0)
>> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>>  #endif
>>
>>  #ifndef HAVE_ARCH_WARN_ON
>> --
> 
> This seems wrong to me.  Wouldn't you always want to do the endless
> loop?  In the absence of an arch-specific method to jump to an
> exception handler, it isn't really unreachable.  On gcc 4.5 this would
> essentially become a no-op.
> 

Several points:

* When you hit a BUG() you are screwed.

* When you configure with !CONFIG_BUG you are asserting that you don't 
want to try to trap on BUG();.

The existing code just falls through to whatever happens to follow the 
BUG().  This is not what the programmer intended, but the person that 
chose !CONFIG_BUG decided that they would like undefined behavior in 
order to save a few bytes of code.

With the patch one of two things will happen:

pre-GCC-4.5) We will now enter an endless loop and not fall through. 
This makes the code slightly larger than pre patch.

post-GCC-4.5) We do something totally undefined.  It will not 
necessarily fall through to the code after the BUG()  It could really 
end up doing almost anything.  On the plus side, we save a couple of 
bytes of code and eliminate some compiler warnings.

If you don't like it, don't configure with !CONFIG_BUG.  But the patch 
doesn't really change the fact that hitting a BUG() with !CONFIG_BUG 
leads to undefined behavior.  It only makes the case where you don't hit 
BUG() nicer.

David Daney

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Linus Torvalds]

On Mon, 14 Sep 2009, David Daney wrote:
> 
> The existing code just falls through to whatever happens to follow the BUG().

Brian was talking BUG_ON().

And the existing !CONFIG_BUG BUG_ON() is actually set up so that gcc will 
just optimize it away entirely (yet give the same compile-time warnings as 
the "real" BUG_ON() does).

Changing it to "if (cond) unreachable()" is likely to generate _more_ 
code, which is against the whole point of wanting to disable CONFIG_BUG.

			Linus

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Linus Torvalds]

On Mon, 14 Sep 2009, David Daney wrote:
> 
> The existing code just falls through to whatever happens to follow the BUG().

Brian was talking BUG_ON().

And the existing !CONFIG_BUG BUG_ON() is actually set up so that gcc will 
just optimize it away entirely (yet give the same compile-time warnings as 
the "real" BUG_ON() does).

Changing it to "if (cond) unreachable()" is likely to generate _more_ 
code, which is against the whole point of wanting to disable CONFIG_BUG.

			Linus

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[David Daney]

Linus Torvalds wrote:
> 
> On Mon, 14 Sep 2009, David Daney wrote:
>> The existing code just falls through to whatever happens to follow the BUG().
> 
> Brian was talking BUG_ON().
> 
> And the existing !CONFIG_BUG BUG_ON() is actually set up so that gcc will 
> just optimize it away entirely (yet give the same compile-time warnings as 
> the "real" BUG_ON() does).
> 
> Changing it to "if (cond) unreachable()" is likely to generate _more_ 
> code, which is against the whole point of wanting to disable CONFIG_BUG.
> 

Yes, you are correct.  I said the same thing in the log message for the 
patch.

Really it may be too early for this patch to be appropriate for your 
tree.  GCC-4.5 will probably not be released for several more months, 
and it will be several years before a GCC with __builtin_unreachable() 
is being used by the majority of people compiling kernels.

Ingo had suggested the approach of this patch as a way of eliminating 
many warnings when using !CONFIG_BUG.  I think it clearly makes sense 
for compilers that support __builtin_unreachable(), but clearly it is 
not an unquestionable win if we end up generating larger code.

With this particular patch, I don't really care if you merge it or not. 
  Perhaps I shouldn't have made it part of the set.

The rest of the set I think would make sense for 2.6.32 or 2.6.33.

David Daney

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Brian Gerst]

On Mon, Sep 14, 2009 at 7:28 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> Brian Gerst wrote:
>>
>> On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com>
>> wrote:
>>>
>>> The subject says it all (most).  The only drawback here is that for a
>>> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
>>> BUG() to an endless loop.  Before the patch when configured with
>>> !CONFIG_BUG() you might get some warnings, but the code would be
>>> small.  After the patch there are no warnings, but there is an endless
>>> loop at each BUG() site.
>>>
>>> Of course for the GCC-4.5 case we get the best of both worlds.
>>>
>>> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
>>> Suggested-by: Ingo Molnar <mingo@elte.hu>
>>> CC: Ingo Molnar <mingo@elte.hu>
>>> ---
>>>  include/asm-generic/bug.h |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>>> index 4b67559..e952242 100644
>>> --- a/include/asm-generic/bug.h
>>> +++ b/include/asm-generic/bug.h
>>> @@ -89,11 +89,11 @@ 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
>>> -#define BUG_ON(condition) do { if (condition) ; } while(0)
>>> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>>>  #endif
>>>
>>>  #ifndef HAVE_ARCH_WARN_ON
>>> --
>>
>> This seems wrong to me.  Wouldn't you always want to do the endless
>> loop?  In the absence of an arch-specific method to jump to an
>> exception handler, it isn't really unreachable.  On gcc 4.5 this would
>> essentially become a no-op.
>>
>
> Several points:
>
> * When you hit a BUG() you are screwed.
>
> * When you configure with !CONFIG_BUG you are asserting that you don't want
> to try to trap on BUG();.
>
> The existing code just falls through to whatever happens to follow the
> BUG().  This is not what the programmer intended, but the person that chose
> !CONFIG_BUG decided that they would like undefined behavior in order to save
> a few bytes of code.
>
> With the patch one of two things will happen:
>
> pre-GCC-4.5) We will now enter an endless loop and not fall through. This
> makes the code slightly larger than pre patch.
>
> post-GCC-4.5) We do something totally undefined.  It will not necessarily
> fall through to the code after the BUG()  It could really end up doing
> almost anything.  On the plus side, we save a couple of bytes of code and
> eliminate some compiler warnings.
>
> If you don't like it, don't configure with !CONFIG_BUG.  But the patch
> doesn't really change the fact that hitting a BUG() with !CONFIG_BUG leads
> to undefined behavior.  It only makes the case where you don't hit BUG()
> nicer.
>
> David Daney
>

Let me rephrase this.  The original BUG() is simply a no-op, not an
infinite loop.  GCC will optimize it away (and possibly other dead
code around it).  Adding unreachable() makes the code do potentially
unpredictable things.  It's not necessary.  The same goes for BUG_ON.
In that case the test does get optimized away too, but is still needed
to silence warnings about unused variables, etc.

--
Brian Gerst

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[Brian Gerst]

On Mon, Sep 14, 2009 at 7:28 PM, David Daney <ddaney@caviumnetworks.com> wrote:
> Brian Gerst wrote:
>>
>> On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com>
>> wrote:
>>>
>>> The subject says it all (most).  The only drawback here is that for a
>>> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
>>> BUG() to an endless loop.  Before the patch when configured with
>>> !CONFIG_BUG() you might get some warnings, but the code would be
>>> small.  After the patch there are no warnings, but there is an endless
>>> loop at each BUG() site.
>>>
>>> Of course for the GCC-4.5 case we get the best of both worlds.
>>>
>>> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
>>> Suggested-by: Ingo Molnar <mingo@elte.hu>
>>> CC: Ingo Molnar <mingo@elte.hu>
>>> ---
>>>  include/asm-generic/bug.h |    4 ++--
>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>>> index 4b67559..e952242 100644
>>> --- a/include/asm-generic/bug.h
>>> +++ b/include/asm-generic/bug.h
>>> @@ -89,11 +89,11 @@ 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
>>> -#define BUG_ON(condition) do { if (condition) ; } while(0)
>>> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>>>  #endif
>>>
>>>  #ifndef HAVE_ARCH_WARN_ON
>>> --
>>
>> This seems wrong to me.  Wouldn't you always want to do the endless
>> loop?  In the absence of an arch-specific method to jump to an
>> exception handler, it isn't really unreachable.  On gcc 4.5 this would
>> essentially become a no-op.
>>
>
> Several points:
>
> * When you hit a BUG() you are screwed.
>
> * When you configure with !CONFIG_BUG you are asserting that you don't want
> to try to trap on BUG();.
>
> The existing code just falls through to whatever happens to follow the
> BUG().  This is not what the programmer intended, but the person that chose
> !CONFIG_BUG decided that they would like undefined behavior in order to save
> a few bytes of code.
>
> With the patch one of two things will happen:
>
> pre-GCC-4.5) We will now enter an endless loop and not fall through. This
> makes the code slightly larger than pre patch.
>
> post-GCC-4.5) We do something totally undefined.  It will not necessarily
> fall through to the code after the BUG()  It could really end up doing
> almost anything.  On the plus side, we save a couple of bytes of code and
> eliminate some compiler warnings.
>
> If you don't like it, don't configure with !CONFIG_BUG.  But the patch
> doesn't really change the fact that hitting a BUG() with !CONFIG_BUG leads
> to undefined behavior.  It only makes the case where you don't hit BUG()
> nicer.
>
> David Daney
>

Let me rephrase this.  The original BUG() is simply a no-op, not an
infinite loop.  GCC will optimize it away (and possibly other dead
code around it).  Adding unreachable() makes the code do potentially
unpredictable things.  It's not necessary.  The same goes for BUG_ON.
In that case the test does get optimized away too, but is still needed
to silence warnings about unused variables, etc.

--
Brian Gerst

Re: [PATCH 11/11] Use unreachable() in asm-generic/bug.h for !CONFIG_BUG case.

[David Daney]

Brian Gerst wrote:
> On Mon, Sep 14, 2009 at 7:28 PM, David Daney <ddaney@caviumnetworks.com> wrote:
>> Brian Gerst wrote:
>>> On Mon, Sep 14, 2009 at 5:55 PM, David Daney <ddaney@caviumnetworks.com>
>>> wrote:
>>>> The subject says it all (most).  The only drawback here is that for a
>>>> pre-GCC-5.4 compiler, instead of expanding to nothing we now expand
>>>> BUG() to an endless loop.  Before the patch when configured with
>>>> !CONFIG_BUG() you might get some warnings, but the code would be
>>>> small.  After the patch there are no warnings, but there is an endless
>>>> loop at each BUG() site.
>>>>
>>>> Of course for the GCC-4.5 case we get the best of both worlds.
>>>>
>>>> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
>>>> Suggested-by: Ingo Molnar <mingo@elte.hu>
>>>> CC: Ingo Molnar <mingo@elte.hu>
>>>> ---
>>>>  include/asm-generic/bug.h |    4 ++--
>>>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
>>>> index 4b67559..e952242 100644
>>>> --- a/include/asm-generic/bug.h
>>>> +++ b/include/asm-generic/bug.h
>>>> @@ -89,11 +89,11 @@ 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
>>>> -#define BUG_ON(condition) do { if (condition) ; } while(0)
>>>> +#define BUG_ON(condition) do { if (condition) unreachable(); } while (0)
>>>>  #endif
>>>>
>>>>  #ifndef HAVE_ARCH_WARN_ON
>>>> --
>>> This seems wrong to me.  Wouldn't you always want to do the endless
>>> loop?  In the absence of an arch-specific method to jump to an
>>> exception handler, it isn't really unreachable.  On gcc 4.5 this would
>>> essentially become a no-op.
>>>
>> Several points:
>>
>> * When you hit a BUG() you are screwed.
>>
>> * When you configure with !CONFIG_BUG you are asserting that you don't want
>> to try to trap on BUG();.
>>
>> The existing code just falls through to whatever happens to follow the
>> BUG().  This is not what the programmer intended, but the person that chose
>> !CONFIG_BUG decided that they would like undefined behavior in order to save
>> a few bytes of code.
>>
>> With the patch one of two things will happen:
>>
>> pre-GCC-4.5) We will now enter an endless loop and not fall through. This
>> makes the code slightly larger than pre patch.
>>
>> post-GCC-4.5) We do something totally undefined.  It will not necessarily
>> fall through to the code after the BUG()  It could really end up doing
>> almost anything.  On the plus side, we save a couple of bytes of code and
>> eliminate some compiler warnings.
>>
>> If you don't like it, don't configure with !CONFIG_BUG.  But the patch
>> doesn't really change the fact that hitting a BUG() with !CONFIG_BUG leads
>> to undefined behavior.  It only makes the case where you don't hit BUG()
>> nicer.
>>
>> David Daney
>>
> 
> Let me rephrase this.  The original BUG() is simply a no-op, not an
> infinite loop.  GCC will optimize it away (and possibly other dead
> code around it).  Adding unreachable() makes the code do potentially
> unpredictable things.

The code already does unpredictable things (also known as undefined 
behavior) without the patch.  Consider this code:

enum values {GOOD, BAD, RUN_NORMALLY};

int foo(int a)
{
	if (a = GOOD)
		return RUN_NORMALLY;
	BUG();
}


void bar(void)
{
	if (foo(BAD) == RUN_NORMALLY)
		do_something_useful();
	else
		irreversibly_damage_hardware();
}


Q: What does this do with CONFIG_BUG?

A: It traps in BUG().

Q: What does this do with !CONFIG_BUG?

A: The compiler issues a warning about reaching the end of a non-void 
function.  At runtime we don't know what happens.

With my patch the answer to the second question changes to:

A: No compiler warnings are issued.  Depending on compiler version code 
may be larger. Runtime behavior depends on compiler version (either an 
endless loop in BUG, or undefined).

Since the behavior of the program when configured !CONFIG_BUG is 
undefined for cases that would trap had CONFIG_BUG be selected, the only 
tangible differences pre and post patch are:

GCC-4.4: No warnings, slightly larger code.

GCC-4.5: No warnings, code should not be any larger.

>  It's not necessary.

Many patches are 'not necessary', the question should be: are they 
desirable.

> The same goes for BUG_ON.
> In that case the test does get optimized away too, but is still needed
> to silence warnings about unused variables, etc.

For the GCC-4.5 case, the patch is even better.  Not only does the 
evaluation of the condition get optimized away, the compiler knows the 
condition is false in the code following the the BUG() and can propagate 
that knowledge into optimizations on the following code.


David Daney