2018-10-15 00:38:27

by Nick Desaulniers

[permalink] [raw]
Subject: [PATCH] x86/mm: annotate no_context with UNWIND_HINTS

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 <[email protected]>
---
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



2018-10-15 03:44:09

by Andy Lutomirski

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

On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers
<[email protected]> 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 <[email protected]>
> ---
> 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

2018-10-15 05:17:58

by Nathan Chancellor

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

On Sun, Oct 14, 2018 at 08:43:18PM -0700, Andy Lutomirski wrote:
> On Sun, Oct 14, 2018 at 5:37 PM Nick Desaulniers
> <[email protected]> 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 <[email protected]>
> > ---
> > 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

2018-10-15 14:23:46

by Josh Poimboeuf

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

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
> > <[email protected]> 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 <[email protected]>
> > > ---
> > > 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

2018-10-15 14:34:56

by Andy Lutomirski

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



> On Oct 14, 2018, at 10:17 PM, Nathan Chancellor <[email protected]> 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
>> <[email protected]> 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 <[email protected]>
>>> ---
>>> 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?

2018-10-15 15:23:26

by Nathan Chancellor

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

On Mon, Oct 15, 2018 at 07:34:14AM -0700, Andy Lutomirski wrote:
>
>
> > On Oct 14, 2018, at 10:17 PM, Nathan Chancellor <[email protected]> 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
> >> <[email protected]> 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 <[email protected]>
> >>> ---
> >>> 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

2018-10-15 15:33:12

by Josh Poimboeuf

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

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

2018-10-15 16:04:46

by Andy Lutomirski

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

On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <[email protected]> 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

2018-10-15 16:10:09

by Nathan Chancellor

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

On Mon, Oct 15, 2018 at 09:03:40AM -0700, Andy Lutomirski wrote:
> On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <[email protected]> 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

2018-10-15 16:21:15

by Josh Poimboeuf

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

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

2018-10-15 16:58:27

by Nick Desaulniers

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

On Mon, Oct 15, 2018 at 9:03 AM Andy Lutomirski <[email protected]> wrote:
>
> On Mon, Oct 15, 2018 at 8:31 AM Josh Poimboeuf <[email protected]> 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!

2018-10-15 17:24:21

by Nick Desaulniers

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

On Mon, Oct 15, 2018 at 9:57 AM Nick Desaulniers
<[email protected]> 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