When compiling on LoongArch, there exists PC relative relocation type,
it needs to get the symbol offset with "S + A - PC" according to the
spec of "ELF for the LoongArch Architecture".

Add an arch-specific function arch_adjust_offset() to assign the symbol
offset, the default value is "reloc->sym->offset + reloc_addend(reloc)"
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         | 20 +++++++++++++++----
 .../objtool/arch/loongarch/include/arch/elf.h |  7 +++++++
 tools/objtool/check.c                         |  7 ++++++-
 tools/objtool/include/objtool/arch.h          |  1 +
 4 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index b64205b89f6b..431eb4757458 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -5,10 +5,7 @@
 #include <asm/inst.h>
 #include <asm/orc_types.h>
 #include <linux/objtool_types.h>
-
-#ifndef EM_LOONGARCH
-#define EM_LOONGARCH	258
-#endif
+#include <arch/elf.h>
 
 int arch_ftrace_match(char *name)
 {
@@ -374,3 +371,18 @@ unsigned int arch_reloc_size(struct reloc *reloc)
 		return 8;
 	}
 }
+
+unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table)
+{
+	switch (reloc_type(reloc)) {
+	case R_LARCH_32_PCREL:
+	case R_LARCH_64_PCREL:
+		if (reloc->sym->type == STT_SECTION)
+			return reloc->sym->offset + reloc_addend(reloc) -
+			       (reloc_offset(reloc) - reloc_offset(table));
+		else
+			return reloc->sym->offset;
+	default:
+		return reloc->sym->offset + reloc_addend(reloc);
+	}
+}
diff --git a/tools/objtool/arch/loongarch/include/arch/elf.h b/tools/objtool/arch/loongarch/include/arch/elf.h
index 9623d663220e..ec79062c9554 100644
--- a/tools/objtool/arch/loongarch/include/arch/elf.h
+++ b/tools/objtool/arch/loongarch/include/arch/elf.h
@@ -18,6 +18,13 @@
 #ifndef R_LARCH_32_PCREL
 #define R_LARCH_32_PCREL	99
 #endif
+#ifndef R_LARCH_64_PCREL
+#define R_LARCH_64_PCREL	109
+#endif
+
+#ifndef EM_LOONGARCH
+#define EM_LOONGARCH		258
+#endif
 
 #define R_NONE			R_LARCH_NONE
 #define R_ABS32			R_LARCH_32
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 7271e959520e..4a23aae40c03 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1948,6 +1948,11 @@ __weak unsigned int arch_reloc_size(struct reloc *reloc)
 	return 8;
 }
 
+__weak unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table)
+{
+	return reloc->sym->offset + reloc_addend(reloc);
+}
+
 static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 			  struct reloc *next_table)
 {
@@ -1979,7 +1984,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (prev_offset && reloc_offset(reloc) != prev_offset + entry_size)
 			break;
 
-		sym_offset = reloc->sym->offset + reloc_addend(reloc);
+		sym_offset = arch_adjust_offset(reloc, table);
 
 		/* Detect function pointers from contiguous objects: */
 		if (reloc->sym->sec == pfunc->sec && sym_offset == pfunc->offset)
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 396f7c6c81c0..34f41ba6d910 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -98,5 +98,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);
+unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table);
 
 #endif /* _ARCH_H */
-- 
2.42.0

