Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-25 Thread Sami Tolvanen
On Fri, Jan 22, 2021 at 6:26 PM Josh Poimboeuf  wrote:
>
> On Fri, Jan 22, 2021 at 05:32:43PM -0800, Nick Desaulniers wrote:
> > > In this specific case, find_func_by_offset returns NULL for
> > > .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> > > jump table symbols for static functions:
> > >
> > >  <__typeid__ZTSFjmiE_global_addr>:
> > >0:   e9 00 00 00 00  jmpq   5 
> > > <__typeid__ZTSFjmiE_global_addr+0x5>
> > > 1: R_X86_64_PLT32   io_serial_in-0x4
> > >5:   cc  int3
> > >6:   cc  int3
> > >7:   cc  int3
> > >8:   e9 00 00 00 00  jmpq   d 
> > > <__typeid__ZTSFjmiE_global_addr+0xd>
> > > 9: R_X86_64_PLT32   mem32_serial_in-0x4
> > >d:   cc  int3
> > >e:   cc  int3
> > >f:   cc  int3
> > >
> > > Nick, do you remember if there were plans to change this?
> >
> > Do you have a link to any previous discussion to help jog my mind; I
> > don't really remember this one.
> >
> > Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
> > table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
> > the symbol table?
>
> I think he means there's not a 'mem32_serial_in.cfi_jt' symbol at
> '__typeid__ZTSFjmiE_global_addr+8'.  Probably more aggressive symbol
> pruning from the assembler.

Correct, the compiler is not emitting mem32_serial_in.cfi_jt here. I
seem to recall that the missing jump table symbols also made stack
traces harder to follow (__typeid__ZTSFjmiE_global_addr+8 is not very
readable), so ideally they shouldn't be pruned.

> It's fine though.  I just need to rewrite the CFI support a bit.
>
> But that can come later.  For now I'll just drop the two CFI-related
> patches from this set so I can merge the others next week.

Sure, sounds good.

Sami


Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Sedat Dilek
On Sat, Jan 23, 2021 at 3:46 AM Josh Poimboeuf  wrote:
>
> On Sat, Jan 23, 2021 at 03:31:20AM +0100, Sedat Dilek wrote:
> > On Sat, Jan 23, 2021 at 3:26 AM Josh Poimboeuf  wrote:
> > >
> > > On Fri, Jan 22, 2021 at 05:32:43PM -0800, Nick Desaulniers wrote:
> > > > > In this specific case, find_func_by_offset returns NULL for
> > > > > .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> > > > > jump table symbols for static functions:
> > > > >
> > > > >  <__typeid__ZTSFjmiE_global_addr>:
> > > > >0:   e9 00 00 00 00  jmpq   5 
> > > > > <__typeid__ZTSFjmiE_global_addr+0x5>
> > > > > 1: R_X86_64_PLT32   io_serial_in-0x4
> > > > >5:   cc  int3
> > > > >6:   cc  int3
> > > > >7:   cc  int3
> > > > >8:   e9 00 00 00 00  jmpq   d 
> > > > > <__typeid__ZTSFjmiE_global_addr+0xd>
> > > > > 9: R_X86_64_PLT32   mem32_serial_in-0x4
> > > > >d:   cc  int3
> > > > >e:   cc  int3
> > > > >f:   cc  int3
> > > > >
> > > > > Nick, do you remember if there were plans to change this?
> > > >
> > > > Do you have a link to any previous discussion to help jog my mind; I
> > > > don't really remember this one.
> > > >
> > > > Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
> > > > table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
> > > > the symbol table?
> > >
> > > I think he means there's not a 'mem32_serial_in.cfi_jt' symbol at
> > > '__typeid__ZTSFjmiE_global_addr+8'.  Probably more aggressive symbol
> > > pruning from the assembler.
> > >
> > > It's fine though.  I just need to rewrite the CFI support a bit.
> > >
> > > But that can come later.  For now I'll just drop the two CFI-related
> > > patches from this set so I can merge the others next week.
> > >
> >
> > Two CFI-related patches?
> >
> > What's the other than "objtool: Add CONFIG_CFI_CLANG support"?
>
> I was referring to patches 10 and 11:
>
>   objtool: Move unsuffixed symbol conversion to a helper function
>   objtool: Add CONFIG_CFI_CLANG support
>

OK, thanks.

I will test with a revert of these two patches in another scenario
where I have problems with Clang's IAS but not with GNU AS.

