[PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool

Tiezhu Yang posted 5 patches 2 weeks, 4 days ago
There is a newer version of this series
[PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool
Posted by Tiezhu Yang 2 weeks, 4 days ago
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.

By the way, the compiler behaviors are different for various archs,
there are some corner issues after removing -fno-jump-tables if the
compiler is Clang, so just keep the compiler option -fno-jump-tables
for Clang at present.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig  | 3 +++
 arch/loongarch/Makefile | 9 +++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index bb35c34f86d2..500ee9b2cd88 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -284,6 +284,9 @@ config AS_HAS_LBT_EXTENSION
 config AS_HAS_LVZ_EXTENSION
 	def_bool $(as-instr,hvcl 0)
 
+config CC_HAS_ANNOTATE_TABLEJUMP
+	def_bool $(cc-option,-mannotate-tablejump)
+
 menu "Kernel type and options"
 
 source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index ae3f80622f4c..61484df4eccc 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -101,8 +101,17 @@ KBUILD_AFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
 KBUILD_CFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
 
 ifdef CONFIG_OBJTOOL
+ifdef CONFIG_CC_IS_GCC
+ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
+KBUILD_CFLAGS			+= $(call cc-option,-mannotate-tablejump)
+else
 KBUILD_CFLAGS			+= -fno-jump-tables
 endif
+endif
+ifdef CONFIG_CC_IS_CLANG
+KBUILD_CFLAGS			+= -fno-jump-tables
+endif
+endif
 
 KBUILD_RUSTFLAGS		+= --target=loongarch64-unknown-none-softfloat
 KBUILD_RUSTFLAGS_KERNEL		+= -Zdirect-access-external-data=yes
-- 
2.42.0
Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool
Posted by Peter Zijlstra 2 weeks, 4 days ago
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.
> 
> By the way, the compiler behaviors are different for various archs,
> there are some corner issues after removing -fno-jump-tables if the
> compiler is Clang, so just keep the compiler option -fno-jump-tables
> for Clang at present.
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  arch/loongarch/Kconfig  | 3 +++
>  arch/loongarch/Makefile | 9 +++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> index bb35c34f86d2..500ee9b2cd88 100644
> --- a/arch/loongarch/Kconfig
> +++ b/arch/loongarch/Kconfig
> @@ -284,6 +284,9 @@ config AS_HAS_LBT_EXTENSION
>  config AS_HAS_LVZ_EXTENSION
>  	def_bool $(as-instr,hvcl 0)
>  
> +config CC_HAS_ANNOTATE_TABLEJUMP
> +	def_bool $(cc-option,-mannotate-tablejump)
> +
>  menu "Kernel type and options"
>  
>  source "kernel/Kconfig.hz"
> diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
> index ae3f80622f4c..61484df4eccc 100644
> --- a/arch/loongarch/Makefile
> +++ b/arch/loongarch/Makefile
> @@ -101,8 +101,17 @@ KBUILD_AFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
>  KBUILD_CFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
>  
>  ifdef CONFIG_OBJTOOL
> +ifdef CONFIG_CC_IS_GCC
> +ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
> +KBUILD_CFLAGS			+= $(call cc-option,-mannotate-tablejump)
> +else
>  KBUILD_CFLAGS			+= -fno-jump-tables
>  endif
> +endif
> +ifdef CONFIG_CC_IS_CLANG
> +KBUILD_CFLAGS			+= -fno-jump-tables
> +endif
> +endif

This seems excessive. Why split between GCC and Clang, isn't
CC_HAS_ANNOTATE_JUMPTABLE sufficient?
Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool
Posted by Tiezhu Yang 2 weeks, 4 days ago
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_OBJTOOL
>> +ifdef CONFIG_CC_IS_GCC
>> +ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
>> +KBUILD_CFLAGS			+= $(call cc-option,-mannotate-tablejump)
>> +else
>>  KBUILD_CFLAGS			+= -fno-jump-tables
>>  endif
>> +endif
>> +ifdef CONFIG_CC_IS_CLANG
>> +KBUILD_CFLAGS			+= -fno-jump-tables
>> +endif
>> +endif
>
> This seems excessive. Why split between GCC and Clang, isn't
> CC_HAS_ANNOTATE_JUMPTABLE sufficient?

Thanks for your reply.

In theory, it is sufficient to only check CC_HAS_ANNOTATE_JUMPTABLE
to use -fno-jump-tables or not, and also this is my initial aim.

In fact, when compling with Clang on LoongArch, if the compiler has
the option -mannotate-tablejump and config CC_HAS_ANNOTATE_TABLEJUMP
is set, there still exists some objtool warnings if remove the option
-fno-jump-tables, this is because there are some special cases such
as different rodata relocation type and rodata entry size generated
by Clang, I am working in progress to address the corner issues, and
the final code looks something like this:

ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
KBUILD_CFLAGS                   += $(call cc-option,-mannotate-tablejump)
else
KBUILD_CFLAGS                   += -fno-jump-tables
endif

Thanks,
Tiezhu
Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool
Posted by Xi Ruoyao 1 week, 5 days ago
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_OBJTOOL
> > > +ifdef CONFIG_CC_IS_GCC
> > > +ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
> > > +KBUILD_CFLAGS			+= $(call cc-option,-mannotate-tablejump)
> > > +else
> > >  KBUILD_CFLAGS			+= -fno-jump-tables
> > >  endif
> > > +endif
> > > +ifdef CONFIG_CC_IS_CLANG
> > > +KBUILD_CFLAGS			+= -fno-jump-tables
> > > +endif
> > > +endif
> > 
> > This seems excessive. Why split between GCC and Clang, isn't
> > CC_HAS_ANNOTATE_JUMPTABLE sufficient?
> 
> Thanks for your reply.
> 
> In theory, it is sufficient to only check CC_HAS_ANNOTATE_JUMPTABLE
> to use -fno-jump-tables or not, and also this is my initial aim.
> 
> In fact, when compling with Clang on LoongArch, if the compiler has
> the option -mannotate-tablejump and config CC_HAS_ANNOTATE_TABLEJUMP
> is set, there still exists some objtool warnings if remove the option
> -fno-jump-tables, this is because there are some special cases such
> as different rodata relocation type and rodata entry size generated
> by Clang, I am working in progress to address the corner issues, and
> the final code looks something like this:
> 
> 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?  IMO it's better to add it
to Clang first, and add Clang & GCC support at once into objtool.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool
Posted by Tiezhu Yang 1 week, 4 days ago
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.

Thanks,
Tiezhu
annotating jump tables (Re: [PATCH v2 5/5] LoongArch: Enable jump table with GCC for objtool)
Posted by Josh Poimboeuf 1 week, 3 days ago
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)
Posted by Nick Desaulniers 1 week, 2 days ago
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)
Posted by Ard Biesheuvel 1 week, 2 days ago
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.