When compling with Clang on LoongArch, there are unsorted table offset
of rodata if there exist many jump tables, it will get the wrong table
end and find the wrong jump destination instructions in add_jump_table(),
so it needs to check the table size of rodata to break the process when
parsing the relocation section of rodata.
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 | 7 +++++++
tools/objtool/include/objtool/check.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index b21e47d8d3d1..3b2d94c90011 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2101,6 +2101,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
struct alternative *alt;
unsigned long offset;
unsigned long rodata_entry_size;
+ unsigned long rodata_table_size = insn->table_size;
/*
* Each @reloc is a switch table relocation which points to the target
@@ -2112,6 +2113,12 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
if (reloc != table && reloc == next_table)
break;
+ /* Handle the special cases compiled with Clang on LoongArch */
+ if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
+ reloc->sym->type == STT_SECTION && reloc != table &&
+ reloc_offset(reloc) == reloc_offset(table) + rodata_table_size)
+ break;
+
/* Handle the special cases compiled with Clang on LoongArch */
if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
reloc_type(reloc) == R_LARCH_32_PCREL)
diff --git a/tools/objtool/include/objtool/check.h b/tools/objtool/include/objtool/check.h
index daa46f1f0965..11aafcc7b0c7 100644
--- a/tools/objtool/include/objtool/check.h
+++ b/tools/objtool/include/objtool/check.h
@@ -46,6 +46,7 @@ struct instruction {
struct section *sec;
unsigned long offset;
unsigned long immediate;
+ unsigned long table_size;
u8 len;
u8 prev_len;
--
2.42.0On 11/22/2024 12:50 PM, Tiezhu Yang wrote:
> When compling with Clang on LoongArch, there are unsorted table offset
> of rodata if there exist many jump tables, it will get the wrong table
> end and find the wrong jump destination instructions in add_jump_table(),
> so it needs to check the table size of rodata to break the process when
> parsing the relocation section of rodata.
>
> 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 | 7 +++++++
> tools/objtool/include/objtool/check.h | 1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index b21e47d8d3d1..3b2d94c90011 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2101,6 +2101,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> struct alternative *alt;
> unsigned long offset;
> unsigned long rodata_entry_size;
> + unsigned long rodata_table_size = insn->table_size;
>
> /*
> * Each @reloc is a switch table relocation which points to the target
> @@ -2112,6 +2113,12 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> if (reloc != table && reloc == next_table)
> break;
>
> + /* Handle the special cases compiled with Clang on LoongArch */
> + if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
> + reloc->sym->type == STT_SECTION && reloc != table &&
> + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size)
> + break;
I think it can be generic, like this:
/* Check for the end of the table: */
if (reloc != table && reloc == next_table)
break;
if (reloc != table &&
reloc_offset(reloc) == reloc_offset(table) +
rodata_table_size)
break;
What do you think?
Thanks,
Tiezhu
On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: > > + /* Handle the special cases compiled with Clang on LoongArch */ > > + if (file->elf->ehdr.e_machine == EM_LOONGARCH && > > + reloc->sym->type == STT_SECTION && reloc != table && > > + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) > > + break; > > I think it can be generic, like this: > > /* Check for the end of the table: */ > if (reloc != table && reloc == next_table) > break; > > if (reloc != table && > reloc_offset(reloc) == reloc_offset(table) + > rodata_table_size) > break; > > What do you think? I'm not sure, this patch is hard to review because it uses insn->table_size which doesn't get set until the next patch. Maybe this patch should come after patches 7 & 8, or maybe they should be squashed? -- Josh
On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: > On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: >>> + /* Handle the special cases compiled with Clang on LoongArch */ >>> + if (file->elf->ehdr.e_machine == EM_LOONGARCH && >>> + reloc->sym->type == STT_SECTION && reloc != table && >>> + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) >>> + break; >> >> I think it can be generic, like this: >> >> /* Check for the end of the table: */ >> if (reloc != table && reloc == next_table) >> break; >> >> if (reloc != table && >> reloc_offset(reloc) == reloc_offset(table) + >> rodata_table_size) >> break; >> >> What do you think? > > I'm not sure, this patch is hard to review because it uses > insn->table_size which doesn't get set until the next patch. > > Maybe this patch should come after patches 7 & 8, or maybe they should > be squashed? OK, I will squash the changes into patch #7.
On Wed, Nov 27, 2024 at 03:01:33PM +0800, Tiezhu Yang wrote: > > > On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: > > On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: > > > > + /* Handle the special cases compiled with Clang on LoongArch */ > > > > + if (file->elf->ehdr.e_machine == EM_LOONGARCH && > > > > + reloc->sym->type == STT_SECTION && reloc != table && > > > > + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) > > > > + break; > > > > > > I think it can be generic, like this: > > > > > > /* Check for the end of the table: */ > > > if (reloc != table && reloc == next_table) > > > break; > > > > > > if (reloc != table && > > > reloc_offset(reloc) == reloc_offset(table) + > > > rodata_table_size) > > > break; > > > > > > What do you think? > > > > I'm not sure, this patch is hard to review because it uses > > insn->table_size which doesn't get set until the next patch. > > > > Maybe this patch should come after patches 7 & 8, or maybe they should > > be squashed? > > OK, I will squash the changes into patch #7. I remembered Ard already solved a similar problem when he prototyped x86 jump table annotation. Can you pull this patch into your series: https://lore.kernel.org/20241011170847.334429-12-ardb+git@google.com -- Josh
On Wed, Nov 27, 2024 at 04:10:18PM -0800, Josh Poimboeuf wrote: > On Wed, Nov 27, 2024 at 03:01:33PM +0800, Tiezhu Yang wrote: > > > > > > On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: > > > On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: > > > > > + /* Handle the special cases compiled with Clang on LoongArch */ > > > > > + if (file->elf->ehdr.e_machine == EM_LOONGARCH && > > > > > + reloc->sym->type == STT_SECTION && reloc != table && > > > > > + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) > > > > > + break; > > > > > > > > I think it can be generic, like this: > > > > > > > > /* Check for the end of the table: */ > > > > if (reloc != table && reloc == next_table) > > > > break; > > > > > > > > if (reloc != table && > > > > reloc_offset(reloc) == reloc_offset(table) + > > > > rodata_table_size) > > > > break; > > > > > > > > What do you think? > > > > > > I'm not sure, this patch is hard to review because it uses > > > insn->table_size which doesn't get set until the next patch. > > > > > > Maybe this patch should come after patches 7 & 8, or maybe they should > > > be squashed? > > > > OK, I will squash the changes into patch #7. > > I remembered Ard already solved a similar problem when he prototyped x86 > jump table annotation. Can you pull this patch into your series: > > https://lore.kernel.org/20241011170847.334429-12-ardb+git@google.com Actually, I think I'm going to merge patches 2-5 from Ard's series as they're a nice cleanup. Let me do that and then you can base your next version off tip/objtool/core once it gets updated with Ard's and Peter's patches. -- Josh
On Wed, Nov 27, 2024 at 04:16:29PM -0800, Josh Poimboeuf wrote: > On Wed, Nov 27, 2024 at 04:10:18PM -0800, Josh Poimboeuf wrote: > > On Wed, Nov 27, 2024 at 03:01:33PM +0800, Tiezhu Yang wrote: > > > > > > > > > On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: > > > > On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: > > > > > > + /* Handle the special cases compiled with Clang on LoongArch */ > > > > > > + if (file->elf->ehdr.e_machine == EM_LOONGARCH && > > > > > > + reloc->sym->type == STT_SECTION && reloc != table && > > > > > > + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) > > > > > > + break; > > > > > > > > > > I think it can be generic, like this: > > > > > > > > > > /* Check for the end of the table: */ > > > > > if (reloc != table && reloc == next_table) > > > > > break; > > > > > > > > > > if (reloc != table && > > > > > reloc_offset(reloc) == reloc_offset(table) + > > > > > rodata_table_size) > > > > > break; > > > > > > > > > > What do you think? > > > > > > > > I'm not sure, this patch is hard to review because it uses > > > > insn->table_size which doesn't get set until the next patch. > > > > > > > > Maybe this patch should come after patches 7 & 8, or maybe they should > > > > be squashed? > > > > > > OK, I will squash the changes into patch #7. > > > > I remembered Ard already solved a similar problem when he prototyped x86 > > jump table annotation. Can you pull this patch into your series: > > > > https://lore.kernel.org/20241011170847.334429-12-ardb+git@google.com > > Actually, I think I'm going to merge patches 2-5 from Ard's series as > they're a nice cleanup. Let me do that and then you can base your next > version off tip/objtool/core once it gets updated with Ard's and Peter's > patches. Still talking to myself here, I think we'll only merge the above patch, since we don't know what the generic annotations are going to look like yet. -- Josh
On 11/28/2024 09:00 AM, Josh Poimboeuf wrote: > On Wed, Nov 27, 2024 at 04:16:29PM -0800, Josh Poimboeuf wrote: >> On Wed, Nov 27, 2024 at 04:10:18PM -0800, Josh Poimboeuf wrote: >>> On Wed, Nov 27, 2024 at 03:01:33PM +0800, Tiezhu Yang wrote: >>>> >>>> >>>> On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: >>>>> On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: >>>>>>> + /* Handle the special cases compiled with Clang on LoongArch */ >>>>>>> + if (file->elf->ehdr.e_machine == EM_LOONGARCH && >>>>>>> + reloc->sym->type == STT_SECTION && reloc != table && >>>>>>> + reloc_offset(reloc) == reloc_offset(table) + rodata_table_size) >>>>>>> + break; >>>>>> >>>>>> I think it can be generic, like this: >>>>>> >>>>>> /* Check for the end of the table: */ >>>>>> if (reloc != table && reloc == next_table) >>>>>> break; >>>>>> >>>>>> if (reloc != table && >>>>>> reloc_offset(reloc) == reloc_offset(table) + >>>>>> rodata_table_size) >>>>>> break; >>>>>> >>>>>> What do you think? >>>>> >>>>> I'm not sure, this patch is hard to review because it uses >>>>> insn->table_size which doesn't get set until the next patch. >>>>> >>>>> Maybe this patch should come after patches 7 & 8, or maybe they should >>>>> be squashed? >>>> >>>> OK, I will squash the changes into patch #7. >>> >>> I remembered Ard already solved a similar problem when he prototyped x86 >>> jump table annotation. Can you pull this patch into your series: >>> >>> https://lore.kernel.org/20241011170847.334429-12-ardb+git@google.com >> >> Actually, I think I'm going to merge patches 2-5 from Ard's series as >> they're a nice cleanup. Let me do that and then you can base your next >> version off tip/objtool/core once it gets updated with Ard's and Peter's >> patches. > > Still talking to myself here, I think we'll only merge the above patch, > since we don't know what the generic annotations are going to look like > yet. OK, my next version will be based on tip/objtool/core after the merge window, by that time, hope the tree include Ard's and Peter's patches to avoid conflicts. Thanks, Tiezhu
On Thu, Nov 28, 2024 at 10:28:01AM +0800, Tiezhu Yang wrote: > On 11/28/2024 09:00 AM, Josh Poimboeuf wrote: > > Still talking to myself here, I think we'll only merge the above patch, > > since we don't know what the generic annotations are going to look like > > yet. > > OK, my next version will be based on tip/objtool/core after > the merge window, by that time, hope the tree include Ard's > and Peter's patches to avoid conflicts. Should now all be pushed out and visible in tip/objtool/core. Thanks!
Hi Josh, On 11/28/2024 10:28 AM, Tiezhu Yang wrote: > On 11/28/2024 09:00 AM, Josh Poimboeuf wrote: >> On Wed, Nov 27, 2024 at 04:16:29PM -0800, Josh Poimboeuf wrote: >>> On Wed, Nov 27, 2024 at 04:10:18PM -0800, Josh Poimboeuf wrote: >>>> On Wed, Nov 27, 2024 at 03:01:33PM +0800, Tiezhu Yang wrote: >>>>> >>>>> >>>>> On 11/27/2024 09:20 AM, Josh Poimboeuf wrote: >>>>>> On Tue, Nov 26, 2024 at 09:28:19PM +0800, Tiezhu Yang wrote: ... >> Still talking to myself here, I think we'll only merge the above patch, >> since we don't know what the generic annotations are going to look like >> yet. > > OK, my next version will be based on tip/objtool/core after > the merge window, by that time, hope the tree include Ard's > and Peter's patches to avoid conflicts. I need to rebase my patches based on tip/tip.git objtool/core branch. Could you please update tip/tip.git objtool/core branch to 6.13-rc1, and then merge jpoimboe/linux.git objtool/core & objtool-loongarch branches into tip/tip.git objtool/core branch? Thanks, Tiezhu
© 2016 - 2026 Red Hat, Inc.