[v1,5/6] objtool: Add skipped member in struct reloc

Message ID 1690272910-11869-6-git-send-email-yangtiezhu@loongson.cn
State New
Headers
Series Add objtool and orc support for LoongArch |

Commit Message

Tiezhu Yang July 25, 2023, 8:15 a.m. UTC
  There exist multiple relocation types in one location, such as a pair of
R_LARCH_ADD32 and R_LARCH_SUB32 in section .rela.discard.unreachable and
.rela.discard.reachable on LoongArch.

Here is an example:

$ readelf -rW init/main.o

Relocation section '.rela.discard.unreachable' at offset 0x3e20 contains 2 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000000  0000000a00000032 R_LARCH_ADD32          0000000000000000 .init.text + 230
0000000000000000  0000001a00000037 R_LARCH_SUB32          0000000000000000 L0^A + 0

Relocation section '.rela.discard.reachable' at offset 0x4a00 contains 6 entries:
    Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
0000000000000000  0000000a00000032 R_LARCH_ADD32          0000000000000000 .init.text + ae0
0000000000000000  0000002800000037 R_LARCH_SUB32          0000000000000000 L0^A + 0
0000000000000004  0000000a00000032 R_LARCH_ADD32          0000000000000000 .init.text + b88
0000000000000004  0000002b00000037 R_LARCH_SUB32          0000000000000004 L0^A + 0
0000000000000008  0000000200000032 R_LARCH_ADD32          0000000000000000 .text + 23c
0000000000000008  0000002e00000037 R_LARCH_SUB32          0000000000000008 L0^A + 0

In order to silence the related objtool warnings, add skipped member
in struct reloc to record and skip the latter relocation.

Co-developed-by: Jinyang He <hejinyang@loongson.cn>
Signed-off-by: Jinyang He <hejinyang@loongson.cn>
Co-developed-by: Youling Tang <tangyouling@loongson.cn>
Signed-off-by: Youling Tang <tangyouling@loongson.cn>
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c               | 4 ++++
 tools/objtool/elf.c                 | 6 ++++++
 tools/objtool/include/objtool/elf.h | 1 +
 3 files changed, 11 insertions(+)
  

Comments

Peter Zijlstra July 25, 2023, 11:59 a.m. UTC | #1
On Tue, Jul 25, 2023 at 04:15:09PM +0800, Tiezhu Yang wrote:
> There exist multiple relocation types in one location, such as a pair of
> R_LARCH_ADD32 and R_LARCH_SUB32 in section .rela.discard.unreachable and
> .rela.discard.reachable on LoongArch.
> 
> Here is an example:
> 
> $ readelf -rW init/main.o
> 
> Relocation section '.rela.discard.unreachable' at offset 0x3e20 contains 2 entries:
>     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
> 0000000000000000  0000000a00000032 R_LARCH_ADD32          0000000000000000 .init.text + 230
> 0000000000000000  0000001a00000037 R_LARCH_SUB32          0000000000000000 L0^A + 0
> 

Please explain; why is this?

How does:

#define __annotate_unreachable(c) ({					\
	   asm volatile(__stringify_label(c) ":\n\t"                       \
			".pushsection .discard.unreachable\n\t"            \
			".long " __stringify_label(c) "b - .\n\t"          \
			".popsection\n\t" : : "i" (c));                    \
})
#define annotate_unreachable() __annotate_unreachable(__COUNTER__)

Manage to generate this..
  
Tiezhu Yang Aug. 3, 2023, 11:36 a.m. UTC | #2
On 07/25/2023 07:59 PM, Peter Zijlstra wrote:
> On Tue, Jul 25, 2023 at 04:15:09PM +0800, Tiezhu Yang wrote:
>> There exist multiple relocation types in one location, such as a pair of
>> R_LARCH_ADD32 and R_LARCH_SUB32 in section .rela.discard.unreachable and
>> .rela.discard.reachable on LoongArch.
>>
>> Here is an example:
>>
>> $ readelf -rW init/main.o
>>
>> Relocation section '.rela.discard.unreachable' at offset 0x3e20 contains 2 entries:
>>     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
>> 0000000000000000  0000000a00000032 R_LARCH_ADD32          0000000000000000 .init.text + 230
>> 0000000000000000  0000001a00000037 R_LARCH_SUB32          0000000000000000 L0^A + 0
>>
>
> Please explain; why is this?
>
> How does:
>
> #define __annotate_unreachable(c) ({					\
> 	   asm volatile(__stringify_label(c) ":\n\t"                       \
> 			".pushsection .discard.unreachable\n\t"            \
> 			".long " __stringify_label(c) "b - .\n\t"          \
> 			".popsection\n\t" : : "i" (c));                    \
> })
> #define annotate_unreachable() __annotate_unreachable(__COUNTER__)
>
> Manage to generate this..
>

Sorry for the late reply, let me try to explain it.

R_LARCH_ADD32 relocation type is 32-bit label addition,
R_LARCH_SUB32 relocation type is 32-bit label subtraction,
they are intended for local labels, the label difference
will be calculated as a constant before linking, preserve
a pair of R_LARCH_ADD32 and R_LARCH_SUB32 to fix the label
difference.

Here is a simple example:
-------------------------------
.L0
  // do something
.L1

.data
.word .L0 - .L1
-------------------------------

NewDiff = 0
R_LARCH_ADD32:
NewDiff = NewDiff + .L0_Addr
R_LARCH_SUB32:
NewDiff = NewDiff - .L1_Addr

