[PATCH v2 0/5] Add jump table support for objtool on LoongArch

[PATCH v2 0/5] Add jump table support for objtool on LoongArch

[Tiezhu Yang]

This series is based on 6.12-rc6, tested with the upstream mainline
binutils, GCC and Clang, all the changes are under tools/objtool and
arch/loongarch.

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, the next work is to add the corresponding
support with Clang after addressing some corner issues due to different
compiler behaviors.

Tiezhu Yang (5):
  objtool: Handle various symbol types for rodata
  objtool: Handle special cases of discard.reachable
  objtool/LoongArch: Add support for switch table
  objtool/LoongArch: Add support for goto table
  LoongArch: Enable jump table with GCC for objtool

 arch/loongarch/Kconfig                 |  3 +
 arch/loongarch/Makefile                |  9 +++
 tools/objtool/arch/loongarch/special.c | 83 +++++++++++++++++++++++++-
 tools/objtool/check.c                  | 38 ++++++++++--
 4 files changed, 128 insertions(+), 5 deletions(-)

-- 
2.42.0

[PATCH v2 1/5] objtool: Handle various symbol types for rodata

[Tiezhu Yang]

In the relocation section ".rela.rodata" of each .o file compiled with
LoongArch toolchain, there are various symbol types such as STT_NOTYPE,
STT_OBJECT, STT_FUNC in addition to the usual STT_SECTION, it needs to
use reloc symbol offset instead of reloc addend to find the destination
instruction in find_jump_table() and add_jump_table().

This is preparation for later patch on LoongArch, there is no effect for
the other archs with this patch.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 6604f5d038aa..9601235e908d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2079,6 +2079,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 	unsigned int prev_offset = 0;
 	struct reloc *reloc = table;
 	struct alternative *alt;
+	unsigned long offset;
 
 	/*
 	 * Each @reloc is a switch table relocation which points to the target
@@ -2094,12 +2095,19 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
 			break;
 
+		if (reloc->sym->type == STT_SECTION) {
+			/* Addend field in the relocation entry associated with the symbol */
+			offset = reloc_addend(reloc);
+		} else {
+			/* The address of the symbol in the relocation entry */
+			offset = reloc->sym->offset;
+		}
+
 		/* Detect function pointers from contiguous objects: */
-		if (reloc->sym->sec == pfunc->sec &&
-		    reloc_addend(reloc) == pfunc->offset)
+		if (reloc->sym->sec == pfunc->sec && offset == pfunc->offset)
 			break;
 
-		dest_insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+		dest_insn = find_insn(file, reloc->sym->sec, offset);
 		if (!dest_insn)
 			break;
 
@@ -2137,6 +2145,7 @@ static struct reloc *find_jump_table(struct objtool_file *file,
 {
 	struct reloc *table_reloc;
 	struct instruction *dest_insn, *orig_insn = insn;
+	unsigned long offset;
 
 	/*
 	 * Backward search using the @first_jump_src links, these help avoid
@@ -2160,7 +2169,16 @@ static struct reloc *find_jump_table(struct objtool_file *file,
 		table_reloc = arch_find_switch_table(file, insn);
 		if (!table_reloc)
 			continue;
-		dest_insn = find_insn(file, table_reloc->sym->sec, reloc_addend(table_reloc));
+
+		if (table_reloc->sym->type == STT_SECTION) {
+			/* Addend field in the relocation entry associated with the symbol */
+			offset = reloc_addend(table_reloc);
+		} else {
+			/* The address of the symbol in the relocation entry */
+			offset = table_reloc->sym->offset;
+		}
+
+		dest_insn = find_insn(file, table_reloc->sym->sec, offset);
 		if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
 			continue;
 
-- 
2.42.0

[PATCH v2 2/5] objtool: Handle special cases of discard.reachable

[Tiezhu Yang]

There are some "unreachable instruction" objtool warnings when compling
with Clang on LoongArch, this is because the "break" instruction is set
as dead end due to its type is INSN_BUG in decode_instructions() at the
beginning, and it does not set insn->dead_end of the "break" instruction
as false after checking ".rela.discard.reachable" in add_dead_ends(), so
the next instruction of "break" is marked as unreachable.

Actually, it can find the reachable instruction after parsing the section
".rela.discard.reachable", in some cases, the "break" instruction may not
be the first previous instruction with scheduling by Machine Instruction
Scheduler of LLVM, it should find more times and then set insn->dead_end
of the "break" instruction as false.

This is preparation for later patch on LoongArch, there is no effect for
the other archs with this patch.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 9601235e908d..6607cd56459b 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -711,6 +711,18 @@ static int add_dead_ends(struct objtool_file *file)
 		}
 
 		insn->dead_end = false;
