bug: mark generic BUG() as unreachable
cover bug: mark generic BUG() as unreachable
From: Tom Rix <[email protected]>
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(-)
Signature · 1 line
2.26.3
PATCH 1/1 bug: mark generic BUG() as unreachable
From: Tom Rix <[email protected]>
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);
}
--- include/asm-generic/bug.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
include/asm-generic/bug.h +4 -1
@@ -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
Signature · 1 line
2.26.3
Arnd Bergmann
maintainerQuoted 9 lines · On Thu, Jun 17, 2021 at 11:44 PM <[email protected]> wrote:
From: Tom Rix <[email protected]>
This spurious error is reported for powerpc64,
CONFIG_BUG=ninclude/asm-generic/bug.h +0
-0
include/asm-generic/bug.h void __warn(const char *file, int line, void *caller, unsigned taint, · line 184
Quoted 9 lines
include/asm-generic/bug.h +4
-1@@ -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
Quoted 29 lines · Arnd Bergmann
Quoted 9 lines · On Thu, Jun 17, 2021 at 11:44 PM <[email protected]> wrote:
From: Tom Rix <[email protected]>
This spurious error is reported for powerpc64,
CONFIG_BUG=ninclude/asm-generic/bug.h +0
-0include/asm-generic/bug.h void __warn(const char *file, int line, void *caller, unsigned taint, · line 184
Quoted 9 lines
include/asm-generic/bug.h +4
-1@@ -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 aBUG()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.
Quoted 5 lines
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
Quoted 3 lines
Arnd
Arnd Bergmann
maintainerQuoted 36 lines · Tom Rix
Quoted 29 lines · Arnd Bergmann
Quoted 9 lines · On Thu, Jun 17, 2021 at 11:44 PM <[email protected]> wrote:
From: Tom Rix <[email protected]>
This spurious error is reported for powerpc64,
CONFIG_BUG=ninclude/asm-generic/bug.h +0
-0include/asm-generic/bug.h void __warn(const char *file, int line, void *caller, unsigned taint, · line 184
Quoted 9 lines
include/asm-generic/bug.h +4
-1@@ -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 aBUG()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.
Quoted 5 lines
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.
Quoted 4 lines
Please follow the link in the cover letter to the original issue reported for
fs/afs/dir+ gcc ppc64 9.x / 10.3.1Adding 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