annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)

annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)

[Josh Poimboeuf]

On Tue, Nov 12, 2024 at 08:26:56PM +0800, Tiezhu Yang wrote:
> On 11/12/2024 11:15 AM, Xi Ruoyao wrote:
> > On Wed, 2024-11-06 at 13:03 +0800, Tiezhu Yang wrote:
> > > On 11/05/2024 10:15 PM, Peter Zijlstra wrote:
> > > > On Tue, Nov 05, 2024 at 08:39:06PM +0800, Tiezhu Yang wrote:
> > > > > For now, it is time to remove the compiler option -fno-jump-tables
> > > > > to enable jump table for objtool if the compiler is GCC and it has
> > > > > the compiler option -mannotate-tablejump, otherwise still keep the
> > > > > compiler option -fno-jump-tables to maintain compatibility with the
> > > > > older compilers.
> 
> ...
> 
> > > ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
> > > KBUILD_CFLAGS                   += $(call cc-option,-mannotate-tablejump)
> > > else
> > > KBUILD_CFLAGS                   += -fno-jump-tables
> > > endif
> > 
> > Has -mannotate-tablejump been added to Clang?
> 
> Yes.
> 
> > IMO it's better to add it
> > to Clang first, and add Clang & GCC support at once into objtool.
> 
> Looks reasonable, the fact is that there are some corner issues
> compiled with Clang due to different compiler behaviors, most of
> the issues have been addressed and I need to do more test, I will
> send v3 with about 10 patches after the coming merge window.

Hm, I didn't know -mannotate-tablejump existed.  We really need
something which supports all arches, not just loongarch.

Others were looking at adding something similar (adding them to Cc).

-- 
Josh

Re: annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)

[Nick Desaulniers]

On Wed, Nov 13, 2024 at 1:11 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Nov 12, 2024 at 08:26:56PM +0800, Tiezhu Yang wrote:
> > On 11/12/2024 11:15 AM, Xi Ruoyao wrote:
> > > On Wed, 2024-11-06 at 13:03 +0800, Tiezhu Yang wrote:
> > > > On 11/05/2024 10:15 PM, Peter Zijlstra wrote:
> > > > > On Tue, Nov 05, 2024 at 08:39:06PM +0800, Tiezhu Yang wrote:
> > > > > > For now, it is time to remove the compiler option -fno-jump-tables
> > > > > > to enable jump table for objtool if the compiler is GCC and it has
> > > > > > the compiler option -mannotate-tablejump, otherwise still keep the
> > > > > > compiler option -fno-jump-tables to maintain compatibility with the
> > > > > > older compilers.
> >
> > ...
> >
> > > > ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
> > > > KBUILD_CFLAGS                   += $(call cc-option,-mannotate-tablejump)
> > > > else
> > > > KBUILD_CFLAGS                   += -fno-jump-tables
> > > > endif
> > >
> > > Has -mannotate-tablejump been added to Clang?
> >
> > Yes.
> >
> > > IMO it's better to add it
> > > to Clang first, and add Clang & GCC support at once into objtool.
> >
> > Looks reasonable, the fact is that there are some corner issues
> > compiled with Clang due to different compiler behaviors, most of
> > the issues have been addressed and I need to do more test, I will
> > send v3 with about 10 patches after the coming merge window.
>
> Hm, I didn't know -mannotate-tablejump existed.  We really need
> something which supports all arches, not just loongarch.
>
> Others were looking at adding something similar (adding them to Cc).

Looks like this was added to clang in:
https://github.com/llvm/llvm-project/pull/102411

A comment in llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
describes the scheme:
+  // Emit an additional section to store the correlation info as pairs of
+  // addresses, each pair contains the address of a jump instruction (jr) and
+  // the address of the jump table.

Ard had a prototype in:
https://github.com/llvm/llvm-project/pull/112606
which used relocations rather than a discardable section.
-- 
Thanks,
~Nick Desaulniers

Re: annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)

[Ard Biesheuvel]