+
+		/* Handle the special cases compiled with Clang on LoongArch */
+		if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
+		    reloc->sym->type == STT_SECTION) {
+			while (insn && insn_func(insn)) {
+				insn = prev_insn_same_sym(file, insn);
+				if (insn && insn->dead_end) {
+					insn->dead_end = false;
+					break;
+				}
+			}
+		}
 	}
 
 	return 0;
-- 
2.42.0

[PATCH v2 3/5] objtool/LoongArch: Add support for switch table

[Tiezhu Yang]

The objtool program needs to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that a
branch instruction may jump into, if a jump table is used, objtool has to
correlate the jump instruction with the table.

On x86 which is the only port supported by objtool before LoongArch, there
is a relocation on the jump instruction and to the table directly. But on
LoongArch, the relocation is on some kind of instruction prior to the jump
instruction, and also with scheduling it is not easy to tell the offset of
that instruction from the jump instruction. Furthermore, because LoongArch
has -fsection-anchors (often enabled at -O1 or above) the relocation may
actually points to a section anchor instead of the table itself.

There is a GCC patch "LoongArch: Add support to annotate tablejump" which
has been merged into the upstream mainline, the changes are very trivial
in GCC, it makes life much easier for switch table support of objtool on
LoongArch.

By now, there is an additional section ".discard.tablejump_annotate" to
store the jump info as pairs of addresses, each pair contains the address
of jump instruction and the address of jump table.

In order to find switch table, it is easy to parse the relocation section
".rela.discard.tablejump_annotate" to get table_sec and table_offset, the
rest process is somehow like x86.

Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=0ee028f55640
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/special.c | 59 +++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 9bba1e9318e0..31e0f94e31e8 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+#include <string.h>
 #include <objtool/special.h>
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
@@ -8,8 +9,64 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
 	return false;
 }
 
+static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
+						  struct instruction *insn)
+{
+	struct section *rsec;
+	struct reloc *reloc;
+	unsigned long offset;
+
+	rsec = find_section_by_name(file->elf, ".rela.discard.tablejump_annotate");
+	if (!rsec)
+		return NULL;
+
+	for_each_reloc(rsec, reloc) {
+		if (reloc->sym->sec->rodata)
+			continue;
+
+		if (strcmp(insn->sec->name, reloc->sym->sec->name))
+			continue;
+
+		if (reloc->sym->type == STT_SECTION)
+			offset = reloc_addend(reloc);
+		else
+			offset = reloc->sym->offset;
+
+		if (insn->offset == offset) {
+			reloc++;
+			return reloc;
+		}
+	}
+
+	return NULL;
+}
+
 struct reloc *arch_find_switch_table(struct objtool_file *file,
 				     struct instruction *insn)
 {
-	return NULL;
+	struct reloc *annotate_reloc;
+	struct reloc *rodata_reloc;
+	struct section *table_sec;
+	unsigned long table_offset;
+
+	annotate_reloc = find_reloc_by_table_annotate(file, insn);
+	if (!annotate_reloc)
+		return NULL;
+
+	table_sec = annotate_reloc->sym->sec;
+	if (annotate_reloc->sym->type == STT_SECTION)
+		table_offset = reloc_addend(annotate_reloc);
+	else
+		table_offset = annotate_reloc->sym->offset;
+
+	/*
+	 * Each table entry has a rela associated with it.  The rela
+	 * should reference text in the same function as the original
+	 * instruction.
+	 */
+	rodata_reloc = find_reloc_by_dest(file->elf, table_sec, table_offset);
+	if (!rodata_reloc)
+		return NULL;
+
+	return rodata_reloc;
 }
-- 
2.42.0

[PATCH v2 4/5] objtool/LoongArch: Add support for goto table

[Tiezhu Yang]

The objtool program needs to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that a
branch instruction may jump into, if a jump table is used, objtool has to
correlate the jump instruction with the table.