- Sedat -

> You can just drop those patches from your testing (and don't test with
> CFI).
>
> > Do you plan (or offer) a v3 of objtool-vmlinux?
>
> Not unless there are any more comments.
>
> --
> Josh
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/clang-built-linux/20210123024634.qu62tyq3nkqusken%40treble.


Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Josh Poimboeuf
On Sat, Jan 23, 2021 at 03:31:20AM +0100, Sedat Dilek wrote:
> On Sat, Jan 23, 2021 at 3:26 AM Josh Poimboeuf  wrote:
> >
> > On Fri, Jan 22, 2021 at 05:32:43PM -0800, Nick Desaulniers wrote:
> > > > In this specific case, find_func_by_offset returns NULL for
> > > > .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> > > > jump table symbols for static functions:
> > > >
> > > >  <__typeid__ZTSFjmiE_global_addr>:
> > > >0:   e9 00 00 00 00  jmpq   5 
> > > > <__typeid__ZTSFjmiE_global_addr+0x5>
> > > > 1: R_X86_64_PLT32   io_serial_in-0x4
> > > >5:   cc  int3
> > > >6:   cc  int3
> > > >7:   cc  int3
> > > >8:   e9 00 00 00 00  jmpq   d 
> > > > <__typeid__ZTSFjmiE_global_addr+0xd>
> > > > 9: R_X86_64_PLT32   mem32_serial_in-0x4
> > > >d:   cc  int3
> > > >e:   cc  int3
> > > >f:   cc  int3
> > > >
> > > > Nick, do you remember if there were plans to change this?
> > >
> > > Do you have a link to any previous discussion to help jog my mind; I
> > > don't really remember this one.
> > >
> > > Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
> > > table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
> > > the symbol table?
> >
> > I think he means there's not a 'mem32_serial_in.cfi_jt' symbol at
> > '__typeid__ZTSFjmiE_global_addr+8'.  Probably more aggressive symbol
> > pruning from the assembler.
> >
> > It's fine though.  I just need to rewrite the CFI support a bit.
> >
> > But that can come later.  For now I'll just drop the two CFI-related
> > patches from this set so I can merge the others next week.
> >
> 
> Two CFI-related patches?
> 
> What's the other than "objtool: Add CONFIG_CFI_CLANG support"?

I was referring to patches 10 and 11:

  objtool: Move unsuffixed symbol conversion to a helper function
  objtool: Add CONFIG_CFI_CLANG support

You can just drop those patches from your testing (and don't test with
CFI).

> Do you plan (or offer) a v3 of objtool-vmlinux?

Not unless there are any more comments.

-- 
Josh



Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Sedat Dilek
On Sat, Jan 23, 2021 at 3:26 AM Josh Poimboeuf  wrote:
>
> On Fri, Jan 22, 2021 at 05:32:43PM -0800, Nick Desaulniers wrote:
> > > In this specific case, find_func_by_offset returns NULL for
> > > .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> > > jump table symbols for static functions:
> > >
> > >  <__typeid__ZTSFjmiE_global_addr>:
> > >0:   e9 00 00 00 00  jmpq   5 
> > > <__typeid__ZTSFjmiE_global_addr+0x5>
> > > 1: R_X86_64_PLT32   io_serial_in-0x4
> > >5:   cc  int3
> > >6:   cc  int3
> > >7:   cc  int3
> > >8:   e9 00 00 00 00  jmpq   d 
> > > <__typeid__ZTSFjmiE_global_addr+0xd>
> > > 9: R_X86_64_PLT32   mem32_serial_in-0x4
> > >d:   cc  int3
> > >e:   cc  int3
> > >f:   cc  int3
> > >
> > > Nick, do you remember if there were plans to change this?
> >
> > Do you have a link to any previous discussion to help jog my mind; I
> > don't really remember this one.
> >
> > Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
> > table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
> > the symbol table?
>
> I think he means there's not a 'mem32_serial_in.cfi_jt' symbol at
> '__typeid__ZTSFjmiE_global_addr+8'.  Probably more aggressive symbol
> pruning from the assembler.
>
> It's fine though.  I just need to rewrite the CFI support a bit.
>
> But that can come later.  For now I'll just drop the two CFI-related
> patches from this set so I can merge the others next week.
>

Two CFI-related patches?

What's the other than "objtool: Add CONFIG_CFI_CLANG support"?

Do you plan (or offer) a v3 of objtool-vmlinux?

Thanks.

