* [PATCH 0/1] bug: mark generic BUG() as unreachable @ 2021-06-17 21:43 trix 2021-06-17 21:43 ` [PATCH 1/1] " trix 0 siblings, 1 reply; 5+ messages in thread From: trix @ 2021-06-17 21:43 UTC (permalink / raw) To: arnd; +Cc: linux-arch, linux-kernel, Tom Rix From: Tom Rix <trix@redhat.com> This is a followup patch discussed here https://lore.kernel.org/lkml/162375813191.653958.11993495571264748407.stgit@warthog.procyon.org.uk/ Arches that defined BUG() were reviewed either by inspection or by cross building the fs/afs/dir.c reproducer, all lotm. There was no change to the assembly for the ppc64 reproducer. Tom Rix (1): bug: mark generic BUG() as unreachable include/asm-generic/bug.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) -- 2.26.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/1] bug: mark generic BUG() as unreachable 2021-06-17 21:43 [PATCH 0/1] bug: mark generic BUG() as unreachable trix @ 2021-06-17 21:43 ` trix 2021-06-18 8:20 ` Arnd Bergmann 0 siblings, 1 reply; 5+ messages in thread From: trix @ 2021-06-17 21:43 UTC (permalink / raw) To: arnd; +Cc: linux-arch, linux-kernel, Tom Rix From: Tom Rix <trix@redhat.com> This spurious error is reported for powerpc64, CONFIG_BUG=n fs/afs/dir.c: In function 'afs_dir_set_page_dirty': fs/afs/dir.c:51:1: error: no return statement in function returning non-void [-Werror=return-type] 51 | } | ^ When CONFIG_BUG=y is BUG is expanded from #define BUG() do { BUG_ENTRY("twi 31, 0, 0", 0); unreachable(); } while (0) to static int afs_dir_set_page_dirty(struct page *page) { do { __asm__ __volatile__( "1: " "twi 31, 0, 0" ... do { ; asm volatile(""); __builtin_unreachable(); } while (0); } while (0); } When CONFIG_BUG=n, the generic BUG() is used which expands out to static int afs_dir_set_page_dirty(struct page *page) { do {} while (1); } Without the __builtin_unreachable(), gcc reports the warning ref: gcc docs https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html " ... without the __builtin_unreachable, GCC issues a warning that control reaches the end of a non-void function." So add an unreachable() to the generic BUG(), the resulting expansiion will be static int afs_dir_set_page_dirty(struct page *page) { do { do {} while (1); do { ; asm volatile(""); __builtin_unreachable(); } while (0); } while (0); } Signed-off-by: Tom Rix <trix@redhat.com> --- include/asm-generic/bug.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h index f152b9bb916fc..b250e06d7de26 100644 --- a/include/asm-generic/bug.h +++ b/include/asm-generic/bug.h @@ -177,7 +177,10 @@ void __warn(const char *file, int line, void *caller, unsigned taint, #else /* !CONFIG_BUG */ #ifndef HAVE_ARCH_BUG -#define BUG() do {} while (1) +#define BUG() do { \ + do {} while (1); \ + unreachable(); \ + } while (0) #endif #ifndef HAVE_ARCH_BUG_ON -- 2.26.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] bug: mark generic BUG() as unreachable 2021-06-17 21:43 ` [PATCH 1/1] " trix @ 2021-06-18 8:20 ` Arnd Bergmann 2021-06-18 13:43 ` Tom Rix 0 siblings, 1 reply; 5+ messages in thread From: Arnd Bergmann @ 2021-06-18 8:20 UTC (permalink / raw) To: Tom Rix; +Cc: linux-arch, Linux Kernel Mailing List On Thu, Jun 17, 2021 at 11:44 PM <trix@redhat.com> wrote: > From: Tom Rix <trix@redhat.com> > > This spurious error is reported for powerpc64, CONFIG_BUG=n > > diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h > index f152b9bb916fc..b250e06d7de26 100644 > --- a/include/asm-generic/bug.h > +++ b/include/asm-generic/bug.h > @@ -177,7 +177,10 @@ void __warn(const char *file, int line, void *caller, unsigned taint, > > #else /* !CONFIG_BUG */ > #ifndef HAVE_ARCH_BUG > -#define BUG() do {} while (1) > +#define BUG() do { \ > + do {} while (1); \ > + unreachable(); \ > + } while (0) > #endif Please let's not go back to this version, we had good reasons to use the infinite loop, mostly to avoid undefined behavior that would lead to the compiler producing completely random output in code paths that lead to a BUG() statement. Those do cause other kinds of warnings from objtool and from other compilers. The obvious workaround here would be to add a return statement locally, but it may also help to figure out what exactly triggers the warning, as I don't see it in my randconfig builds and it may be that there is a bug elsewhere. I've tried a simple reproducer on https://godbolt.org/z/341P949bG that did not show this warning in any of the compilers I tried. Can you try to narrow down the exact compiler versions and commmand line options that produce the warning? https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/ has most of the supported gcc versions in case you need those. Arnd ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] bug: mark generic BUG() as unreachable 2021-06-18 8:20 ` Arnd Bergmann @ 2021-06-18 13:43 ` Tom Rix 2021-06-18 14:35 ` Arnd Bergmann 0 siblings, 1 reply; 5+ messages in thread From: Tom Rix @ 2021-06-18 13:43 UTC (permalink / raw) To: Arnd Bergmann; +Cc: linux-arch, Linux Kernel Mailing List On 6/18/21 1:20 AM, Arnd Bergmann wrote: > On Thu, Jun 17, 2021 at 11:44 PM <trix@redhat.com> wrote: >> From: Tom Rix <trix@redhat.com> >> >> This spurious error is reported for powerpc64, CONFIG_BUG=n >> >> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h >> index f152b9bb916fc..b250e06d7de26 100644 >> --- a/include/asm-generic/bug.h >> +++ b/include/asm-generic/bug.h >> @@ -177,7 +177,10 @@ void __warn(const char *file, int line, void *caller, unsigned taint, >> >> #else /* !CONFIG_BUG */ >> #ifndef HAVE_ARCH_BUG >> -#define BUG() do {} while (1) >> +#define BUG() do { \ >> + do {} while (1); \ >> + unreachable(); \ >> + } while (0) >> #endif > Please let's not go back to this version, we had good reasons to use > the infinite loop, > mostly to avoid undefined behavior that would lead to the compiler producing > completely random output in code paths that lead to a BUG() statement. Those > do cause other kinds of warnings from objtool and from other compilers. > > The obvious workaround here would be to add a return statement locally, but > it may also help to figure out what exactly triggers the warning, as I don't see > it in my randconfig builds and it may be that there is a bug elsewhere. > > I've tried a simple reproducer on https://godbolt.org/z/341P949bG that did not > show this warning in any of the compilers I tried. Can you try to narrow down > the exact compiler versions and commmand line options that produce the > warning? https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/ has > most of the supported gcc versions in case you need those. Please follow the link in the cover letter to the original issue reported for fs/afs/dir + gcc ppc64 9.x / 10.3.1 Adding the return was the first, rejected solution. Tom > > Arnd > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] bug: mark generic BUG() as unreachable 2021-06-18 13:43 ` Tom Rix @ 2021-06-18 14:35 ` Arnd Bergmann 0 siblings, 0 replies; 5+ messages in thread From: Arnd Bergmann @ 2021-06-18 14:35 UTC (permalink / raw) To: Tom Rix; +Cc: linux-arch, Linux Kernel Mailing List On Fri, Jun 18, 2021 at 3:43 PM Tom Rix <trix@redhat.com> wrote: > On 6/18/21 1:20 AM, Arnd Bergmann wrote: > > On Thu, Jun 17, 2021 at 11:44 PM <trix@redhat.com> wrote: > >> From: Tom Rix <trix@redhat.com> > >> > >> This spurious error is reported for powerpc64, CONFIG_BUG=n > >> > >> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h > >> index f152b9bb916fc..b250e06d7de26 100644 > >> --- a/include/asm-generic/bug.h > >> +++ b/include/asm-generic/bug.h > >> @@ -177,7 +177,10 @@ void __warn(const char *file, int line, void *caller, unsigned taint, > >> > >> #else /* !CONFIG_BUG */ > >> #ifndef HAVE_ARCH_BUG > >> -#define BUG() do {} while (1) > >> +#define BUG() do { \ > >> + do {} while (1); \ > >> + unreachable(); \ > >> + } while (0) > >> #endif > > Please let's not go back to this version, we had good reasons to use > > the infinite loop, > > mostly to avoid undefined behavior that would lead to the compiler producing > > completely random output in code paths that lead to a BUG() statement. Those > > do cause other kinds of warnings from objtool and from other compilers. > > > > The obvious workaround here would be to add a return statement locally, but > > it may also help to figure out what exactly triggers the warning, as I don't see > > it in my randconfig builds and it may be that there is a bug elsewhere. > > > > I've tried a simple reproducer on https://godbolt.org/z/341P949bG that did not > > show this warning in any of the compilers I tried. Can you try to narrow down > > the exact compiler versions and commmand line options that produce the > > warning? https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/ has > > most of the supported gcc versions in case you need those. > > Please follow the link in the cover letter to the original issue > reported for fs/afs/dir + gcc ppc64 9.x / 10.3.1 > > Adding the return was the first, rejected solution. Ok, I was able to reproduce it now and have a better idea of what is going on. I also misread your patch (sorry about that), and missed that you keep the "do {} while (1)" loop ahead of the unreachable(), and that should address all the concerns I had. I would still try to find the old email thread about the change, just to make sure we don't already know about other problems with your version. Arnd ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-18 14:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-17 21:43 [PATCH 0/1] bug: mark generic BUG() as unreachable trix 2021-06-17 21:43 ` [PATCH 1/1] " trix 2021-06-18 8:20 ` Arnd Bergmann 2021-06-18 13:43 ` Tom Rix 2021-06-18 14:35 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).