I discussed offline with the engineers who are familiar with gnu
assembler of LoongArch, maybe it can modify the gnu assembler
to use R_LARCH_32_PCREL to replace a pair of R_LARCH_ADD32 and
R_LARCH_SUB32, then I will test it again and drop this change if
possible.

Here is LoongArch ELF Relocations spec:
https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc

R_LARCH_ADD32
32-bit in-place addition
*(int32_t *) PC += S + A

R_LARCH_SUB32
32-bit in-place subtraction
*(int32_t *) PC -= S + A

R_LARCH_32_PCREL
32-bit PC relative
(*(uint32_t *) PC) = (S+A-PC) [31 ... 0]

Thanks very much for your review comments, it is helpful to
make progress in the right direction.

Thanks,
Tiezhu
  
Peter Zijlstra Aug. 3, 2023, 12:11 p.m. UTC | #3
On Thu, Aug 03, 2023 at 07:36:24PM +0800, Tiezhu Yang wrote:

> > 			".long " __stringify_label(c) "b - .\n\t"          \

> I discussed offline with the engineers who are familiar with gnu
> assembler of LoongArch, maybe it can modify the gnu assembler
> to use R_LARCH_32_PCREL to replace a pair of R_LARCH_ADD32 and
> R_LARCH_SUB32, then I will test it again and drop this change if
> possible.

Yes, this is exactly what PC relative relocations are for.
  
Xi Ruoyao Aug. 9, 2023, 11:51 a.m. UTC | #4
On Thu, 2023-08-03 at 14:11 +0200, Peter Zijlstra wrote:
> On Thu, Aug 03, 2023 at 07:36:24PM +0800, Tiezhu Yang wrote:
> 
> > >                         ".long " __stringify_label(c) "b - .\n\t"          \
> 
> > I discussed offline with the engineers who are familiar with gnu
> > assembler of LoongArch, maybe it can modify the gnu assembler
> > to use R_LARCH_32_PCREL to replace a pair of R_LARCH_ADD32 and
> > R_LARCH_SUB32, then I will test it again and drop this change if
> > possible.

Hmm, but I don't like the idea to disallow users having GAS <= 2.41 from
using ORC unwinder.

Can we emit the R_LARCH_32_PCREL reloc with an explicit .reloc
directive?

> Yes, this is exactly what PC relative relocations are for.
  
Xi Ruoyao Aug. 9, 2023, 11:58 a.m. UTC | #5
On Wed, 2023-08-09 at 19:51 +0800, Xi Ruoyao wrote:
> On Thu, 2023-08-03 at 14:11 +0200, Peter Zijlstra wrote:
> > On Thu, Aug 03, 2023 at 07:36:24PM +0800, Tiezhu Yang wrote:
> > 
> > > >                         ".long " __stringify_label(c) "b - .\n\t"          \
> > 
> > > I discussed offline with the engineers who are familiar with gnu
> > > assembler of LoongArch, maybe it can modify the gnu assembler
> > > to use R_LARCH_32_PCREL to replace a pair of R_LARCH_ADD32 and
> > > R_LARCH_SUB32, then I will test it again and drop this change if
> > > possible.
> 
> Hmm, but I don't like the idea to disallow users having GAS <= 2.41 from
> using ORC unwinder.
> 
> Can we emit the R_LARCH_32_PCREL reloc with an explicit .reloc
> directive?

Answering myself: "maybe", I've written a simple PoC:

$ cat t.s
.global main
main:
  la.pcrel $t0, f
  ld.w     $t1, $t0, 0
  ldx.d    $a0, $t1, $t0
  jr       $ra

t:
  .dword   42
  .zero    64

f:
  .reloc   f, R_LARCH_32_PCREL, t
  .word    0
$ ./a.out
$ echo $?
42
  

Patch

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 9e5e462..602318e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -594,6 +594,8 @@  static int add_dead_ends(struct objtool_file *file)
 		goto reachable;
 
 	for_each_reloc(rsec, reloc) {
+		if (reloc->skipped)
+			continue;
 
 		if (reloc->sym->type != STT_SECTION) {
 			WARN("unexpected relocation symbol type in %s", rsec->name);
@@ -633,6 +635,8 @@  static int add_dead_ends(struct objtool_file *file)
 		return 0;
 
 	for_each_reloc(rsec, reloc) {
+		if (reloc->skipped)
+			continue;
 
 		if (reloc->sym->type != STT_SECTION) {
 			WARN("unexpected relocation symbol type in %s", rsec->name);
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index d420b5d..bd950d4 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -975,6 +975,12 @@  static int read_relocs(struct elf *elf)
 				return -1;
 			}
 
+			if (!(strcmp(rsec->name, ".rela.discard.unreachable")) ||
+			    !(strcmp(rsec->name, ".rela.discard.reachable"))) {
+				if (reloc->sym->type != STT_SECTION)
+					reloc->skipped = true;
+			}
+
 			elf_hash_add(reloc, &reloc->hash, reloc_hash(reloc));
 			reloc->sym_next_reloc = sym->relocs;
 			sym->relocs = reloc;
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index c532d70..4141837 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -75,6 +75,7 @@  struct reloc {
 	struct section *sec;
 	struct symbol *sym;
 	struct reloc *sym_next_reloc;
+	bool skipped;
 };
 
 struct elf {