[PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nick Desaulniers]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Andy Lutomirski]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nathan Chancellor]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Josh Poimboeuf]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Andy Lutomirski]

> 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?

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nathan Chancellor]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Josh Poimboeuf]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Andy Lutomirski]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nathan Chancellor]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Josh Poimboeuf]

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

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nick Desaulniers]

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!

Re: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

[Nick Desaulniers]

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