On x86 which is the only port supported by objtool before LoongArch, there
is a relocation on the jump instruction and to the table directly. But on
LoongArch, the relocation is on some kind of instruction prior to the jump
instruction, and also with scheduling it is not easy to tell the offset of
that instruction from the jump instruction. Furthermore, because LoongArch
has -fsection-anchors (often enabled at -O1 or above) the relocation may
actually points to a section anchor instead of the table itself.

For the jump table of switch cases, a GCC patch "LoongArch: Add support to
annotate tablejump" has been merged into the upstream mainline, it makes
life much easier with the additional section ".discard.tablejump_annotate"
which stores the jump info as pairs of addresses, each pair contains the
address of jump instruction and the address of jump table.

For the jump table of computed gotos, it is indeed not easy to implement
in the compiler, especially if there is more than one computed goto in a
function such as ___bpf_prog_run(). objdump kernel/bpf/core.o shows that
there are many table jump instructions in ___bpf_prog_run(), but there are
no relocations on the table jump instructions and to the table directly on
LoongArch.

Without the help of compiler, in order to figure out the address of goto
table for the special case of ___bpf_prog_run(), since the instruction
sequence is relatively single and stable, it makes sense to add a helper
find_reloc_of_rodata_c_jump_table() to find the relocation which points
to the section ".rodata..c_jump_table".

If find_reloc_by_table_annotate() failed, it means there is no relocation
info of switch table address in ".rela.discard.tablejump_annotate", then
objtool may find the relocation info of goto table ".rodata..c_jump_table"
with find_reloc_of_rodata_c_jump_table().

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/special.c | 28 ++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 31e0f94e31e8..fe7413bcd98b 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -41,6 +41,27 @@ static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
 	return NULL;
 }
 
+static struct reloc *find_reloc_of_rodata_c_jump_table(struct section *sec,
+							unsigned long offset)
+{
+	struct section *rsec;
+	struct reloc *reloc;
+
+	rsec = sec->rsec;
+	if (!rsec)
+		return NULL;
+
+	for_each_reloc(rsec, reloc) {
+		if (reloc_offset(reloc) > offset)
+			break;
+
+		if (!strncmp(reloc->sym->sec->name, ".rodata..c_jump_table", 21))
+			return reloc;
+	}
+
+	return NULL;
+}
+
 struct reloc *arch_find_switch_table(struct objtool_file *file,
 				     struct instruction *insn)
 {
@@ -50,8 +71,11 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	unsigned long table_offset;
 
 	annotate_reloc = find_reloc_by_table_annotate(file, insn);
-	if (!annotate_reloc)
-		return NULL;
+	if (!annotate_reloc) {
+		annotate_reloc = find_reloc_of_rodata_c_jump_table(insn->sec, insn->offset);
+		if (!annotate_reloc)
+			return NULL;
+	}
 
 	table_sec = annotate_reloc->sym->sec;
 	if (annotate_reloc->sym->type == STT_SECTION)
-- 
2.42.0

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

[Tiezhu Yang]

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 0/5] Add jump table support for objtool on LoongArch

[Huacai Chen]

Hi, Jinyang,

Could you please take some time to review this series?

Huacai

On Tue, Nov 5, 2024 at 8:39 PM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> This series is based on 6.12-rc6, tested with the upstream mainline
> binutils, GCC and Clang, all the changes are under tools/objtool and
> arch/loongarch.
>
> 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, the next work is to add the corresponding
> support with Clang after addressing some corner issues due to different
> compiler behaviors.
>
> Tiezhu Yang (5):
>   objtool: Handle various symbol types for rodata
>   objtool: Handle special cases of discard.reachable
>   objtool/LoongArch: Add support for switch table
>   objtool/LoongArch: Add support for goto table
>   LoongArch: Enable jump table with GCC for objtool
>
>  arch/loongarch/Kconfig                 |  3 +
>  arch/loongarch/Makefile                |  9 +++
>  tools/objtool/arch/loongarch/special.c | 83 +++++++++++++++++++++++++-
>  tools/objtool/check.c                  | 38 ++++++++++--
>  4 files changed, 128 insertions(+), 5 deletions(-)
>
> --
> 2.42.0
>

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

[Peter Zijlstra]

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

[Tiezhu Yang]

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

[Xi Ruoyao]

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

[Tiezhu Yang]

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)

[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