| Message ID | 20181015172221.35709-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | compiler.h: update definition of unreachable() | expand |
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
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
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
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 /*
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(-)