- Sedat -

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/commit/?h=objtool-vmlinux-v2=d743f4b36e120c06506567a9f87a062ae03da47f


Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Josh Poimboeuf
On Fri, Jan 22, 2021 at 05:32:43PM -0800, Nick Desaulniers wrote:
> > In this specific case, find_func_by_offset returns NULL for
> > .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> > jump table symbols for static functions:
> >
> >  <__typeid__ZTSFjmiE_global_addr>:
> >0:   e9 00 00 00 00  jmpq   5 
> > <__typeid__ZTSFjmiE_global_addr+0x5>
> > 1: R_X86_64_PLT32   io_serial_in-0x4
> >5:   cc  int3
> >6:   cc  int3
> >7:   cc  int3
> >8:   e9 00 00 00 00  jmpq   d 
> > <__typeid__ZTSFjmiE_global_addr+0xd>
> > 9: R_X86_64_PLT32   mem32_serial_in-0x4
> >d:   cc  int3
> >e:   cc  int3
> >f:   cc  int3
> >
> > Nick, do you remember if there were plans to change this?
> 
> Do you have a link to any previous discussion to help jog my mind; I
> don't really remember this one.
> 
> Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
> table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
> the symbol table?

I think he means there's not a 'mem32_serial_in.cfi_jt' symbol at
'__typeid__ZTSFjmiE_global_addr+8'.  Probably more aggressive symbol
pruning from the assembler.

It's fine though.  I just need to rewrite the CFI support a bit.

But that can come later.  For now I'll just drop the two CFI-related
patches from this set so I can merge the others next week.

-- 
Josh



Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Nick Desaulniers
On Fri, Jan 22, 2021 at 1:17 PM Sami Tolvanen  wrote:
>
> Hi Josh,
>
> On Fri, Jan 22, 2021 at 7:42 AM Josh Poimboeuf  wrote:
> >
> > On Thu, Jan 21, 2021 at 11:38:54PM +0100, Sedat Dilek wrote:
> > > On Thu, Jan 21, 2021 at 10:29 PM Josh Poimboeuf  
> > > wrote:
> > > >
> > > > v2:
> > > > - fix commit description for why xen hypervisor page contents don't
> > > >   matter [Juergen]
> > > > - annotate indirect jumps as safe instead of converting them to
> > > >   retpolines [Andrew, Juergen]
> > > > - drop patch 1 - fake jumps no longer exist
> > > > - add acks
> > > >
> > > > Based on tip/objtool/core.
> > > >
> > > >
> > > > Add support for proper vmlinux.o validation, which will be needed for
> > > > Sami's upcoming x86 LTO set.  (And vmlinux validation is the future for
> > > > objtool anyway, for other reasons.)
> > > >
> > > > This isn't 100% done -- most notably, crypto still needs to be supported
> > > > -- but I think this gets us most of the way there.
> > > >
> > > > This can also be found at
> > > >
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> > > > objtool-vmlinux
> > > >
> > >
> > > Should this be s/objtool-vmlinux/objtool-vmlinux-v2 ?
> >
> > Indeed:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> > objtool-vmlinux-v2
>
> I tested v2 on top of the LTO tree and with allyesconfig + relevant
> crypto configs disabled, and I only see the warnings I reported
> earlier.
>
> I also tested this on top of the CFI tree and with LTO+CFI enabled, I
> can reproduce the segfault Sedat reported in the previous thread. This
> happens because find_unsuffixed_func is called with a NULL sym in
> read_relocs:
>
> Program received signal SIGSEGV, Segmentation fault.
> find_unsuffixed_func (elf=elf@entry=0x755a5010, sym=0x0,
> suffix=0x416cbf ".cfi_jt", func=func@entry=0x7fffd5f0) at
> elf.c:274
> 274 loc = strstr(sym->name, suffix);
> (gdb) bt
> #0  find_unsuffixed_func (elf=elf@entry=0x755a5010, sym=0x0,
> suffix=0x416cbf ".cfi_jt", func=func@entry=0x7fffd5f0) at
> elf.c:274
> #1  0x0040d8f8 in read_relocs (elf=0x755a5010) at elf.c:637
> ...
>
> In this specific case, find_func_by_offset returns NULL for
> .text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
> jump table symbols for static functions:
>
>  <__typeid__ZTSFjmiE_global_addr>:
>0:   e9 00 00 00 00  jmpq   5 <__typeid__ZTSFjmiE_global_addr+0x5>
> 1: R_X86_64_PLT32   io_serial_in-0x4
>5:   cc  int3
>6:   cc  int3
>7:   cc  int3
>8:   e9 00 00 00 00  jmpq   d <__typeid__ZTSFjmiE_global_addr+0xd>
> 9: R_X86_64_PLT32   mem32_serial_in-0x4
>d:   cc  int3
>e:   cc  int3
>f:   cc  int3
>
> Nick, do you remember if there were plans to change this?

