* [PATCH] x86/mm: annotate no_context with UNWIND_HINTS @ 2018-10-15 0:37 Nick Desaulniers 2018-10-15 3:43 ` Andy Lutomirski 0 siblings, 1 reply; 12+ messages in thread From: Nick Desaulniers @ 2018-10-15 0:37 UTC (permalink / raw) To: dave.hansen, luto, peterz, tglx, mingo, bp, hpa Cc: natechancellor, Nick Desaulniers, x86, linux-kernel Fixes the objtool warning: arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable instruction Link: https://github.com/ClangBuiltLinux/linux/issues/204 Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> --- arch/x86/mm/fault.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c index 47bebfe6efa7..057d2178fa19 100644 --- a/arch/x86/mm/fault.c +++ b/arch/x86/mm/fault.c @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, * and then double-fault, though, because we're likely to * break the console driver and lose most of the stack dump. */ - asm volatile ("movq %[stack], %%rsp\n\t" + asm volatile (UNWIND_HINT_SAVE + "movq %[stack], %%rsp\n\t" "call handle_stack_overflow\n\t" - "1: jmp 1b" + "1: jmp 1b\n\t" + UNWIND_HINT_RESTORE : ASM_CALL_CONSTRAINT : "D" ("kernel stack overflow (page fault)"), "S" (regs), "d" (address), -- 2.17.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 0:37 [PATCH] x86/mm: annotate no_context with UNWIND_HINTS Nick Desaulniers @ 2018-10-15 3:43 ` Andy Lutomirski 2018-10-15 5:17 ` Nathan Chancellor 0 siblings, 1 reply; 12+ messages in thread From: Andy Lutomirski @ 2018-10-15 3:43 UTC (permalink / raw) To: nick.desaulniers, Josh Poimboeuf Cc: Dave Hansen, Andrew Lutomirski, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, natechancellor, X86 ML, LKML On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers <nick.desaulniers@gmail.com> wrote: > > Fixes the objtool warning: > arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable > instruction > > Link: https://github.com/ClangBuiltLinux/linux/issues/204 > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > --- > arch/x86/mm/fault.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index 47bebfe6efa7..057d2178fa19 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > * and then double-fault, though, because we're likely to > * break the console driver and lose most of the stack dump. > */ > - asm volatile ("movq %[stack], %%rsp\n\t" > + asm volatile (UNWIND_HINT_SAVE > + "movq %[stack], %%rsp\n\t" > "call handle_stack_overflow\n\t" > - "1: jmp 1b" > + "1: jmp 1b\n\t" > + UNWIND_HINT_RESTORE > : ASM_CALL_CONSTRAINT > : "D" ("kernel stack overflow (page fault)"), > "S" (regs), "d" (address), NAK. Just below this snippet is unreachable(); Can you reply with objtool -dr output on a problematic fault.o? Josh, it *looks* like annotate_unreachable() should be doing the right thing, but something is clearly busted. Also, shouldn't compiler-clang.h contain a reasonable definition of unreachable()? --Andy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 3:43 ` Andy Lutomirski @ 2018-10-15 5:17 ` Nathan Chancellor 2018-10-15 14:22 ` Josh Poimboeuf 2018-10-15 14:34 ` Andy Lutomirski 0 siblings, 2 replies; 12+ messages in thread From: Nathan Chancellor @ 2018-10-15 5:17 UTC (permalink / raw) To: Andy Lutomirski Cc: nick.desaulniers, Josh Poimboeuf, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Sun, Oct 14, 2018 at 08:43:18PM -0700, Andy Lutomirski wrote: > On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers > <nick.desaulniers@gmail.com> wrote: > > > > Fixes the objtool warning: > > arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable > > instruction > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/204 > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > > --- > > arch/x86/mm/fault.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > > index 47bebfe6efa7..057d2178fa19 100644 > > --- a/arch/x86/mm/fault.c > > +++ b/arch/x86/mm/fault.c > > @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > * and then double-fault, though, because we're likely to > > * break the console driver and lose most of the stack dump. > > */ > > - asm volatile ("movq %[stack], %%rsp\n\t" > > + asm volatile (UNWIND_HINT_SAVE > > + "movq %[stack], %%rsp\n\t" > > "call handle_stack_overflow\n\t" > > - "1: jmp 1b" > > + "1: jmp 1b\n\t" > > + UNWIND_HINT_RESTORE > > : ASM_CALL_CONSTRAINT > > : "D" ("kernel stack overflow (page fault)"), > > "S" (regs), "d" (address), > > NAK. Just below this snippet is unreachable(); > > Can you reply with objtool -dr output on a problematic fault.o? Josh, > it *looks* like annotate_unreachable() should be doing the right > thing, but something is clearly busted. > > Also, shouldn't compiler-clang.h contain a reasonable definition of > unreachable()? > > --Andy Hi Andy, Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I should have pasted it here instead): https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e Nathan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 5:17 ` Nathan Chancellor @ 2018-10-15 14:22 ` Josh Poimboeuf 2018-10-15 14:34 ` Andy Lutomirski 1 sibling, 0 replies; 12+ messages in thread From: Josh Poimboeuf @ 2018-10-15 14:22 UTC (permalink / raw) To: Nathan Chancellor Cc: Andy Lutomirski, nick.desaulniers, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Sun, Oct 14, 2018 at 10:17:05PM -0700, Nathan Chancellor wrote: > On Sun, Oct 14, 2018 at 08:43:18PM -0700, Andy Lutomirski wrote: > > On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers > > <nick.desaulniers@gmail.com> wrote: > > > > > > Fixes the objtool warning: > > > arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable > > > instruction > > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/204 > > > Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > > > --- > > > arch/x86/mm/fault.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > > > index 47bebfe6efa7..057d2178fa19 100644 > > > --- a/arch/x86/mm/fault.c > > > +++ b/arch/x86/mm/fault.c > > > @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > > * and then double-fault, though, because we're likely to > > > * break the console driver and lose most of the stack dump. > > > */ > > > - asm volatile ("movq %[stack], %%rsp\n\t" > > > + asm volatile (UNWIND_HINT_SAVE > > > + "movq %[stack], %%rsp\n\t" > > > "call handle_stack_overflow\n\t" > > > - "1: jmp 1b" > > > + "1: jmp 1b\n\t" > > > + UNWIND_HINT_RESTORE > > > : ASM_CALL_CONSTRAINT > > > : "D" ("kernel stack overflow (page fault)"), > > > "S" (regs), "d" (address), > > > > NAK. Just below this snippet is unreachable(); > > > > Can you reply with objtool -dr output on a problematic fault.o? Josh, > > it *looks* like annotate_unreachable() should be doing the right > > thing, but something is clearly busted. > > > > Also, shouldn't compiler-clang.h contain a reasonable definition of > > unreachable()? > > > > --Andy > > Hi Andy, > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > should have pasted it here instead): > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e As Andy said, I think compiler-clang.h needs an unreachable() definition. Without that, I'm guessing you're getting this: #ifndef unreachable # define unreachable() do { annotate_reachable(); do { } while (1); } while (0) #endif which converts "unreachable" to "reachable". That used to be needed for pre-4.5 versions of GCC when we supported them, but it's the wrong behavior for this piece of code (and I should remove all those old GCC hacks soon...) -- Josh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 5:17 ` Nathan Chancellor 2018-10-15 14:22 ` Josh Poimboeuf @ 2018-10-15 14:34 ` Andy Lutomirski 2018-10-15 15:22 ` Nathan Chancellor 1 sibling, 1 reply; 12+ messages in thread From: Andy Lutomirski @ 2018-10-15 14:34 UTC (permalink / raw) To: Nathan Chancellor Cc: Andy Lutomirski, nick.desaulniers, Josh Poimboeuf, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML > On Oct 14, 2018, at 10:17 PM, Nathan Chancellor <natechancellor@gmail.com> wrote: > >> On Sun, Oct 14, 2018 at 08:43:18PM -0700, Andy Lutomirski wrote: >> On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers >> <nick.desaulniers@gmail.com> wrote: >>> >>> Fixes the objtool warning: >>> arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable >>> instruction >>> >>> Link: https://github.com/ClangBuiltLinux/linux/issues/204 >>> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> >>> --- >>> arch/x86/mm/fault.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c >>> index 47bebfe6efa7..057d2178fa19 100644 >>> --- a/arch/x86/mm/fault.c >>> +++ b/arch/x86/mm/fault.c >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, >>> * and then double-fault, though, because we're likely to >>> * break the console driver and lose most of the stack dump. >>> */ >>> - asm volatile ("movq %[stack], %%rsp\n\t" >>> + asm volatile (UNWIND_HINT_SAVE >>> + "movq %[stack], %%rsp\n\t" >>> "call handle_stack_overflow\n\t" >>> - "1: jmp 1b" >>> + "1: jmp 1b\n\t" >>> + UNWIND_HINT_RESTORE >>> : ASM_CALL_CONSTRAINT >>> : "D" ("kernel stack overflow (page fault)"), >>> "S" (regs), "d" (address), >> >> NAK. Just below this snippet is unreachable(); >> >> Can you reply with objtool -dr output on a problematic fault.o? Josh, >> it *looks* like annotate_unreachable() should be doing the right >> thing, but something is clearly busted. >> >> Also, shouldn't compiler-clang.h contain a reasonable definition of >> unreachable()? >> >> --Andy > > Hi Andy, > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > should have pasted it here instead): > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 14:34 ` Andy Lutomirski @ 2018-10-15 15:22 ` Nathan Chancellor 2018-10-15 15:31 ` Josh Poimboeuf 0 siblings, 1 reply; 12+ messages in thread From: Nathan Chancellor @ 2018-10-15 15:22 UTC (permalink / raw) To: Andy Lutomirski Cc: Andy Lutomirski, nick.desaulniers, Josh Poimboeuf, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Mon, Oct 15, 2018 at 07:34:14AM -0700, Andy Lutomirski wrote: > > > > On Oct 14, 2018, at 10:17 PM, Nathan Chancellor <natechancellor@gmail.com> wrote: > > > >> On Sun, Oct 14, 2018 at 08:43:18PM -0700, Andy Lutomirski wrote: > >> On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers > >> <nick.desaulniers@gmail.com> wrote: > >>> > >>> Fixes the objtool warning: > >>> arch/x86/mm/fault.o: warning: objtool: no_context()+0x220: unreachable > >>> instruction > >>> > >>> Link: https://github.com/ClangBuiltLinux/linux/issues/204 > >>> Signed-off-by: Nick Desaulniers <nick.desaulniers@gmail.com> > >>> --- > >>> arch/x86/mm/fault.c | 6 ++++-- > >>> 1 file changed, 4 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > >>> index 47bebfe6efa7..057d2178fa19 100644 > >>> --- a/arch/x86/mm/fault.c > >>> +++ b/arch/x86/mm/fault.c > >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > >>> * and then double-fault, though, because we're likely to > >>> * break the console driver and lose most of the stack dump. > >>> */ > >>> - asm volatile ("movq %[stack], %%rsp\n\t" > >>> + asm volatile (UNWIND_HINT_SAVE > >>> + "movq %[stack], %%rsp\n\t" > >>> "call handle_stack_overflow\n\t" > >>> - "1: jmp 1b" > >>> + "1: jmp 1b\n\t" > >>> + UNWIND_HINT_RESTORE > >>> : ASM_CALL_CONSTRAINT > >>> : "D" ("kernel stack overflow (page fault)"), > >>> "S" (regs), "d" (address), > >> > >> NAK. Just below this snippet is unreachable(); > >> > >> Can you reply with objtool -dr output on a problematic fault.o? Josh, > >> it *looks* like annotate_unreachable() should be doing the right > >> thing, but something is clearly busted. > >> > >> Also, shouldn't compiler-clang.h contain a reasonable definition of > >> unreachable()? > >> > >> --Andy > > > > Hi Andy, > > > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > > should have pasted it here instead): > > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > > > > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? Here you go: https://nathanchance.me/downloads/.tmp/fault.o Thanks, Nathan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 15:22 ` Nathan Chancellor @ 2018-10-15 15:31 ` Josh Poimboeuf 2018-10-15 16:03 ` Andy Lutomirski 0 siblings, 1 reply; 12+ messages in thread From: Josh Poimboeuf @ 2018-10-15 15:31 UTC (permalink / raw) To: Nathan Chancellor Cc: Andy Lutomirski, Andy Lutomirski, nick.desaulniers, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Mon, Oct 15, 2018 at 08:22:21AM -0700, Nathan Chancellor wrote: > > >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > >>> * and then double-fault, though, because we're likely to > > >>> * break the console driver and lose most of the stack dump. > > >>> */ > > >>> - asm volatile ("movq %[stack], %%rsp\n\t" > > >>> + asm volatile (UNWIND_HINT_SAVE > > >>> + "movq %[stack], %%rsp\n\t" > > >>> "call handle_stack_overflow\n\t" > > >>> - "1: jmp 1b" > > >>> + "1: jmp 1b\n\t" > > >>> + UNWIND_HINT_RESTORE > > >>> : ASM_CALL_CONSTRAINT > > >>> : "D" ("kernel stack overflow (page fault)"), > > >>> "S" (regs), "d" (address), > > >> > > >> NAK. Just below this snippet is unreachable(); > > >> > > >> Can you reply with objtool -dr output on a problematic fault.o? Josh, > > >> it *looks* like annotate_unreachable() should be doing the right > > >> thing, but something is clearly busted. > > >> > > >> Also, shouldn't compiler-clang.h contain a reasonable definition of > > >> unreachable()? > > >> > > >> --Andy > > > > > > Hi Andy, > > > > > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > > > should have pasted it here instead): > > > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > > > > > > > > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? > > Here you go: https://nathanchance.me/downloads/.tmp/fault.o $ eu-readelf -S /tmp/fault.o |grep reachable [12] .discard.reachable PROGBITS 0000000000000000 00002bc0 00000014 0 0 0 1 [13] .rela.discard.reachable RELA 0000000000000000 00002bd8 00000078 24 I 32 12 8 That confirms that you need a clang version of the unreachable() macro. -- Josh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 15:31 ` Josh Poimboeuf @ 2018-10-15 16:03 ` Andy Lutomirski 2018-10-15 16:07 ` Nathan Chancellor ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Andy Lutomirski @ 2018-10-15 16:03 UTC (permalink / raw) To: Josh Poimboeuf Cc: natechancellor, Andrew Lutomirski, nick.desaulniers, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On Mon, Oct 15, 2018 at 08:22:21AM -0700, Nathan Chancellor wrote: > > > >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > > >>> * and then double-fault, though, because we're likely to > > > >>> * break the console driver and lose most of the stack dump. > > > >>> */ > > > >>> - asm volatile ("movq %[stack], %%rsp\n\t" > > > >>> + asm volatile (UNWIND_HINT_SAVE > > > >>> + "movq %[stack], %%rsp\n\t" > > > >>> "call handle_stack_overflow\n\t" > > > >>> - "1: jmp 1b" > > > >>> + "1: jmp 1b\n\t" > > > >>> + UNWIND_HINT_RESTORE > > > >>> : ASM_CALL_CONSTRAINT > > > >>> : "D" ("kernel stack overflow (page fault)"), > > > >>> "S" (regs), "d" (address), > > > >> > > > >> NAK. Just below this snippet is unreachable(); > > > >> > > > >> Can you reply with objtool -dr output on a problematic fault.o? Josh, > > > >> it *looks* like annotate_unreachable() should be doing the right > > > >> thing, but something is clearly busted. > > > >> > > > >> Also, shouldn't compiler-clang.h contain a reasonable definition of > > > >> unreachable()? > > > >> > > > >> --Andy > > > > > > > > Hi Andy, > > > > > > > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > > > > should have pasted it here instead): > > > > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > > > > > > > > > > > > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? > > > > Here you go: https://nathanchance.me/downloads/.tmp/fault.o > > $ eu-readelf -S /tmp/fault.o |grep reachable > [12] .discard.reachable PROGBITS 0000000000000000 00002bc0 00000014 0 0 0 1 > [13] .rela.discard.reachable RELA 0000000000000000 00002bd8 00000078 24 I 32 12 8 > > That confirms that you need a clang version of the unreachable() macro. > Duh. That being said, the generic macro is: # define unreachable() do { annotate_reachable(); do { } while (1); } while (0) I'm probably missing some subtlety here, but shouldn't that be annotate_*un*reachable()? Of course, there are any number of reasons why there should be a real definition. Nathan and Nick, does adding something like: #define unreachable() \ do { \ annotate_unreachable(); \ __builtin_unreachable(); \ } while (0) to compiler-clang.h fix the problem? --Andy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 16:03 ` Andy Lutomirski @ 2018-10-15 16:07 ` Nathan Chancellor 2018-10-15 16:20 ` Josh Poimboeuf 2018-10-15 16:57 ` Nick Desaulniers 2 siblings, 0 replies; 12+ messages in thread From: Nathan Chancellor @ 2018-10-15 16:07 UTC (permalink / raw) To: Andy Lutomirski Cc: Josh Poimboeuf, Andrew Lutomirski, nick.desaulniers, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Mon, Oct 15, 2018 at 09:03:40AM -0700, Andy Lutomirski wrote: > On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > On Mon, Oct 15, 2018 at 08:22:21AM -0700, Nathan Chancellor wrote: > > > > >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > > > >>> * and then double-fault, though, because we're likely to > > > > >>> * break the console driver and lose most of the stack dump. > > > > >>> */ > > > > >>> - asm volatile ("movq %[stack], %%rsp\n\t" > > > > >>> + asm volatile (UNWIND_HINT_SAVE > > > > >>> + "movq %[stack], %%rsp\n\t" > > > > >>> "call handle_stack_overflow\n\t" > > > > >>> - "1: jmp 1b" > > > > >>> + "1: jmp 1b\n\t" > > > > >>> + UNWIND_HINT_RESTORE > > > > >>> : ASM_CALL_CONSTRAINT > > > > >>> : "D" ("kernel stack overflow (page fault)"), > > > > >>> "S" (regs), "d" (address), > > > > >> > > > > >> NAK. Just below this snippet is unreachable(); > > > > >> > > > > >> Can you reply with objtool -dr output on a problematic fault.o? Josh, > > > > >> it *looks* like annotate_unreachable() should be doing the right > > > > >> thing, but something is clearly busted. > > > > >> > > > > >> Also, shouldn't compiler-clang.h contain a reasonable definition of > > > > >> unreachable()? > > > > >> > > > > >> --Andy > > > > > > > > > > Hi Andy, > > > > > > > > > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > > > > > should have pasted it here instead): > > > > > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > > > > > > > > > > > > > > > > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? > > > > > > Here you go: https://nathanchance.me/downloads/.tmp/fault.o > > > > $ eu-readelf -S /tmp/fault.o |grep reachable > > [12] .discard.reachable PROGBITS 0000000000000000 00002bc0 00000014 0 0 0 1 > > [13] .rela.discard.reachable RELA 0000000000000000 00002bd8 00000078 24 I 32 12 8 > > > > That confirms that you need a clang version of the unreachable() macro. > > > > Duh. > > That being said, the generic macro is: > > # define unreachable() do { annotate_reachable(); do { } while (1); } while (0) > > I'm probably missing some subtlety here, but shouldn't that be > annotate_*un*reachable()? > > Of course, there are any number of reasons why there should be a real > definition. Nathan and Nick, does adding something like: > > #define unreachable() \ > do { \ > annotate_unreachable(); \ > __builtin_unreachable(); \ > } while (0) > > to compiler-clang.h fix the problem? > > --Andy Ha, I was just typing out a message summarizing that that exact definition fixed this warning. Nathan ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 16:03 ` Andy Lutomirski 2018-10-15 16:07 ` Nathan Chancellor @ 2018-10-15 16:20 ` Josh Poimboeuf 2018-10-15 16:57 ` Nick Desaulniers 2 siblings, 0 replies; 12+ messages in thread From: Josh Poimboeuf @ 2018-10-15 16:20 UTC (permalink / raw) To: Andy Lutomirski Cc: natechancellor, Andrew Lutomirski, nick.desaulniers, Dave Hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin, X86 ML, LKML On Mon, Oct 15, 2018 at 09:03:40AM -0700, Andy Lutomirski wrote: > That being said, the generic macro is: > > # define unreachable() do { annotate_reachable(); do { } while (1); } while (0) > > I'm probably missing some subtlety here, but shouldn't that be > annotate_*un*reachable()? That code should have had a comment, but that subtlety was intentional. As I mentioned earlier, that was a hack for old versions of GCC which didn't have __builtin_unreachable(). In those cases, GCC doesn't treat "ud2" as fatal, so this was a way of telling objtool that. Luckily we can get rid of this hack now that the minimum supported GCC version has gone up to 4.6. -- Josh ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 16:03 ` Andy Lutomirski 2018-10-15 16:07 ` Nathan Chancellor 2018-10-15 16:20 ` Josh Poimboeuf @ 2018-10-15 16:57 ` Nick Desaulniers 2018-10-15 17:23 ` Nick Desaulniers 2 siblings, 1 reply; 12+ messages in thread From: Nick Desaulniers @ 2018-10-15 16:57 UTC (permalink / raw) To: luto Cc: Josh Poimboeuf, Nathan Chancellor, luto, dave.hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, bp, H. Peter Anvin, x86, Linux Kernel Mailing List On Mon, Oct 15, 2018 at 9:03 AM Andy Lutomirski <luto@amacapital.net> wrote: > > On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > On Mon, Oct 15, 2018 at 08:22:21AM -0700, Nathan Chancellor wrote: > > > > >>> @@ -760,9 +760,11 @@ no_context(struct pt_regs *regs, unsigned long error_code, > > > > >>> * and then double-fault, though, because we're likely to > > > > >>> * break the console driver and lose most of the stack dump. > > > > >>> */ > > > > >>> - asm volatile ("movq %[stack], %%rsp\n\t" > > > > >>> + asm volatile (UNWIND_HINT_SAVE > > > > >>> + "movq %[stack], %%rsp\n\t" > > > > >>> "call handle_stack_overflow\n\t" > > > > >>> - "1: jmp 1b" > > > > >>> + "1: jmp 1b\n\t" > > > > >>> + UNWIND_HINT_RESTORE > > > > >>> : ASM_CALL_CONSTRAINT > > > > >>> : "D" ("kernel stack overflow (page fault)"), > > > > >>> "S" (regs), "d" (address), > > > > >> > > > > >> NAK. Just below this snippet is unreachable(); > > > > >> > > > > >> Can you reply with objtool -dr output on a problematic fault.o? Josh, > > > > >> it *looks* like annotate_unreachable() should be doing the right > > > > >> thing, but something is clearly busted. > > > > >> > > > > >> Also, shouldn't compiler-clang.h contain a reasonable definition of > > > > >> unreachable()? > > > > >> > > > > >> --Andy > > > > > > > > > > Hi Andy, > > > > > > > > > > Did you mean 'objdump -dr'? If so, here you go (rather long, sorry if I > > > > > should have pasted it here instead): > > > > > https://gist.github.com/nathanchance/f038bb0a6653b975bb8a4e64fcd5503e > > > > > > > > > > > > > > > > > > Hmm, -dr wasn’t quite enough to dump the .discard bits, assuming they’re there at all. Can you just put the whole .o file somewhere? > > > > > > Here you go: https://nathanchance.me/downloads/.tmp/fault.o > > > > $ eu-readelf -S /tmp/fault.o |grep reachable > > [12] .discard.reachable PROGBITS 0000000000000000 00002bc0 00000014 0 0 0 1 > > [13] .rela.discard.reachable RELA 0000000000000000 00002bd8 00000078 24 I 32 12 8 > > > > That confirms that you need a clang version of the unreachable() macro. > > > > Duh. > > That being said, the generic macro is: > > # define unreachable() do { annotate_reachable(); do { } while (1); } while (0) > > I'm probably missing some subtlety here, but shouldn't that be > annotate_*un*reachable()? > > Of course, there are any number of reasons why there should be a real > definition. Nathan and Nick, does adding something like: > > #define unreachable() \ > do { \ > annotate_unreachable(); \ > __builtin_unreachable(); \ > } while (0) > > to compiler-clang.h fix the problem? I broke this myself in commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually exclusive"). Thanks for the suggestion, will verify then send a patch with your suggested by tag. Thanks everyone for helping us sort this out! ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS 2018-10-15 16:57 ` Nick Desaulniers @ 2018-10-15 17:23 ` Nick Desaulniers 0 siblings, 0 replies; 12+ messages in thread From: Nick Desaulniers @ 2018-10-15 17:23 UTC (permalink / raw) To: luto Cc: Josh Poimboeuf, Nathan Chancellor, luto, dave.hansen, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, bp, H. Peter Anvin, x86, Linux Kernel Mailing List On Mon, Oct 15, 2018 at 9:57 AM Nick Desaulniers <nick.desaulniers@gmail.com> wrote: > I broke this myself in commit 815f0ddb346c > ("include/linux/compiler*.h: make compiler-*.h mutually exclusive"). > Thanks for the suggestion, will verify then send a patch with your > suggested by tag. Thanks everyone for helping us sort this out! https://lkml.org/lkml/2018/10/15/626 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-10-15 17:23 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-15 0:37 [PATCH] x86/mm: annotate no_context with UNWIND_HINTS Nick Desaulniers 2018-10-15 3:43 ` Andy Lutomirski 2018-10-15 5:17 ` Nathan Chancellor 2018-10-15 14:22 ` Josh Poimboeuf 2018-10-15 14:34 ` Andy Lutomirski 2018-10-15 15:22 ` Nathan Chancellor 2018-10-15 15:31 ` Josh Poimboeuf 2018-10-15 16:03 ` Andy Lutomirski 2018-10-15 16:07 ` Nathan Chancellor 2018-10-15 16:20 ` Josh Poimboeuf 2018-10-15 16:57 ` Nick Desaulniers 2018-10-15 17:23 ` Nick Desaulniers
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).