In the most cases, the entry size of rodata is 8 bytes because the
relocation type is 64 bit, but when compiling with Clang on LoongArch,
there exists 32 bit relocation type, the entry size of rodata should
be 4 bytes in this case.

Add an arch-specific function arch_reloc_size() to assign the entry
size of rodata, the default value is 8 in the weak definition, it can
be overridden by archs that have different requirements.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/decode.c | 11 +++++++++++
 tools/objtool/check.c                 | 10 +++++++++-
 tools/objtool/include/objtool/arch.h  |  2 ++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index 69b66994f2a1..b64205b89f6b 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -363,3 +363,14 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
 	state->cfa.base = CFI_SP;
 	state->cfa.offset = 0;
 }
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+	switch (reloc_type(reloc)) {
+	case R_LARCH_32:
+	case R_LARCH_32_PCREL:
+		return 4;
+	default:
+		return 8;
+	}
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f64435ad3514..7271e959520e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1943,6 +1943,11 @@ static int add_special_section_alts(struct objtool_file *file)
 	return ret;
 }
 
+__weak unsigned int arch_reloc_size(struct reloc *reloc)
+{
+	return 8;
+}
+
 static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 			  struct reloc *next_table)
 {
@@ -1954,6 +1959,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 	struct reloc *reloc = table;
 	struct alternative *alt;
 	unsigned long sym_offset;
+	unsigned int entry_size;
 
 	/*
 	 * Each @reloc is a switch table relocation which points to the target
@@ -1967,8 +1973,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (reloc != table && reloc == next_table)
 			break;
 
+		entry_size = arch_reloc_size(reloc);
+
 		/* Make sure the table entries are consecutive: */
-		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
+		if (prev_offset && reloc_offset(reloc) != prev_offset + entry_size)
 			break;
 
 		sym_offset = reloc->sym->offset + reloc_addend(reloc);
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index d63b46a19f39..396f7c6c81c0 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -97,4 +97,6 @@ int arch_rewrite_retpolines(struct objtool_file *file);
 
 bool arch_pc_relative_reloc(struct reloc *reloc);
 
+unsigned int arch_reloc_size(struct reloc *reloc);
+
 #endif /* _ARCH_H */
-- 
2.42.0

On Sat, Dec 07, 2024 at 09:59:07AM +0800, Tiezhu Yang wrote:
> +__weak unsigned int arch_reloc_size(struct reloc *reloc)
> +{
> +	return 8;
> +}

Instead of making a weak function, each arch should have an explicit
definition of this function, as it's not always 8:

- x86 has R_X86_64_PC32 and R_X86_64_PLT32 which are 4 bytes.

- 32-bit powerpc

- 64-bit powerpc has R_PPC64_REL32

> @@ -1967,8 +1973,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
>  		if (reloc != table && reloc == next_table)
>  			break;
>  
> +		entry_size = arch_reloc_size(reloc);
> +
>  		/* Make sure the table entries are consecutive: */
> -		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
> +		if (prev_offset && reloc_offset(reloc) != prev_offset + entry_size)
>  			break;

No need to use a variable here, just call arch_reloc_size() directly.

-- 
Josh

On 12/10/2024 03:54 AM, Josh Poimboeuf wrote:
> On Sat, Dec 07, 2024 at 09:59:07AM +0800, Tiezhu Yang wrote:
>> +__weak unsigned int arch_reloc_size(struct reloc *reloc)
>> +{
>> +	return 8;
>> +}
>
> Instead of making a weak function, each arch should have an explicit
> definition of this function, as it's not always 8:
>
> - x86 has R_X86_64_PC32 and R_X86_64_PLT32 which are 4 bytes.
>
> - 32-bit powerpc
>
> - 64-bit powerpc has R_PPC64_REL32

For x86, like this, I am not sure "case R_X86_64_32" is necessary,
leave it as is if no objections, please let me know if it should be
removed.

unsigned int arch_reloc_size(struct reloc *reloc)
{
         switch (reloc_type(reloc)) {
         case R_X86_64_32:
         case R_X86_64_PC32:
         case R_X86_64_PLT32:
                 return 4;
         default:
                 return 8;
         }
}

For ppc, like this:

unsigned int arch_reloc_size(struct reloc *reloc)
{
         switch (reloc_type(reloc)) {
         case R_PPC_REL32:
         case R_PPC64_REL32:
                 return 4;
         default:
                 return 8;
         }
}

>
>> @@ -1967,8 +1973,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
>>  		if (reloc != table && reloc == next_table)
>>  			break;
>>
>> +		entry_size = arch_reloc_size(reloc);
>> +
>>  		/* Make sure the table entries are consecutive: */
>> -		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
>> +		if (prev_offset && reloc_offset(reloc) != prev_offset + entry_size)
>>  			break;
>
> No need to use a variable here, just call arch_reloc_size() directly.

OK, will do it.

Thanks,
Tiezhu

On Wed, Dec 11, 2024 at 11:11:28AM +0800, Tiezhu Yang wrote:
> unsigned int arch_reloc_size(struct reloc *reloc)
> {
>         switch (reloc_type(reloc)) {
>         case R_X86_64_32:
>         case R_X86_64_PC32:
>         case R_X86_64_PLT32:
>                 return 4;
>         default:
>                 return 8;
>         }

Looks good to me, you can add R_X86_64_32S as well.

> For ppc, like this:
> 
> unsigned int arch_reloc_size(struct reloc *reloc)
> {
>         switch (reloc_type(reloc)) {
>         case R_PPC_REL32:
>         case R_PPC64_REL32:
>                 return 4;
>         default:
>                 return 8;
>         }

Also:

R_PPC_ADDR32
R_PPC_UADDR32	
R_PPC_PLT32
R_PPC_PLTREL32

And the ppc64 counterparts have the same values, so not necessary to add
them.

-- 
Josh