Do you have a link to any previous discussion to help jog my mind; I
don't really remember this one.

Is it that `__typeid__ZTSFjmiE_global_addr` is the synthesized jump
table, and yet there is no `__typeid__ZTSFjmiE_global_addr` entry in
the symbol table?
-- 
Thanks,
~Nick Desaulniers


Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Sami Tolvanen
Hi Josh,

On Fri, Jan 22, 2021 at 7:42 AM Josh Poimboeuf  wrote:
>
> On Thu, Jan 21, 2021 at 11:38:54PM +0100, Sedat Dilek wrote:
> > On Thu, Jan 21, 2021 at 10:29 PM Josh Poimboeuf  wrote:
> > >
> > > v2:
> > > - fix commit description for why xen hypervisor page contents don't
> > >   matter [Juergen]
> > > - annotate indirect jumps as safe instead of converting them to
> > >   retpolines [Andrew, Juergen]
> > > - drop patch 1 - fake jumps no longer exist
> > > - add acks
> > >
> > > Based on tip/objtool/core.
> > >
> > >
> > > Add support for proper vmlinux.o validation, which will be needed for
> > > Sami's upcoming x86 LTO set.  (And vmlinux validation is the future for
> > > objtool anyway, for other reasons.)
> > >
> > > This isn't 100% done -- most notably, crypto still needs to be supported
> > > -- but I think this gets us most of the way there.
> > >
> > > This can also be found at
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> > > objtool-vmlinux
> > >
> >
> > Should this be s/objtool-vmlinux/objtool-vmlinux-v2 ?
>
> Indeed:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> objtool-vmlinux-v2

I tested v2 on top of the LTO tree and with allyesconfig + relevant
crypto configs disabled, and I only see the warnings I reported
earlier.

I also tested this on top of the CFI tree and with LTO+CFI enabled, I
can reproduce the segfault Sedat reported in the previous thread. This
happens because find_unsuffixed_func is called with a NULL sym in
read_relocs:

Program received signal SIGSEGV, Segmentation fault.
find_unsuffixed_func (elf=elf@entry=0x755a5010, sym=0x0,
suffix=0x416cbf ".cfi_jt", func=func@entry=0x7fffd5f0) at
elf.c:274
274 loc = strstr(sym->name, suffix);
(gdb) bt
#0  find_unsuffixed_func (elf=elf@entry=0x755a5010, sym=0x0,
suffix=0x416cbf ".cfi_jt", func=func@entry=0x7fffd5f0) at
elf.c:274
#1  0x0040d8f8 in read_relocs (elf=0x755a5010) at elf.c:637
...

In this specific case, find_func_by_offset returns NULL for
.text..L.cfi.jumptable.43 at addend 0x8, because Clang doesn't emit
jump table symbols for static functions:

 <__typeid__ZTSFjmiE_global_addr>:
   0:   e9 00 00 00 00  jmpq   5 <__typeid__ZTSFjmiE_global_addr+0x5>
1: R_X86_64_PLT32   io_serial_in-0x4
   5:   cc  int3
   6:   cc  int3
   7:   cc  int3
   8:   e9 00 00 00 00  jmpq   d <__typeid__ZTSFjmiE_global_addr+0xd>
9: R_X86_64_PLT32   mem32_serial_in-0x4
   d:   cc  int3
   e:   cc  int3
   f:   cc  int3

Nick, do you remember if there were plans to change this?

Sami


Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-22 Thread Josh Poimboeuf
On Thu, Jan 21, 2021 at 11:38:54PM +0100, Sedat Dilek wrote:
> On Thu, Jan 21, 2021 at 10:29 PM Josh Poimboeuf  wrote:
> >
> > v2:
> > - fix commit description for why xen hypervisor page contents don't
> >   matter [Juergen]
> > - annotate indirect jumps as safe instead of converting them to
> >   retpolines [Andrew, Juergen]
> > - drop patch 1 - fake jumps no longer exist
> > - add acks
> >
> > Based on tip/objtool/core.
> >
> >
> > Add support for proper vmlinux.o validation, which will be needed for
> > Sami's upcoming x86 LTO set.  (And vmlinux validation is the future for
> > objtool anyway, for other reasons.)
> >
> > This isn't 100% done -- most notably, crypto still needs to be supported
> > -- but I think this gets us most of the way there.
> >
> > This can also be found at
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> > objtool-vmlinux
> >
> 
> Should this be s/objtool-vmlinux/objtool-vmlinux-v2 ?

