[PATCH v3 05/10] objtool: Handle unreachable entry of rodata

Tiezhu Yang posted 10 patches 4 days, 8 hours ago
There is a newer version of this series
[PATCH v3 05/10] objtool: Handle unreachable entry of rodata
Posted by Tiezhu Yang 4 days, 8 hours ago
When compling with Clang on LoongArch, there exists unreachable entry
of rodata which points to a nop instruction after the function return
instruction, this is generated by compiler to fill the non-existent
switch case, just skip the entry 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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index eb4c89501493..17df3738e087 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2136,6 +2136,14 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (!dest_insn)
 			break;
 
+		/* Handle the special cases compiled with Clang on LoongArch */
+		if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
+		    reloc->sym->type == STT_SECTION && dest_insn->type == INSN_NOP &&
+		    (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)) {
+			prev_offset = reloc_offset(reloc);
+			continue;
+		}
+
 		/* Make sure the destination is in the same function: */
 		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
 			break;
-- 
2.42.0
Re: [PATCH v3 05/10] objtool: Handle unreachable entry of rodata
Posted by Jinyang He 4 days, 4 hours ago
On 2024-11-19 14:56, Tiezhu Yang wrote:

> When compling with Clang on LoongArch, there exists unreachable entry
> of rodata which points to a nop instruction after the function return
> instruction, this is generated by compiler to fill the non-existent
> switch case, just skip the entry 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 | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index eb4c89501493..17df3738e087 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2136,6 +2136,14 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
>   		if (!dest_insn)
>   			break;
>   
> +		/* Handle the special cases compiled with Clang on LoongArch */
> +		if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
> +		    reloc->sym->type == STT_SECTION && dest_insn->type == INSN_NOP &&
Why it points to NOP, and whether it could be some other instruction 
(e.g. break)?
It seems fragile. I think it should fix in Clang.
> +		    (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)) {
> +			prev_offset = reloc_offset(reloc);
> +			continue;
> +		}
> +
>   		/* Make sure the destination is in the same function: */
>   		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
>   			break;
Re: [PATCH v3 05/10] objtool: Handle unreachable entry of rodata
Posted by Huacai Chen 2 days, 2 hours ago
Hi, Jinyang,

On Tue, Nov 19, 2024 at 6:47 PM Jinyang He <hejinyang@loongson.cn> wrote:
>
> On 2024-11-19 14:56, Tiezhu Yang wrote:
>
> > When compling with Clang on LoongArch, there exists unreachable entry
> > of rodata which points to a nop instruction after the function return
> > instruction, this is generated by compiler to fill the non-existent
> > switch case, just skip the entry 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 | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> > index eb4c89501493..17df3738e087 100644
> > --- a/tools/objtool/check.c
> > +++ b/tools/objtool/check.c
> > @@ -2136,6 +2136,14 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
> >               if (!dest_insn)
> >                       break;
> >
> > +             /* Handle the special cases compiled with Clang on LoongArch */
> > +             if (file->elf->ehdr.e_machine == EM_LOONGARCH &&
> > +                 reloc->sym->type == STT_SECTION && dest_insn->type == INSN_NOP &&
> Why it points to NOP, and whether it could be some other instruction
> (e.g. break)?
> It seems fragile. I think it should fix in Clang.

As discussed with Tiezhu and the developer of Clang compiler (Wang
Lei), this is just the implementation of Clang, the non-existent
switch case is filled in the jump table, the table entry points to a
position after the function return instruction, this is not an issue
for Clang.

Currently, objdump the object files of Linux kernel shows that the
position is a nop instruction.

But it is not necessary to check the position is a nop instruction, it
only needs to check whether the position is after the function return
instruction, so "dest_insn->type == INSN_NOP" can be removed. I will
modify it when applying the patch.

Other patches seem to have no fundamental issues.

Huacai

> > +                 (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)) {
> > +                     prev_offset = reloc_offset(reloc);
> > +                     continue;
> > +             }
> > +
> >               /* Make sure the destination is in the same function: */
> >               if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
> >                       break;
>