BUG() never returns, so code after it is unreachable: remove it.
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
---
drivers/base/regmap/regmap-irq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 6c6869188c31..14f5fcc3ec1d 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -436,7 +436,6 @@ static irqreturn_t regmap_irq_thread(int irq, void *d)
break;
default:
BUG();
- goto exit;
}
}
--
2.39.5On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: > BUG() never returns, so code after it is unreachable: remove it. Thank you, this is the right change to do. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: > BUG() never returns, so code after it is unreachable: remove it. BUG() can be compiled out, CONFIG_BUG.
On Wed Apr 9, 2025 at 5:19 PM CEST, Mark Brown wrote: > On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: >> BUG() never returns, so code after it is unreachable: remove it. > > BUG() can be compiled out, CONFIG_BUG. Hi Mark, Thanks for your review. As there has been a bit of discussion on this patch, may I ask you for your current opinion about this change? Should I drop it from my next series? Best regards, Mathieu -- Mathieu Dubois-Briand, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Wed, Apr 23, 2025 at 02:41:47PM +0200, Mathieu Dubois-Briand wrote: > As there has been a bit of discussion on this patch, may I ask you for > your current opinion about this change? > Should I drop it from my next series? Please.
On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote:
> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote:
> > BUG() never returns, so code after it is unreachable: remove it.
>
> BUG() can be compiled out, CONFIG_BUG.
In that case BUG is defined as:
#define BUG() do { \
do {} while (1); \
unreachable(); \
} while (0)
so the return can be dropped as suggested in the patch.
Best regards
Uwe
On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote: > On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: > > BUG() never returns, so code after it is unreachable: remove it. > > BUG() can be compiled out, CONFIG_BUG. Yes, and it's still has unreachable() there. So, this change is correct. See include/asm-generic/bug.h for the details of the implementation. And yes, if we have an architecture that does not do this way, it has to be fixed. -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 06:38:33PM +0300, Andy Shevchenko wrote: > On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote: > > On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: > > > BUG() never returns, so code after it is unreachable: remove it. > > > > BUG() can be compiled out, CONFIG_BUG. > > Yes, and it's still has unreachable() there. So, this change is correct. > See include/asm-generic/bug.h for the details of the implementation. > And yes, if we have an architecture that does not do this way, it has to > be fixed. FWIW, I just have briefly checked the architectures and all of them include asm-generic/bug.h independently on the configuration option, some of them even define BUG() despite the configuration option (e.g. arc). -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 06:38:32PM +0300, Andy Shevchenko wrote: > On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote: > > BUG() can be compiled out, CONFIG_BUG. > Yes, and it's still has unreachable() there. So, this change is correct. > See include/asm-generic/bug.h for the details of the implementation. > And yes, if we have an architecture that does not do this way, it has to > be fixed. unreachable() just annotates things, AFAICT it doesn't actually guarantee to do anything in particular if the annotation turns out to be incorrect.
On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote: > On Wed, Apr 09, 2025 at 06:38:32PM +0300, Andy Shevchenko wrote: > > On Wed, Apr 09, 2025 at 04:19:27PM +0100, Mark Brown wrote: > > > > BUG() can be compiled out, CONFIG_BUG. > > > Yes, and it's still has unreachable() there. So, this change is correct. > > See include/asm-generic/bug.h for the details of the implementation. > > And yes, if we have an architecture that does not do this way, it has to > > be fixed. > > unreachable() just annotates things, AFAICT it doesn't actually > guarantee to do anything in particular if the annotation turns out to be > incorrect. I;m not sure I follow. unreachable is a wrapper on top of __builtin_unreachable() which is intrinsic of the compiler. https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote: > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote: > > unreachable() just annotates things, AFAICT it doesn't actually > > guarantee to do anything in particular if the annotation turns out to be > > incorrect. > I;m not sure I follow. unreachable is a wrapper on top of > __builtin_unreachable() which is intrinsic of the compiler. > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable That just says that the program is undefined if we get to the __builtin_undefined() and documents some behaviour around warnings. One example of undefined behaviour would be doing nothing.
On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote: > On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote: > > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote: > > > > unreachable() just annotates things, AFAICT it doesn't actually > > > guarantee to do anything in particular if the annotation turns out to be > > > incorrect. > > > I;m not sure I follow. unreachable is a wrapper on top of > > __builtin_unreachable() which is intrinsic of the compiler. > > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable > > That just says that the program is undefined if we get to the > __builtin_undefined() and documents some behaviour around warnings. One > example of undefined behaviour would be doing nothing. Theoretically yes, practically return after a BUG() makes no sense. Note, that compiler effectively removes 'goto exit;' here (that's also mentioned in the documentation independently on the control flow behaviour), so I don't know what you expect from it. -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 07:45:42PM +0300, Andy Shevchenko wrote: > On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote: > > On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote: > > > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote: > > > > > > unreachable() just annotates things, AFAICT it doesn't actually > > > > guarantee to do anything in particular if the annotation turns out to be > > > > incorrect. > > > > > I;m not sure I follow. unreachable is a wrapper on top of > > > __builtin_unreachable() which is intrinsic of the compiler. > > > > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable > > > > That just says that the program is undefined if we get to the > > __builtin_undefined() and documents some behaviour around warnings. One > > example of undefined behaviour would be doing nothing. > > Theoretically yes, practically return after a BUG() makes no sense. Note, > that compiler effectively removes 'goto exit;' here (that's also mentioned > in the documentation independently on the control flow behaviour), so > I don't know what you expect from it. So unreachable() sometimes lears to weird behavior from compiler, for example as mentioned here where we ended up removing it to prevent miscompilations: https://lore.kernel.org/all/20241010222451.GA3571761@thelio-3990X/ Thanks. -- Dmitry
Wed, Apr 09, 2025 at 10:16:40AM -0700, Dmitry Torokhov kirjoitti: > On Wed, Apr 09, 2025 at 07:45:42PM +0300, Andy Shevchenko wrote: > > On Wed, Apr 09, 2025 at 05:32:55PM +0100, Mark Brown wrote: > > > On Wed, Apr 09, 2025 at 07:00:24PM +0300, Andy Shevchenko wrote: > > > > On Wed, Apr 09, 2025 at 04:46:04PM +0100, Mark Brown wrote: > > > > > > > > unreachable() just annotates things, AFAICT it doesn't actually > > > > > guarantee to do anything in particular if the annotation turns out to be > > > > > incorrect. > > > > > > > I;m not sure I follow. unreachable is a wrapper on top of > > > > __builtin_unreachable() which is intrinsic of the compiler. > > > > > > > https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-_005f_005fbuiltin_005funreachable > > > > > > That just says that the program is undefined if we get to the > > > __builtin_undefined() and documents some behaviour around warnings. One > > > example of undefined behaviour would be doing nothing. > > > > Theoretically yes, practically return after a BUG() makes no sense. Note, > > that compiler effectively removes 'goto exit;' here (that's also mentioned > > in the documentation independently on the control flow behaviour), so > > I don't know what you expect from it. > > So unreachable() sometimes lears to weird behavior from compiler, for > example as mentioned here where we ended up removing it to prevent > miscompilations: > > https://lore.kernel.org/all/20241010222451.GA3571761@thelio-3990X/ How does it affect the BUG()? From your link: "I tested using BUG() in lieu of unreachable() like the second change I mentioned above, which resolves the issue cleanly, ..." -- With Best Regards, Andy Shevchenko
On Wed, Apr 09, 2025 at 04:19:34PM +0100, Mark Brown wrote: > On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: > > BUG() never returns, so code after it is unreachable: remove it. > > BUG() can be compiled out, CONFIG_BUG. Also, please don't mix irrelevant patches into random serieses. It just makes everything noisier for everyone.
On Wed Apr 9, 2025 at 5:21 PM CEST, Mark Brown wrote: > On Wed, Apr 09, 2025 at 04:19:34PM +0100, Mark Brown wrote: >> On Wed, Apr 09, 2025 at 04:55:52PM +0200, Mathieu Dubois-Briand wrote: >> > BUG() never returns, so code after it is unreachable: remove it. >> >> BUG() can be compiled out, CONFIG_BUG. > > Also, please don't mix irrelevant patches into random serieses. It just > makes everything noisier for everyone. Hi Mark, Just to provide the context about why this change is part of this series: this goto, if left unmodified, would have to replaced by a return. This is how the topic of dropping it came in the previous iteration of this series. Thanks for your review. Mathieu -- Mathieu Dubois-Briand, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
© 2016 - 2025 Red Hat, Inc.