Indeed:

  git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
objtool-vmlinux-v2

-- 
Josh



Re: [PATCH v2 00/20] objtool: vmlinux.o and CLANG LTO support

2021-01-21 Thread Sedat Dilek
On Thu, Jan 21, 2021 at 10:29 PM Josh Poimboeuf  wrote:
>
> v2:
> - fix commit description for why xen hypervisor page contents don't
>   matter [Juergen]
> - annotate indirect jumps as safe instead of converting them to
>   retpolines [Andrew, Juergen]
> - drop patch 1 - fake jumps no longer exist
> - add acks
>
> Based on tip/objtool/core.
>
>
> Add support for proper vmlinux.o validation, which will be needed for
> Sami's upcoming x86 LTO set.  (And vmlinux validation is the future for
> objtool anyway, for other reasons.)
>
> This isn't 100% done -- most notably, crypto still needs to be supported
> -- but I think this gets us most of the way there.
>
> This can also be found at
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git 
> objtool-vmlinux
>

Should this be s/objtool-vmlinux/objtool-vmlinux-v2 ?

- Sedat -

> And for more testing it can be combined with Sami's x86 LTO patches:
>
>   https://github.com/samitolvanen/linux clang-lto
>
>
> Josh Poimboeuf (20):
>   objtool: Fix error handling for STD/CLD warnings
>   objtool: Fix retpoline detection in asm code
>   objtool: Fix ".cold" section suffix check for newer versions of GCC
>   objtool: Support retpoline jump detection for vmlinux.o
>   x86/ftrace: Add UNWIND_HINT_FUNC annotation for ftrace_stub
>   objtool: Assume only ELF functions do sibling calls
>   objtool: Add asm version of STACK_FRAME_NON_STANDARD
>   objtool: Combine UNWIND_HINT_RET_OFFSET and UNWIND_HINT_FUNC
>   objtool: Add xen_start_kernel() to noreturn list
>   objtool: Move unsuffixed symbol conversion to a helper function
>   objtool: Add CONFIG_CFI_CLANG support
>   x86/xen: Support objtool validation in xen-asm.S
>   x86/xen: Support objtool vmlinux.o validation in xen-head.S
>   x86/xen/pvh: Annotate indirect branch as safe
>   x86/ftrace: Support objtool vmlinux.o validation in ftrace_64.S
>   x86/acpi: Annotate indirect branch as safe
>   x86/acpi: Support objtool validation in wakeup_64.S
>   x86/power: Annotate indirect branches as safe
>   x86/power: Move restore_registers() to top of the file
>   x86/power: Support objtool validation in hibernate_asm_64.S
>
>  arch/x86/include/asm/unwind_hints.h   |  13 +---
>  arch/x86/kernel/acpi/Makefile |   1 -
>  arch/x86/kernel/acpi/wakeup_64.S  |   4 +
>  arch/x86/kernel/ftrace_64.S   |   8 +-
>  arch/x86/lib/retpoline.S  |   2 +-
>  arch/x86/platform/pvh/head.S  |   2 +
>  arch/x86/power/Makefile   |   1 -
>  arch/x86/power/hibernate_asm_64.S | 103 +-
>  arch/x86/xen/Makefile |   1 -
>  arch/x86/xen/xen-asm.S|  29 +---
>  arch/x86/xen/xen-head.S   |   5 +-
>  include/linux/objtool.h   |  13 +++-
>  tools/include/linux/objtool.h |  13 +++-
>  tools/objtool/arch/x86/decode.c   |   4 +-
>  tools/objtool/arch/x86/special.c  |   2 +-
>  tools/objtool/check.c |  89 --
>  tools/objtool/elf.c   |  88 --
>  tools/objtool/include/objtool/check.h |  12 ++-
>  tools/objtool/include/objtool/elf.h   |   2 +-
>  19 files changed, 240 insertions(+), 152 deletions(-)
>
> --
> 2.29.2
>