On Thu, 14 Nov 2024 at 18:13, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Wed, Nov 13, 2024 at 1:11 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Tue, Nov 12, 2024 at 08:26:56PM +0800, Tiezhu Yang wrote:
> > > On 11/12/2024 11:15 AM, Xi Ruoyao wrote:
> > > > On Wed, 2024-11-06 at 13:03 +0800, Tiezhu Yang wrote:
> > > > > On 11/05/2024 10:15 PM, Peter Zijlstra wrote:
> > > > > > On Tue, Nov 05, 2024 at 08:39:06PM +0800, Tiezhu Yang wrote:
> > > > > > > For now, it is time to remove the compiler option -fno-jump-tables
> > > > > > > to enable jump table for objtool if the compiler is GCC and it has
> > > > > > > the compiler option -mannotate-tablejump, otherwise still keep the
> > > > > > > compiler option -fno-jump-tables to maintain compatibility with the
> > > > > > > older compilers.
> > >
> > > ...
> > >
> > > > > ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
> > > > > KBUILD_CFLAGS                   += $(call cc-option,-mannotate-tablejump)
> > > > > else
> > > > > KBUILD_CFLAGS                   += -fno-jump-tables
> > > > > endif
> > > >
> > > > Has -mannotate-tablejump been added to Clang?
> > >
> > > Yes.
> > >
> > > > IMO it's better to add it
> > > > to Clang first, and add Clang & GCC support at once into objtool.
> > >
> > > Looks reasonable, the fact is that there are some corner issues
> > > compiled with Clang due to different compiler behaviors, most of
> > > the issues have been addressed and I need to do more test, I will
> > > send v3 with about 10 patches after the coming merge window.
> >
> > Hm, I didn't know -mannotate-tablejump existed.  We really need
> > something which supports all arches, not just loongarch.
> >
> > Others were looking at adding something similar (adding them to Cc).
>
> Looks like this was added to clang in:
> https://github.com/llvm/llvm-project/pull/102411
>
> A comment in llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
> describes the scheme:
> +  // Emit an additional section to store the correlation info as pairs of
> +  // addresses, each pair contains the address of a jump instruction (jr) and
> +  // the address of the jump table.
>
> Ard had a prototype in:
> https://github.com/llvm/llvm-project/pull/112606
> which used relocations rather than a discardable section.

Thanks for the cc.

I haven't followed up yet because doing this generically is not
straight-forward. The main issue is that AArch64 jump tables could be
emitted into .text with scaled offsets, e.g.,

adr  x16, .Ljumptable
ldrb w17, [x16, xN] // xN is the lookup index
add  x16, x16, w17, sxtw #2  // x16 += 4 * x17
br   x16

.Ljumptable:
.byte (dest0 - .Ljumptable) >> 2
.byte (dest1 - .Ljumptable) >> 2
.byte (dest2 - .Ljumptable) >> 2
.byte (dest3 - .Ljumptable) >> 2

So just emitting a relocation at the call site and a symbol covering
the jump table might work for x86, but if we want some that works in
general, we'll have to come up with some format that describes in more
detail how to infer the potential destinations of an indirect call it
is known to be a limited set at compile time.

Re: annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)

[Josh Poimboeuf]

On Thu, Nov 14, 2024 at 07:13:18PM +0100, Ard Biesheuvel wrote:
> > Looks like this was added to clang in:
> > https://github.com/llvm/llvm-project/pull/102411
> >
> > A comment in llvm/lib/Target/LoongArch/LoongArchAsmPrinter.cpp
> > describes the scheme:
> > +  // Emit an additional section to store the correlation info as pairs of
> > +  // addresses, each pair contains the address of a jump instruction (jr) and
> > +  // the address of the jump table.
> >
> > Ard had a prototype in:
> > https://github.com/llvm/llvm-project/pull/112606
> > which used relocations rather than a discardable section.
> 
> Thanks for the cc.
> 
> I haven't followed up yet because doing this generically is not
> straight-forward. The main issue is that AArch64 jump tables could be
> emitted into .text with scaled offsets, e.g.,
> 
> adr  x16, .Ljumptable
> ldrb w17, [x16, xN] // xN is the lookup index
> add  x16, x16, w17, sxtw #2  // x16 += 4 * x17
> br   x16
> 
> .Ljumptable:
> .byte (dest0 - .Ljumptable) >> 2
> .byte (dest1 - .Ljumptable) >> 2
> .byte (dest2 - .Ljumptable) >> 2
> .byte (dest3 - .Ljumptable) >> 2
> 
> So just emitting a relocation at the call site and a symbol covering
> the jump table might work for x86, but if we want some that works in
> general, we'll have to come up with some format that describes in more
> detail how to infer the potential destinations of an indirect call it
> is known to be a limited set at compile time.

Loongarch is emitting an array of (insn_ptr, jump_table_ptr) tuples
in .discard.tablejump_annotate.  Would that work more generically?

Even better it would also emit the jump table size.

-- 
Josh