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
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?
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
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
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
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
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
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.
© 2016 - 2024 Red Hat, Inc.