On Sat, Dec 07, 2024 at 09:59:08AM +0800, Tiezhu Yang wrote:
> +unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table)
> +{
> +	switch (reloc_type(reloc)) {
> +	case R_LARCH_32_PCREL:
> +	case R_LARCH_64_PCREL:
> +		if (reloc->sym->type == STT_SECTION)
> +			return reloc->sym->offset + reloc_addend(reloc) -
> +			       (reloc_offset(reloc) - reloc_offset(table));

How does this even work?  i.e., why does the reloc offset (basically the
jump table index) have anything to do with calculating the location of
the instruction it's referencing?

> +		else
> +			return reloc->sym->offset;

This also seems odd.  Why is the addend being ignored?  Shouldn't it
point to the instruction's offset?

-- 
Josh

On 12/10/2024 04:35 AM, Josh Poimboeuf wrote:
> On Sat, Dec 07, 2024 at 09:59:08AM +0800, Tiezhu Yang wrote:
>> +unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table)
>> +{
>> +	switch (reloc_type(reloc)) {
>> +	case R_LARCH_32_PCREL:
>> +	case R_LARCH_64_PCREL:
>> +		if (reloc->sym->type == STT_SECTION)
>> +			return reloc->sym->offset + reloc_addend(reloc) -
>> +			       (reloc_offset(reloc) - reloc_offset(table));
>
> How does this even work?  i.e., why does the reloc offset (basically the
> jump table index) have anything to do with calculating the location of
> the instruction it's referencing?

Let me try to explain it, this is related with the relocation type.

In short, the jump table index is not used to calculate the location of
the destination instruction for absolute relocation types, but it should
be used for PC relative relocation types.

For the most part, an absolute relocation type is used for rodata.
In the case of STT_SECTION, reloc->sym->offset is always zero, and
for the other symbol types, reloc_addend(reloc) is always zero, thus it
can use a simple statement "reloc->sym->offset + reloc_addend(reloc)"
to obtain the symbol offset for various symbol types.

When compiling on LoongArch, there are some PC relative relocation types
for rodata, it needs to calculate the symbol offset with "S + A - PC" in
this case according to the spec of "ELF for the LoongArch Architecture",
the "PC" is the index of each jump table which is equal with the value
of reloc_offset(reloc) - reloc_offset(table).

I will add the above description to the commit message to make it clear.

>
>> +		else
>> +			return reloc->sym->offset;
>
> This also seems odd.  Why is the addend being ignored?  Shouldn't it
> point to the instruction's offset?

Sorry for that, I forgot to calculate the table size if the symbol type
is local label generated by GCC on LoongArch, after doing that, no need
to check the symbol type, just check whether the reloc types are PC
relative, the final code should be something like this:

unsigned long arch_adjust_offset(struct reloc *reloc, struct reloc *table)
{
         switch (reloc_type(reloc)) {
         case R_LARCH_32_PCREL:
         case R_LARCH_64_PCREL:
                 return reloc->sym->offset + reloc_addend(reloc) -
                        (reloc_offset(reloc) - reloc_offset(table));
         default:
                 return reloc->sym->offset + reloc_addend(reloc);
         }
}

Thanks,
Tiezhu

On Wed, Dec 11, 2024 at 11:16:33AM +0800, Tiezhu Yang wrote:
> When compiling on LoongArch, there are some PC relative relocation types
> for rodata, it needs to calculate the symbol offset with "S + A - PC" in
> this case according to the spec of "ELF for the LoongArch Architecture",
> the "PC" is the index of each jump table which is equal with the value
> of reloc_offset(reloc) - reloc_offset(table).
> 
> I will add the above description to the commit message to make it clear.

I understand how PC-relative addressing works.

The oddity here is that "PC" is the jump table's base address rather
than the entry address.  That has nothing to do with the ELF spec or
even how R_LARCH_32_PCREL is implemented.  Rather it seems to be a quirk
related to how loongarch jump table code expects the entries to be.

So the arch_adjust_offset() name seems wrong, as this is specific to
the jump table implementation, rather than relocs in general.

Instead call it arch_jump_table_sym_offset()?

-- 
Josh

On 12/12/2024 08:35 AM, Josh Poimboeuf wrote:
> On Wed, Dec 11, 2024 at 11:16:33AM +0800, Tiezhu Yang wrote:
>> When compiling on LoongArch, there are some PC relative relocation types
>> for rodata, it needs to calculate the symbol offset with "S + A - PC" in
>> this case according to the spec of "ELF for the LoongArch Architecture",
>> the "PC" is the index of each jump table which is equal with the value
>> of reloc_offset(reloc) - reloc_offset(table).
>>
>> I will add the above description to the commit message to make it clear.
>
> I understand how PC-relative addressing works.
>
> The oddity here is that "PC" is the jump table's base address rather
> than the entry address.

If there is only one jump table in the rodata, the "PC" is the entry
address which is equal with the value of reloc_offset(reloc), at this
time, reloc_offset(table) is 0.

If there are many jump tables in the rodata, the "PC" is the offset
of the jump table's base address which is equal with the value of 
reloc_offset(reloc) - reloc_offset(table).

> That has nothing to do with the ELF spec or
> even how R_LARCH_32_PCREL is implemented.  Rather it seems to be a quirk
> related to how loongarch jump table code expects the entries to be.
>
> So the arch_adjust_offset() name seems wrong, as this is specific to
> the jump table implementation, rather than relocs in general.
>
> Instead call it arch_jump_table_sym_offset()?

OK, will modify it and add the above description to the commit message
to make it clear.

Thanks,
Tiezhu

On Wed, Dec 11, 2024 at 04:35:27PM -0800, Josh Poimboeuf wrote:
> On Wed, Dec 11, 2024 at 11:16:33AM +0800, Tiezhu Yang wrote:
> > When compiling on LoongArch, there are some PC relative relocation types
> > for rodata, it needs to calculate the symbol offset with "S + A - PC" in
> > this case according to the spec of "ELF for the LoongArch Architecture",
> > the "PC" is the index of each jump table which is equal with the value
> > of reloc_offset(reloc) - reloc_offset(table).
> > 
> > I will add the above description to the commit message to make it clear.
> 
> I understand how PC-relative addressing works.
> 
> The oddity here is that "PC" is the jump table's base address rather
> than the entry address.  That has nothing to do with the ELF spec or
> even how R_LARCH_32_PCREL is implemented.  Rather it seems to be a quirk
> related to how loongarch jump table code expects the entries to be.
> 
> So the arch_adjust_offset() name seems wrong, as this is specific to
> the jump table implementation, rather than relocs in general.
> 
> Instead call it arch_jump_table_sym_offset()?

BTW, do both GCC and Clang have this same behavior for loongarch?

-- 
Josh

On 12/12/2024 08:39 AM, Josh Poimboeuf wrote:
> On Wed, Dec 11, 2024 at 04:35:27PM -0800, Josh Poimboeuf wrote:
>> On Wed, Dec 11, 2024 at 11:16:33AM +0800, Tiezhu Yang wrote:
>>> When compiling on LoongArch, there are some PC relative relocation types
>>> for rodata, it needs to calculate the symbol offset with "S + A - PC" in
>>> this case according to the spec of "ELF for the LoongArch Architecture",
>>> the "PC" is the index of each jump table which is equal with the value
>>> of reloc_offset(reloc) - reloc_offset(table).
>>>
>>> I will add the above description to the commit message to make it clear.
>>
>> I understand how PC-relative addressing works.
>>
>> The oddity here is that "PC" is the jump table's base address rather
>> than the entry address.  That has nothing to do with the ELF spec or
>> even how R_LARCH_32_PCREL is implemented.  Rather it seems to be a quirk
>> related to how loongarch jump table code expects the entries to be.
>>
>> So the arch_adjust_offset() name seems wrong, as this is specific to
>> the jump table implementation, rather than relocs in general.
>>
>> Instead call it arch_jump_table_sym_offset()?
>
> BTW, do both GCC and Clang have this same behavior for loongarch?

Yes.