[PATCH] compiler.h: update definition of unreachable()

[PATCH] compiler.h: update definition of unreachable()

[ndesaulniers]

Fixes the objtool warning seen with Clang:
arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable
instruction

Fixes commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h
mutually exclusive")

Josh noted that the fallback definition was meant to work around a
pre-gcc-4.6 bug. GCC still needs to work around
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365, so compiler-gcc.h
defines its own version of unreachable().  Clang and ICC can use this
shared definition.

Link: https://github.com/ClangBuiltLinux/linux/issues/204
Suggested-by: Andy Lutomirski <luto@amacapital.net>
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Miguel, would you mind taking this up in your new compiler attributes
tree?

 include/linux/compiler.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..8875fd3243fd 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -124,7 +124,10 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 # define ASM_UNREACHABLE
 #endif
 #ifndef unreachable
-# define unreachable() do { annotate_reachable(); do { } while (1); } while (0)
+# define unreachable() do {		\
+	annotate_unreachable();		\
+	__builtin_unreachable();	\
+} while (0)
 #endif
 
 /*
-- 
2.19.0.605.g01d371f741-goog

Re: [PATCH] compiler.h: update definition of unreachable()

[Miguel Ojeda]

On Mon, Oct 15, 2018 at 7:22 PM <ndesaulniers@google.com> wrote:
>
> Fixes the objtool warning seen with Clang:
> arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable
> instruction
>
> Fixes commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h
> mutually exclusive")
>
> Josh noted that the fallback definition was meant to work around a
> pre-gcc-4.6 bug. GCC still needs to work around
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365, so compiler-gcc.h
> defines its own version of unreachable().  Clang and ICC can use this
> shared definition.

Could we, at the same time, update the comment on compiler-gcc.h as
well? i.e. remove the 4.5 comment, add the link to the GCC PR.

>
> Link: https://github.com/ClangBuiltLinux/linux/issues/204
> Suggested-by: Andy Lutomirski <luto@amacapital.net>
> Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Miguel, would you mind taking this up in your new compiler attributes
> tree?

Sure, will do.

Thanks,

Cheers,
Miguel

Re: [PATCH] compiler.h: update definition of unreachable()

[Nick Desaulniers]

On Mon, Oct 15, 2018 at 11:21 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, Oct 15, 2018 at 7:22 PM <ndesaulniers@google.com> wrote:
> >
> > Fixes the objtool warning seen with Clang:
> > arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable
> > instruction
> >
> > Fixes commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h
> > mutually exclusive")
> >
> > Josh noted that the fallback definition was meant to work around a
> > pre-gcc-4.6 bug. GCC still needs to work around
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365, so compiler-gcc.h
> > defines its own version of unreachable().  Clang and ICC can use this
> > shared definition.
>
> Could we, at the same time, update the comment on compiler-gcc.h as
> well? i.e. remove the 4.5 comment, add the link to the GCC PR.

Looking at commit cb984d101b30 ("compiler-gcc: integrate the various
compiler-gcc[345].h files") it seems that the comment is referring to
gcc 4.4 not supporting __builtin_unreachable().  Looks like it was
added in 4.5 timeframe: https://godbolt.org/z/ugv5QO, so the comment
can be deleted.  Would you prefer a v2 (single patch), or an
additional patch on top?

>
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/204
> > Suggested-by: Andy Lutomirski <luto@amacapital.net>
> > Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > Miguel, would you mind taking this up in your new compiler attributes
> > tree?
>
> Sure, will do.
>
> Thanks,
>
> Cheers,
> Miguel



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] compiler.h: update definition of unreachable()

[Miguel Ojeda]

On Mon, Oct 15, 2018 at 10:13 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Looking at commit cb984d101b30 ("compiler-gcc: integrate the various
> compiler-gcc[345].h files") it seems that the comment is referring to
> gcc 4.4 not supporting __builtin_unreachable().  Looks like it was
> added in 4.5 timeframe: https://godbolt.org/z/ugv5QO, so the comment
> can be deleted.  Would you prefer a v2 (single patch), or an
> additional patch on top?

Split would be better, I think, since the changes can be applied
independetly even if related. If you are busy, I can do it myself, so
don't worry too much. :)

Cheers,
Miguel

[PATCH] compiler-gcc: remove comment about gcc 4.5 from unreachable()

[ndesaulniers]

Remove the comment about being unable to detect __builtin_unreachable.
__builtin_unreachable was implemented in the GCC 4.5 timeframe. The
kernel's minimum supported version of GCC is 4.6 since commit
cafa0010cd51 ("Raise the minimum required gcc version to 4.6"). Commit
cb984d101b30 ("compiler-gcc: integrate the various compiler-gcc[345].h
files") shows that unreachable() had different guards based on GCC
version.

Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 include/linux/compiler-gcc.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4d36b27214fd..9890411b33d5 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -107,10 +107,6 @@
  * 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() \
 	do {					\
-- 
2.19.1.331.ge82ca0e54c-goog